Skip to content

Fix QR Login: treat authenticate success(false) as rejection, not completion#25520

Draft
Copilot wants to merge 3 commits intotrunkfrom
copilot/sub-pr-25517-again
Draft

Fix QR Login: treat authenticate success(false) as rejection, not completion#25520
Copilot wants to merge 3 commits intotrunkfrom
copilot/sub-pr-25517-again

Conversation

Copy link
Copy Markdown

Copilot AI commented May 4, 2026

QRLoginServiceRemote.authenticate calls its success closure with false when the server responds but rejects the authentication (i.e., authenticated == false in the API response). The coordinator was ignoring this value and always transitioning to the .done state, incorrectly firing qrLoginAuthenticated and showing the completion UI on a rejected login.

Changes

  • QRLoginVerifyCoordinator: Replace { _ in with { authenticated in } and guard on the value — false now transitions to .error, calls view.showAuthenticationFailedError(), and tracks qrLoginVerifyCodeFailed:
service.authenticate(token: token) { authenticated in
    guard authenticated else {
        self.state = .error
        self.view.showAuthenticationFailedError()
        self.parentCoordinator.track(.qrLoginVerifyCodeFailed, properties: ["error": "authentication_failed"])
        return
    }
    self.parentCoordinator.track(.qrLoginAuthenticated)
    self.state = .done
    self.view.renderCompletion()
} failure: { _ in ...
  • QRLoginVerifyCoordinatorTests: Add .authenticationRejected mock expectation (calls success(false)) and a corresponding test testConfirmFromWaitingForUserVerificationAndAuthenticationRejected asserting the coordinator reaches .error, shows the auth-failed UI, and records the failure track.

mokagio and others added 2 commits May 4, 2026 12:46
Adds `unused_closure_parameter` to `.swiftlint.yml`'s `only_rules` and
applies SwiftLint's autocorrection across the codebase.
340 violations were autofixed across 167 files; no manual fixes or
`swiftlint:disable` comments were needed.

Part of the Orchard SwiftLint rollout campaign.

---

Generated with the help of Claude Code, https://claude.com/claude-code

Co-Authored-By: Claude Code Opus 4.7 (1M context) <noreply@anthropic.com>
@dangermattic
Copy link
Copy Markdown
Collaborator

1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

…jection

When the server responds to the authenticate request but sets authenticated=false,
the success callback fires with `false`. Previously this was ignored (using `_`)
and the UI incorrectly moved to the done state with a qrLoginAuthenticated track.

Now we check the Bool: false → show authentication failed error + track failure;
true → proceed to done state as before.

Also adds a test case for the server-rejected scenario.

Agent-Logs-Url: https://github.com/wordpress-mobile/WordPress-iOS/sessions/378e64a4-563e-46cb-ad8b-017d726bc2ba

Co-authored-by: mokagio <1218433+mokagio@users.noreply.github.com>
Copilot AI changed the title [WIP] [WIP] Address feedback on enabling SwiftLint rule: unused_closure_parameter Fix QR Login: treat authenticate success(false) as rejection, not completion May 4, 2026
Copilot finished work on behalf of mokagio May 4, 2026 04:10
Copilot AI requested a review from mokagio May 4, 2026 04:10
@wpmobilebot
Copy link
Copy Markdown
Contributor

App Icon📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress
ConfigurationRelease-Alpha
Build Number32092
VersionPR #25520
Bundle IDorg.wordpress.alpha
Commitd845891
Installation URL2scpsgfj3p5e0
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Copy Markdown
Contributor

App Icon📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack
ConfigurationRelease-Alpha
Build Number32092
VersionPR #25520
Bundle IDcom.jetpack.alpha
Commitd845891
Installation URL2jamnaj81n78o
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

Base automatically changed from mokagio/swiftlint-unused-closure-parameter to trunk May 5, 2026 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants