Skip to content

Add support for handling banned accounts in login mutations and tests#161

Merged
malkoG merged 1 commit into
hackers-pub:mainfrom
LuticaCANARD:patch/auth-miss-match
Jul 15, 2026
Merged

Add support for handling banned accounts in login mutations and tests#161
malkoG merged 1 commit into
hackers-pub:mainfrom
LuticaCANARD:patch/auth-miss-match

Conversation

@LuticaCANARD

Copy link
Copy Markdown
Contributor

Resolved Symptoms

  • Fixed an issue where email-based login did not complete on Android
  • Fixed a failure in the login flow after verifying the email code/link

Root Cause

  • The completeLoginChallenge response can return a union result: either Session or AccountBannedError
  • The Android GraphQL operation and repository logic still assumed the response was always a Session
  • loginByPasskey uses the same kind of authentication result, so it could hit the same response-mapping issue

Solution

  • Updated completeLoginChallenge and loginByPasskey GraphQL operations to request __typename and union fragments
  • Mapped successful Session results into the existing domain Session model
  • Handled AccountBannedError as a failed login result with a message that includes the banned timestamp
  • Added repository tests for successful session responses and banned-account responses for both email and passkey login

References

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The GraphQL schema and operations introduce a new AccountBannedError type and union result types (CompleteLoginChallengeResult, LoginByPasskeyResult) for completeLoginChallenge and loginByPasskey mutations. The repository maps these union responses to domain sessions or banned-account failures, and new tests validate both outcomes.

Changes

Login union result handling

Layer / File(s) Summary
Schema and operation contract updates
app/src/main/graphql/pub/hackers/android/schema.graphqls, app/src/main/graphql/pub/hackers/android/operations.graphql
Adds AccountBannedError type with since: DateTime!, introduces CompleteLoginChallengeResult and LoginByPasskeyResult unions, updates the mutation return types, and rewrites operation selection sets with __typename and inline fragments for Session/AccountBannedError.
Repository mapping of union results
app/src/main/java/pub/hackers/android/data/repository/HackersPubRepository.kt
completeLoginChallenge and loginByPasskey branch on onSession/onAccountBannedError, mapping success via new toDomainSession() extensions and banned cases via accountBannedMessage(since), with an __typename-based fallback for unknown results.
Repository auth test suite
app/src/test/java/pub/hackers/android/data/repository/HackersPubRepositoryAuthTest.kt
New test file mocks ApolloClient/OkHttpClient/Context, stubs mutation execution, builds success/banned fixtures for both mutations, and asserts correct session mapping and banned-account failure messages.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant HackersPubRepository
  participant ApolloClient
  Client->>HackersPubRepository: completeLoginChallenge() / loginByPasskey()
  HackersPubRepository->>ApolloClient: execute mutation
  ApolloClient-->>HackersPubRepository: union result (Session or AccountBannedError)
  alt onSession present
    HackersPubRepository->>HackersPubRepository: toDomainSession()
    HackersPubRepository-->>Client: success(Session)
  else onAccountBannedError present
    HackersPubRepository->>HackersPubRepository: accountBannedMessage(since)
    HackersPubRepository-->>Client: failure(banned message)
  else unrecognized __typename
    HackersPubRepository-->>Client: failure(unknown result message)
  end
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: banned-account handling in login mutations and tests.
Description check ✅ Passed The description matches the code changes and explains the login union fix and added tests.
Linked Issues check ✅ Passed The changes add union-aware login handling and tests, which aligns with fixing the login failures reported in #160.
Out of Scope Changes check ✅ Passed The summary shows only login-flow schema, repository, and test updates, with no unrelated changes indicated.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
app/src/main/java/pub/hackers/android/data/repository/HackersPubRepository.kt (1)

926-954: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Optional: the two toDomainSession() bodies are identical, and accountBannedMessage takes Any.

The CompleteLoginChallengeMutation.OnSession and LoginByPasskeyMutation.OnSession mappers are byte-for-byte duplicates. Since they operate on distinct generated types they can't be merged directly, but you could funnel both through a small shared builder taking the primitive fields to keep them from drifting. Separately, accountBannedMessage(since: Any) interpolates the raw scalar (e.g. 2026-07-05T00:00:00Z) directly into a user-facing message; consider parsing/formatting the timestamp if this string is surfaced in the UI. Both are minor and consistent with the rest of the file, so feel free to defer.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@app/src/main/java/pub/hackers/android/data/repository/HackersPubRepository.kt`
around lines 926 - 954, The
`CompleteLoginChallengeMutation.OnSession.toDomainSession()` and
`LoginByPasskeyMutation.OnSession.toDomainSession()` mappings in
`HackersPubRepository` are duplicated, so extract the shared field-to-`Session`
construction into a small private helper and have both methods delegate to it to
prevent drift. Also update `accountBannedMessage(since: Any)` to avoid
interpolating the raw scalar directly; convert the `since` value into a properly
formatted, user-friendly timestamp before building the message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@app/src/main/java/pub/hackers/android/data/repository/HackersPubRepository.kt`:
- Around line 926-954: The
`CompleteLoginChallengeMutation.OnSession.toDomainSession()` and
`LoginByPasskeyMutation.OnSession.toDomainSession()` mappings in
`HackersPubRepository` are duplicated, so extract the shared field-to-`Session`
construction into a small private helper and have both methods delegate to it to
prevent drift. Also update `accountBannedMessage(since: Any)` to avoid
interpolating the raw scalar directly; convert the `since` value into a properly
formatted, user-friendly timestamp before building the message.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 75b25a25-4494-4243-b576-8aead5e69ceb

📥 Commits

Reviewing files that changed from the base of the PR and between b926a52 and c690c71.

📒 Files selected for processing (4)
  • app/src/main/graphql/pub/hackers/android/operations.graphql
  • app/src/main/graphql/pub/hackers/android/schema.graphqls
  • app/src/main/java/pub/hackers/android/data/repository/HackersPubRepository.kt
  • app/src/test/java/pub/hackers/android/data/repository/HackersPubRepositoryAuthTest.kt

@malkoG
malkoG merged commit fc06e3c into hackers-pub:main Jul 15, 2026
1 check passed
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.

Cannot login with email auth code and passkey

2 participants