fix: DeadlineLoginDialog.login() returns True on successful login

[crowecawcaw]

Fixes:

What was the problem/requirement? (What/Why)

DeadlineLoginDialog.login() returned False even on a successful login. A successful login calls self.accept(), so the underlying exec_() returns QDialog.Accepted (1), but the code compared the result against QMessageBox.Ok (1024). This broke the documented if ...login(): usage pattern — callers thought every login failed.

What was the solution? (How)

Compare exec_() against QDialog.DialogCode.Accepted instead of QMessageBox.Ok, so a successful (accepted) login returns True and a cancel/reject returns False.

What is the impact of this change?

Callers using the documented if ...login(): pattern now correctly see success.

How was this change tested?

Replaced the original isolated test with realistic pytest-qt (qtbot) tests that construct DeadlineLoginDialog through its real __init__ and drive the real modal exec_(), patching only the api.login backend (no AWS): success → accept() → True; user-cancel → False; login error → False. This exercises __init__, the background-task start, the success/error/cancel signal handlers, and exec_().

• Have you run the unit tests? Yes — test/unit/deadline_client/ui/dialogs/test_deadline_login_dialog.py (3 passed, stable across repeated runs under the default xdist config).
• Have you run the integration tests? No.

Was this change documented?

• No public-contract change (restores the documented return-value behavior).
• README.md not affected.

Does this PR introduce new dependencies?

• This PR adds one or more new dependency Python packages.
• This PR does not add any new dependencies.

Is this a breaking change?

No — it fixes the return value to match the documented contract.

Does this change impact security?

No.

Testing

Automated (headless, QT_QPA_PLATFORM=offscreen, PySide6):

• test/unit/deadline_client/ui/dialogs/test_deadline_login_dialog.py — 4 passed. These construct the real dialog via __init__ (real background task + signal wiring, only api.login patched) and drive the real modal exec_():
  • success with close_on_success=True → accept() → returns True
  • success with close_on_success=False → Ok button clicked → returns True
  • user clicks Cancel while login in flight → returns False
  • api.login raises → Close clicked → returns False
• Scratch probe stubbing super().exec_() directly confirmed the return-value mapping for every relevant result code: QDialog.Accepted (1) → True, QMessageBox.Ok (1024) → True, QDialog.Rejected (0) → False, QMessageBox.Cancel (4194304) → False, QMessageBox.Close (2097152) → False.

Not automatable — needs a human click-through (the real browser/SSO login handshake can't be scripted):

1. In a submitter (or any caller of DeadlineLoginDialog.login(...)), open the login dialog while logged out and complete a real login with the default close_on_success=True — the dialog should close itself and the caller should see success (e.g. "Logged in successfully" path, not the failure path).
2. Repeat with close_on_success=False (if a caller uses it): after login the dialog shows "Successfully logged into: ..." with an Ok button; clicking Ok must also report success to the caller.
3. Cancel mid-login and confirm the caller sees failure.

Manually verified (2026-07-23): completed a real browser SSO login through the GUI DeadlineLoginDialog from an editable install of this branch — on success the dialog closed itself and the caller correctly saw a logged-in state (no re-click needed), and cancelling mid-login was correctly treated as not-logged-in.

[github-actions]

This fixes the close_on_success=True path but regresses the close_on_success=False path.

When close_on_success=False, a successful login does not call self.accept(). Instead handle_login_thread_succeeded (lines 179-185) swaps in a QMessageBox.Ok button and waits for the user to click it. For a QMessageBox, clicking a standard button sets the dialog result to that button's enum value — QMessageBox.Ok (1024) — not QDialog.Accepted (1). So after a genuinely successful login in that mode, super().exec_() returns 1024 and this comparison yields 1024 == 1 → False, incorrectly reporting login failure.

The old == QMessageBox.Ok covered the close_on_success=False success (Ok button) but was wrong for the close_on_success=True success (accept() → 1024 == 1024 was the bug). A correct fix needs to accept both outcomes, e.g.:

result = super().exec_()
return result in (QDialog.DialogCode.Accepted, QMessageBox.Ok)

close_on_success=False is part of the public login() / __init__ signature, so this path is reachable by callers. Note the new tests only exercise close_on_success=True, so this regression is not caught.

[crowecawcaw]

Addressed in a801792: exec_() now returns result in (QDialog.DialogCode.Accepted, QMessageBox.Ok), covering both the close_on_success=True path (accept() -> QDialog.Accepted) and the close_on_success=False path (clicking the swapped-in Ok button -> QMessageBox.Ok). Also added test_exec_returns_true_on_success_when_not_closing_on_success to cover the close_on_success=False success path.