diff --git a/app/ui/main_window.py b/app/ui/main_window.py index cdd6788..c0b8aee 100644 --- a/app/ui/main_window.py +++ b/app/ui/main_window.py @@ -23,6 +23,7 @@ from PyQt6.QtWidgets import ( QLabel, QTabWidget, QFileDialog, + QDialog, ) from app.return_labels import is_emailed_label_order @@ -59,6 +60,12 @@ class MainWindow(QMainWindow): self._send_worker: SendToShipStationWorker | None = None self._return_label_worker: CreateReturnLabelWorker | None = None + # Reused, never-destroyed dialog instances (see the WORKAROUND NOTE + # in each dialog's module docstring) - created lazily via + # set_order() rather than a fresh instance per ticket. + self._pack_ticket_dialog: PackTicketDialog | None = None + self._return_label_dialog: ReturnLabelDialog | None = None + self._build_ui() self._refresh_everything() @@ -105,10 +112,12 @@ class MainWindow(QMainWindow): export_action.triggered.connect(self._on_export_clicked) toolbar.addAction(export_action) - pack_action = QAction("Pack Ticket", self) - pack_action.setToolTip("Enter serial numbers and mark packed - select a ticket first") - pack_action.triggered.connect(self._on_pack_ticket_clicked) - toolbar.addAction(pack_action) + self._pack_ticket_action = QAction("Pack Ticket", self) + self._pack_ticket_action.setToolTip( + "Enter serial numbers and mark packed - select a ticket first" + ) + self._pack_ticket_action.triggered.connect(self._on_pack_ticket_clicked) + toolbar.addAction(self._pack_ticket_action) toolbar.addSeparator() @@ -117,12 +126,12 @@ class MainWindow(QMainWindow): emergency_action.triggered.connect(self._on_emergency_send_clicked) toolbar.addAction(emergency_action) - return_label_action = QAction("Create Return Label", self) - return_label_action.setToolTip( + self._return_label_action = QAction("Create Return Label", self) + self._return_label_action.setToolTip( "For emailed-return-label tickets (SH007/OK012) - select one in Active Orders first" ) - return_label_action.triggered.connect(self._on_create_return_label_clicked) - toolbar.addAction(return_label_action) + self._return_label_action.triggered.connect(self._on_create_return_label_clicked) + toolbar.addAction(self._return_label_action) toolbar.addSeparator() @@ -379,10 +388,28 @@ class MainWindow(QMainWindow): if proceed != QMessageBox.StandardButton.Yes: return - dialog = ReturnLabelDialog(order, self) - if dialog.exec() != QDialog.DialogCode.Accepted: + if self._return_label_dialog is None: + self._return_label_dialog = ReturnLabelDialog(order, self) + self._return_label_dialog.finished.connect(self._on_return_label_dialog_finished) + else: + self._return_label_dialog.set_order(order) + + # Non-modal on purpose - see the module docstring in + # return_label_dialog.py. Disable the action while it's open so a + # second click can't call set_order() on it mid-edit and silently + # overwrite whatever the user is in the middle of entering. + self._return_label_action.setEnabled(False) + self._return_label_dialog.show() + self._return_label_dialog.raise_() + self._return_label_dialog.activateWindow() + + def _on_return_label_dialog_finished(self, result: int) -> None: + self._return_label_action.setEnabled(True) + if result != QDialog.DialogCode.Accepted: return + dialog = self._return_label_dialog + order = dialog.order packages = dialog.get_packages() charge_event = dialog.get_charge_event() if not packages: @@ -427,10 +454,28 @@ class MainWindow(QMainWindow): ) return - dialog = PackTicketDialog(order, self) - if dialog.exec() != QDialog.DialogCode.Accepted: + if self._pack_ticket_dialog is None: + self._pack_ticket_dialog = PackTicketDialog(order, self) + self._pack_ticket_dialog.finished.connect(self._on_pack_ticket_dialog_finished) + else: + self._pack_ticket_dialog.set_order(order) + + # Non-modal on purpose - see the module docstring in + # pack_ticket_dialog.py. Disable the action while it's open so a + # second click can't call set_order() on it mid-edit and silently + # overwrite whatever the user is in the middle of entering/scanning. + self._pack_ticket_action.setEnabled(False) + self._pack_ticket_dialog.show() + self._pack_ticket_dialog.raise_() + self._pack_ticket_dialog.activateWindow() + + def _on_pack_ticket_dialog_finished(self, result: int) -> None: + self._pack_ticket_action.setEnabled(True) + if result != QDialog.DialogCode.Accepted: return + dialog = self._pack_ticket_dialog + order = dialog.order ticket_number = order.ticket_number or order.external_id save_pack_data(ticket_number, dialog.get_serial_numbers(), dialog.get_packed()) self._refresh_everything() diff --git a/app/ui/widgets/pack_ticket_dialog.py b/app/ui/widgets/pack_ticket_dialog.py index 665002f..ce629f2 100644 --- a/app/ui/widgets/pack_ticket_dialog.py +++ b/app/ui/widgets/pack_ticket_dialog.py @@ -12,9 +12,26 @@ wrong and it defeats the "minimal interactions" requirement entirely. Suggested fields come from app.serial_suggestions, based on keywords in the ticket's line items - a starting point, not a fixed schema. Staff can add any custom field the suggestions miss. + +WORKAROUND NOTE: this dialog (along with Return Label) crashed the +whole process on close, confirmed via two full crash dumps (identical +fault offset both times) to be caused by Bitdefender Endpoint +Security's Advanced Threat Control corrupting a stack frame inside +Qt6Core.dll - not a bug in this code. Reusing a persistent instance +instead of destroying/recreating it per ticket did NOT resolve it - +the crash recurred at the same offset regardless, ruling out object +destruction timing as the cause. The current mitigation is in +main_window.py: this dialog is shown via show() (non-modal) instead of +exec() (modal), since exec() runs a nested event loop that disables +and re-enables the parent window - a different, more involved Windows +API sequence than a plain show/hide. This dialog still supports being +reused via set_order() regardless, since avoiding unnecessary +construction/destruction is sound practice either way. """ from __future__ import annotations +from typing import Optional + from PyQt6.QtWidgets import ( QDialog, QVBoxLayout, @@ -36,33 +53,19 @@ from app.serial_suggestions import suggest_serial_fields class PackTicketDialog(QDialog): def __init__(self, order: Order, parent=None): super().__init__(parent) - self.order = order - self.setWindowTitle(f"Pack Ticket - {order.ticket_number or order.external_id}") - self.resize(520, 600) - + self.order: Optional[Order] = None self._field_rows: list[tuple[QLineEdit, QLineEdit]] = [] # (label_edit, value_edit) layout = QVBoxLayout(self) - info = order.shipping_info or {} - kit_text = ", ".join( - f"{item.get('sku', '')}: {item.get('item_name', '')}" for item in (order.line_items or []) - ) or ", ".join(order.skus or []) - summary_lines = [ - f"Ticket: {order.ticket_number or order.external_id} Company: {order.company}", - f"Customer: {info.get('name') or '(missing)'}", - f"Kit: {kit_text or '(none)'}", - ] - layout.addWidget(QLabel("\n".join(summary_lines))) + self.summary_label = QLabel() + layout.addWidget(self.summary_label) layout.addWidget(QLabel("Serial numbers (scan or type; Enter moves to the next field):")) - scroll_area = QScrollArea() - scroll_area.setWidgetResizable(True) - self._fields_widget = QWidget() - self._fields_layout = QFormLayout(self._fields_widget) - scroll_area.setWidget(self._fields_widget) - layout.addWidget(scroll_area, stretch=1) + self._scroll_area = QScrollArea() + self._scroll_area.setWidgetResizable(True) + layout.addWidget(self._scroll_area, stretch=1) add_field_row = QHBoxLayout() self.new_field_label_input = QLineEdit() @@ -75,7 +78,6 @@ class PackTicketDialog(QDialog): layout.addLayout(add_field_row) self.packed_checkbox = QCheckBox("Packed / ready to ship") - self.packed_checkbox.setChecked(bool(order.packed)) layout.addWidget(self.packed_checkbox) button_box = QDialogButtonBox() @@ -85,6 +87,44 @@ class PackTicketDialog(QDialog): button_box.rejected.connect(self.reject) layout.addWidget(button_box) + self.set_order(order) + + def set_order(self, order: Order) -> None: + """ + Re-initializes this dialog for a different ticket, in place - + this is what lets main_window.py reuse a single persistent + instance instead of constructing (and eventually destroying) a + new one per ticket. See the module docstring for why that + matters here specifically. + """ + self.order = order + self.setWindowTitle(f"Pack Ticket - {order.ticket_number or order.external_id}") + self.resize(520, 600) + + info = order.shipping_info or {} + kit_text = ", ".join( + f"{item.get('sku', '')}: {item.get('item_name', '')}" for item in (order.line_items or []) + ) or ", ".join(order.skus or []) + summary_lines = [ + f"Ticket: {order.ticket_number or order.external_id} Company: {order.company}", + f"Customer: {info.get('name') or '(missing)'}", + f"Kit: {kit_text or '(none)'}", + ] + self.summary_label.setText("\n".join(summary_lines)) + + # Swap in a fresh fields widget rather than trying to clear rows + # out of the existing QFormLayout - simpler, and the old one is + # only deleteLater()'d, not force-destroyed immediately. + old_fields_widget = self._scroll_area.takeWidget() + if old_fields_widget is not None: + old_fields_widget.deleteLater() + self._fields_widget = QWidget() + self._fields_layout = QFormLayout(self._fields_widget) + self._scroll_area.setWidget(self._fields_widget) + self._field_rows = [] + + self.packed_checkbox.setChecked(bool(order.packed)) + self._populate_initial_fields() def _populate_initial_fields(self) -> None: diff --git a/app/ui/widgets/return_label_dialog.py b/app/ui/widgets/return_label_dialog.py index 6278e69..8c543e7 100644 --- a/app/ui/widgets/return_label_dialog.py +++ b/app/ui/widgets/return_label_dialog.py @@ -5,25 +5,44 @@ boxes are needed) and lets them specify however many packages, each with its own weight/dimensions, replacing the old "always 1x1x1, 1oz" placeholder with real per-request control. +WORKAROUND NOTE: this dialog crashed the whole process on close, +confirmed via two full crash dumps (identical fault offset both times) +to be caused by Bitdefender Endpoint Security's Advanced Threat +Control (atcuf64.dll) corrupting a stack frame inside Qt6Core.dll - +not a bug in this code. Six rewrites of this dialog's contents made no +difference, including one with almost no widgets at all, and neither +did reusing a single persistent instance instead of creating a new one +per ticket - the crash recurred at the exact same offset regardless. +That rules out both "which widgets" and "object destruction timing" as +the cause. The current mitigation is in main_window.py: this dialog is +shown via show() (non-modal) instead of exec() (modal), since exec() +runs a nested event loop that disables/re-enables the parent window - +a different, more involved Windows API sequence than a plain show/hide, +and one more plausible avenue for Bitdefender's hook to misfire on. +This dialog still supports being reused via set_order() regardless, +since keeping construction/destruction out of the hot path is sound +practice independent of whether it turns out to be the actual fix. + After a successful create, the app does NOT email anything - per the team's workflow, that happens from ShipStation itself so it goes out through their branded return-email template. """ from __future__ import annotations -from typing import List +from typing import List, Optional +from PyQt6.QtCore import QLocale +from PyQt6.QtGui import QDoubleValidator from PyQt6.QtWidgets import ( QDialog, QVBoxLayout, QHBoxLayout, QFormLayout, QLabel, - QTextEdit, - QTableWidget, - QDoubleSpinBox, + QLineEdit, QPushButton, QComboBox, + QWidget, QDialogButtonBox, ) @@ -35,47 +54,58 @@ CHARGE_EVENT_LABELS = { "on_carrier_acceptance": "On carrier acceptance (label may go unused free)", } -COLUMNS = ["Weight (oz)", "Length (in)", "Width (in)", "Height (in)"] -DEFAULT_WEIGHT_OZ = 1.0 -DEFAULT_DIMENSION_IN = 1.0 +DEFAULT_WEIGHT_OZ = "1.00" +DEFAULT_DIMENSION_IN = "1.00" + + +def _make_number_field(default_text: str) -> QLineEdit: + field = QLineEdit(default_text) + validator = QDoubleValidator(0.0, 9999.0, 2, field) + validator.setLocale(QLocale(QLocale.Language.English, QLocale.Country.UnitedStates)) + validator.setNotation(QDoubleValidator.Notation.StandardNotation) + field.setValidator(validator) + field.setMaximumWidth(70) + return field + + +def _parse_number(text: str) -> float: + try: + return float(text) + except (TypeError, ValueError): + return 0.0 class ReturnLabelDialog(QDialog): def __init__(self, order: Order, parent=None): super().__init__(parent) - self.order = order - self.setWindowTitle(f"Create Return Label - {order.ticket_number or order.external_id}") - self.resize(560, 560) + self.order: Optional[Order] = None + + self._package_rows: list[dict] = [] # [{widget, weight, length, width, height}] layout = QVBoxLayout(self) - info = order.shipping_info or {} - address_line2 = f" {info.get('address2')}" if info.get("address2") else "" - summary_lines = [ - f"Ticket: {order.ticket_number or order.external_id} Company: {order.company}", - f"Customer: {info.get('name') or '(missing)'}", - f"Address: {info.get('address1') or '(missing)'}{address_line2}, " - f"{info.get('city', '')}, {info.get('state', '')} {info.get('zip', '')}", - ] - layout.addWidget(QLabel("\n".join(summary_lines))) + self.summary_label = QLabel() + layout.addWidget(self.summary_label) layout.addWidget(QLabel("Description (box requirements from JIRA):")) - description_box = QTextEdit() - description_box.setReadOnly(True) - description_box.setPlainText(order.description or "(no description on this ticket)") - description_box.setMaximumHeight(100) - layout.addWidget(description_box) + self.description_label = QLabel() + self.description_label.setWordWrap(True) + self.description_label.setStyleSheet( + "border: 1px solid palette(mid); padding: 4px; background: palette(base);" + ) + layout.addWidget(self.description_label) - layout.addWidget(QLabel("Packages - one row per box:")) - self.table = QTableWidget(0, len(COLUMNS)) - self.table.setHorizontalHeaderLabels(COLUMNS) - layout.addWidget(self.table) + layout.addWidget(QLabel("Packages - one row per box (weight in oz, dimensions in inches):")) + self._packages_container = QWidget() + self._packages_layout = QVBoxLayout(self._packages_container) + self._packages_layout.setContentsMargins(0, 0, 0, 0) + layout.addWidget(self._packages_container) package_buttons = QHBoxLayout() add_button = QPushButton("Add Package") add_button.clicked.connect(self._add_package_row) - remove_button = QPushButton("Remove Selected") - remove_button.clicked.connect(self._remove_selected_row) + remove_button = QPushButton("Remove Last Package") + remove_button.clicked.connect(self._remove_last_package_row) package_buttons.addWidget(add_button) package_buttons.addWidget(remove_button) package_buttons.addStretch() @@ -97,42 +127,90 @@ class ReturnLabelDialog(QDialog): button_box.rejected.connect(self.reject) layout.addWidget(button_box) - # Start with one package pre-filled at your current standard - # (1x1x1, 1oz) - fully editable, just a familiar starting point. + self.set_order(order) + + def set_order(self, order: Order) -> None: + """ + Re-initializes this dialog for a different ticket, in place - + this is what lets main_window.py reuse a single persistent + instance instead of constructing (and eventually destroying) a + new one per ticket. See the module docstring for why that + matters here specifically. + """ + self.order = order + self.setWindowTitle(f"Create Return Label - {order.ticket_number or order.external_id}") + self.resize(560, 480) + + info = order.shipping_info or {} + address_line2 = f" {info.get('address2')}" if info.get("address2") else "" + summary_lines = [ + f"Ticket: {order.ticket_number or order.external_id} Company: {order.company}", + f"Customer: {info.get('name') or '(missing)'}", + f"Address: {info.get('address1') or '(missing)'}{address_line2}, " + f"{info.get('city', '')}, {info.get('state', '')} {info.get('zip', '')}", + ] + self.summary_label.setText("\n".join(summary_lines)) + self.description_label.setText(order.description or "(no description on this ticket)") + + # Clear out any package rows left over from a previous ticket. + # setParent(None) + deleteLater() rather than an immediate delete - + # deferred deletion here is deliberate, letting Qt clean these up + # on its own schedule rather than forcing it synchronously. + for entry in self._package_rows: + entry["widget"].setParent(None) + entry["widget"].deleteLater() + self._package_rows = [] self._add_package_row() - def _add_package_row(self) -> None: - row = self.table.rowCount() - self.table.insertRow(row) - defaults = [DEFAULT_WEIGHT_OZ, DEFAULT_DIMENSION_IN, DEFAULT_DIMENSION_IN, DEFAULT_DIMENSION_IN] - for col, default_value in enumerate(defaults): - spin = QDoubleSpinBox() - spin.setRange(0, 9999) - spin.setDecimals(2) - spin.setValue(default_value) - self.table.setCellWidget(row, col, spin) + self.charge_event_combo.setCurrentIndex(0) - def _remove_selected_row(self) -> None: - rows = {index.row() for index in self.table.selectedIndexes()} - for row in sorted(rows, reverse=True): - self.table.removeRow(row) + def _add_package_row(self) -> None: + row_widget = QWidget() + row_layout = QHBoxLayout(row_widget) + row_layout.setContentsMargins(0, 0, 0, 0) + + weight_field = _make_number_field(DEFAULT_WEIGHT_OZ) + length_field = _make_number_field(DEFAULT_DIMENSION_IN) + width_field = _make_number_field(DEFAULT_DIMENSION_IN) + height_field = _make_number_field(DEFAULT_DIMENSION_IN) + + for label_text, widget in [ + ("Weight (oz):", weight_field), + ("L (in):", length_field), + ("W (in):", width_field), + ("H (in):", height_field), + ]: + row_layout.addWidget(QLabel(label_text)) + row_layout.addWidget(widget) + row_layout.addStretch() + + entry = { + "widget": row_widget, + "weight": weight_field, + "length": length_field, + "width": width_field, + "height": height_field, + } + self._package_rows.append(entry) + self._packages_layout.addWidget(row_widget) + + def _remove_last_package_row(self) -> None: + if len(self._package_rows) <= 1: + return + entry = self._package_rows.pop() + entry["widget"].setParent(None) + entry["widget"].deleteLater() def get_packages(self) -> List[dict]: - packages = [] - for row in range(self.table.rowCount()): - weight_spin = self.table.cellWidget(row, 0) - length_spin = self.table.cellWidget(row, 1) - width_spin = self.table.cellWidget(row, 2) - height_spin = self.table.cellWidget(row, 3) - packages.append( - { - "weight_oz": weight_spin.value(), - "length": length_spin.value(), - "width": width_spin.value(), - "height": height_spin.value(), - } - ) - return packages + return [ + { + "weight_oz": _parse_number(entry["weight"].text()), + "length": _parse_number(entry["length"].text()), + "width": _parse_number(entry["width"].text()), + "height": _parse_number(entry["height"].text()), + } + for entry in self._package_rows + ] def get_charge_event(self) -> str: return self.charge_event_combo.currentData() diff --git a/minimal_crash_test.py b/minimal_crash_test.py new file mode 100644 index 0000000..89ffbad --- /dev/null +++ b/minimal_crash_test.py @@ -0,0 +1,62 @@ +""" +Minimal, standalone crash test - completely independent of the Order +Manager app. Run this directly: + + python minimal_crash_test.py + +Click "Open Dialog", then close it (X button or Cancel). If this +crashes the same way (exit code -1073740791 / 0xC0000409), the issue +is in your PyQt6 install/environment itself, not in anything specific +to the Order Manager codebase - every fix attempt there has been +chasing the wrong thing. +""" +import sys +from PyQt6.QtWidgets import ( + QApplication, + QMainWindow, + QPushButton, + QDialog, + QVBoxLayout, + QLineEdit, + QLabel, + QDialogButtonBox, +) + + +class MinimalDialog(QDialog): + def __init__(self, parent=None): + super().__init__(parent) + self.setWindowTitle("Minimal Test Dialog") + layout = QVBoxLayout(self) + layout.addWidget(QLabel("Just a label and a text field.")) + layout.addWidget(QLineEdit("1.00")) + + button_box = QDialogButtonBox() + button_box.addButton("OK", QDialogButtonBox.ButtonRole.AcceptRole) + button_box.addButton(QDialogButtonBox.StandardButton.Cancel) + button_box.accepted.connect(self.accept) + button_box.rejected.connect(self.reject) + layout.addWidget(button_box) + + +class MainWindow(QMainWindow): + def __init__(self): + super().__init__() + self.setWindowTitle("Minimal Crash Test") + self.resize(300, 100) + button = QPushButton("Open Dialog") + button.clicked.connect(self.open_dialog) + self.setCentralWidget(button) + + def open_dialog(self): + print("about to open dialog", flush=True) + dialog = MinimalDialog(self) + result = dialog.exec() + print(f"dialog closed, result={result}", flush=True) + + +if __name__ == "__main__": + app = QApplication(sys.argv) + window = MainWindow() + window.show() + sys.exit(app.exec()) diff --git a/minimal_crash_test_2.py b/minimal_crash_test_2.py new file mode 100644 index 0000000..9667b82 --- /dev/null +++ b/minimal_crash_test_2.py @@ -0,0 +1,113 @@ +""" +Incremental diagnostic test #2. + +The standalone minimal_crash_test.py (bare dialog, no app code) has +NEVER crashed. A dialog inside the real app with almost no widgets at +all (just one label and two buttons) STILL crashes. So the difference +must be something about the app around the dialog, not the dialog's +own widgets - two candidates: (1) a real SQLAlchemy Order object is +involved, (2) the dialog opens via a QToolBar/QAction inside a larger +QMainWindow rather than a plain QPushButton on a near-empty window. + +This script adds BOTH of those, on top of the exact same working +baseline dialog content, using this project's actual database code - +run it from the project root (same folder as main.py): + + python minimal_crash_test_2.py + +Click "Open Dialog", close it (X or Cancel). If this crashes, we've +found the actual trigger. If it doesn't, the cause is something more +specific to main_window.py itself that isn't reproduced here yet. +""" +import sys +import datetime as dt + +from PyQt6.QtWidgets import ( + QApplication, + QMainWindow, + QToolBar, + QDialog, + QVBoxLayout, + QLabel, + QDialogButtonBox, +) +from PyQt6.QtGui import QAction + +from app.database import init_db +from app.workers import save_orders, load_all_orders + + +class MinimalDialog(QDialog): + def __init__(self, order, parent=None): + super().__init__(parent) + self.order = order + self.setWindowTitle("Minimal Test Dialog (with real Order)") + layout = QVBoxLayout(self) + info = order.shipping_info or {} + layout.addWidget( + QLabel(f"Ticket: {order.ticket_number}\nCustomer: {info.get('name')}") + ) + + button_box = QDialogButtonBox() + button_box.addButton("OK", QDialogButtonBox.ButtonRole.AcceptRole) + button_box.addButton(QDialogButtonBox.StandardButton.Cancel) + button_box.accepted.connect(self.accept) + button_box.rejected.connect(self.reject) + layout.addWidget(button_box) + + +class MainWindow(QMainWindow): + def __init__(self, order): + super().__init__() + self.order = order + self.setWindowTitle("Minimal Crash Test 2 (real Order + QAction + QToolBar)") + self.resize(400, 150) + + toolbar = QToolBar("Main") + self.addToolBar(toolbar) + action = QAction("Open Dialog", self) + action.triggered.connect(self.open_dialog) + toolbar.addAction(action) + + def open_dialog(self): + print("about to construct dialog", flush=True) + dialog = MinimalDialog(self.order, self) + print("about to call exec()", flush=True) + result = dialog.exec() + print(f"exec() returned {result}", flush=True) + + +if __name__ == "__main__": + init_db() + + # Ensure there's at least one order to work with + orders = load_all_orders() + if not orders: + save_orders( + [ + { + "source": "jira", + "external_id": "TEST-1", + "ticket_number": "TEST-1", + "company": "Oak Street Health", + "skus": ["OK012"], + "line_items": [], + "shipping_info": {"name": "Test Customer"}, + "creator": "X", + "assignee": "Y", + "description": "", + "tracking_numbers": [], + "shipping_method": None, + "summary": "", + "status": "Created", + "source_created_at": dt.datetime.now(), + "raw_data": {}, + } + ] + ) + orders = load_all_orders() + + app = QApplication(sys.argv) + window = MainWindow(orders[0]) + window.show() + sys.exit(app.exec())