Show alliances in replay and spectator player panel#3643
Draft
nicolas-machugounot wants to merge 16 commits intoopenfrontio:mainfrom
Draft
Show alliances in replay and spectator player panel#3643nicolas-machugounot wants to merge 16 commits intoopenfrontio:mainfrom
nicolas-machugounot wants to merge 16 commits intoopenfrontio:mainfrom
Conversation
…rs dans le processus de contribution et les exigences de qualité du code.
…es diviser en plusieurs parties
…ront afin d'améliorer le processus de contribution et de validation.
…endance à Discord, incluant les objectifs, les endpoints API, et les critères d'acceptation.
…session authentifiée et payload email + redirectDomain. Ajout d’une section UI dans le compte connecté Discord (sans email lié) pour saisir un email de secours et lancer la liaison. Gestion des retours utilisateur via alertes succès/erreur, sans casser le flow existant de magic link classique.
…mail de liaison Gestion du callback #link-email?link-email-token=... qui confirme la liaison Appel POST /auth/link/email/confirm pour valider le token et lier l'email au compte UI améliorée dans le modal compte (section de liaison email pour Discord sans email lié) Messages d'alerte explicites et rafraîchissement auto après confirmation
…session authentifiée et payload email + redirectDomain. Ajout d’une section UI dans le compte connecté Discord (sans email lié) pour saisir un email de secours et lancer la liaison. Gestion des retours utilisateur via alertes succès/erreur, sans casser le flow existant de magic link classique.
…th.ts. Ajout de "ignoreDeprecations": "6.0" dans tsconfig.json pour lever l’erreur liée à baseUrl.
…s chemins et des dépréciations
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
WalkthroughAdds an email-based account-linking flow with client Auth APIs, AccountModal UI and hash-route confirmation, spectator-safe PlayerPanel rendering and tests, contributor automation/chatmode/prompt files, a docs plan for issue Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant AccountModal
participant Auth as Auth.ts
participant Server
participant Email as Email Service
participant Main as Main.ts
User->>AccountModal: Enter email & click "Send Backup Link"
AccountModal->>AccountModal: Validate email input
AccountModal->>Auth: sendLinkEmailStart(email)
Auth->>Server: POST /auth/link/email/start {email, redirectDomain}
Server->>Email: Generate token & send link
Email-->>User: Email with link containing hash token
User->>Main: Open site with hash (click link)
Main->>Main: Extract token from hash
Main->>Auth: confirmEmailLink(token)
Auth->>Server: POST /auth/link/email/confirm {token}
Server->>Server: Validate token & link email to account
Server-->>Auth: { email } on success or error
Auth-->>Main: { email } or null
Main->>User: Show success/failure alert and reload on success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/client/AccountModal.ts (1)
416-435: Consider using a more specific success message.The handler works correctly, but on success it shows
account_modal.recovery_email_sentwhich says "Recovery email sent to {email}". For a backup link flow, this might confuse users since it is not a recovery action. Consider adding a specific message like "Backup link sent to {email}" or similar.Suggested change
If a dedicated message is preferred, add a new key in
en.json:"backup_link_sent": "Backup link sent to {email}"Then update the handler:
if (success) { alert( - translateText("account_modal.recovery_email_sent", { + translateText("account_modal.backup_link_sent", { email, }), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/AccountModal.ts` around lines 416 - 435, Change the success message in handleStartEmailLink to use a dedicated backup-link translation key instead of the generic recovery key: add a new i18n key (e.g. "account_modal.backup_link_sent": "Backup link sent to {email}") in the locale file(s) (en.json and other locales as needed), then replace the translateText call that currently uses "account_modal.recovery_email_sent" with the new "account_modal.backup_link_sent" key and keep passing the email interpolation; ensure any tests or snapshot strings referencing the old key are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/agents/openfront-contributor.agent.md:
- Around line 122-129: The "Output Format" block is inconsistent: it currently
reads "Return responses in French" while the rest of the document is English;
update the phrase under the "## Output Format" heading (the line containing
"Return responses in French") to match the repository language (e.g., "Return
responses in English") or change it to a neutral statement like "Return
responses in the user's language" so the instruction aligns with the surrounding
English content and intent.
In @.github/prompts/openfront-contribution-readiness.prompt.md:
- Around line 1-57: This prompt file
(.github/prompts/openfront-contribution-readiness.prompt.md) is unrelated to the
spectator/replay player panel change and should not be included in this
PR—remove the file from this branch or revert its addition in the current
commit, or move it to a new branch and open a separate PR (or open an issue
linking that new PR) so the repository workflow prompt lands independently;
ensure the branch containing changes to the player panel does not modify or add
openfront-contribution-readiness.prompt.md and update the PR so only files
related to the player panel (e.g., files that reference the spectator/replay
player) remain.
In `@docs/issue-3622-login-alternative-plan.md`:
- Around line 1-103: Remove the unrelated planning document "Plan
d'implementation - Issue `#3622`" from this PR (it does not belong to the
replay/spectator alliance visibility change); delete the file (or move it into a
new branch/PR) so the current PR only contains auth/visibility changes, then
amend the branch history (e.g., remove the file from commits or create a focused
branch) and force-push/update the PR so the diff no longer includes the "Plan
d'implementation - Issue `#3622`" planning doc.
In `@src/client/Main.ts`:
- Line 21: The commit introduced email-link auth flow changes into the UI-only
branch; remove or move these auth-route changes into a separate PR by reverting
usage/imports of confirmEmailLink and userAuth from Main.ts and undoing related
auth/session code in the blocks containing the confirmEmailLink and userAuth
handling (also revert edits around the regions you mentioned at the ranges
referencing email-link handling and auth/session logic — e.g., functions or
handlers named confirmEmailLink, userAuth, and any added session/auth route
wiring). Ensure Main.ts imports go back to their previous UI-only set (remove
confirmEmailLink and userAuth), strip any added route handlers or session side
effects in the other referenced code regions, and isolate the entire email-link
flow into a new branch/PR so the UI fix remains spectator/replay-safe.
---
Nitpick comments:
In `@src/client/AccountModal.ts`:
- Around line 416-435: Change the success message in handleStartEmailLink to use
a dedicated backup-link translation key instead of the generic recovery key: add
a new i18n key (e.g. "account_modal.backup_link_sent": "Backup link sent to
{email}") in the locale file(s) (en.json and other locales as needed), then
replace the translateText call that currently uses
"account_modal.recovery_email_sent" with the new
"account_modal.backup_link_sent" key and keep passing the email interpolation;
ensure any tests or snapshot strings referencing the old key are updated
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc8aefa3-525e-42bd-94b0-e31efac74397
📒 Files selected for processing (11)
.github/agents/openfront-contributor.agent.md.github/chatmodes/openfront-contributor.chatmode.md.github/prompts/openfront-contribution-readiness.prompt.mddocs/issue-3622-login-alternative-plan.mdresources/lang/en.jsonsrc/client/AccountModal.tssrc/client/Auth.tssrc/client/Main.tssrc/client/graphics/layers/PlayerPanel.tstests/client/graphics/layers/PlayerPanelKick.test.tstsconfig.json
| import "./AccountModal"; | ||
| import { getUserMe } from "./Api"; | ||
| import { userAuth } from "./Auth"; | ||
| import { confirmEmailLink, userAuth } from "./Auth"; |
Contributor
There was a problem hiding this comment.
Split the email-link flow out of this PR.
These auth-route changes do not support replay/spectator alliance rendering and add new auth/session risk to a UI-only fix. Please move them to a separate PR or drop them from this branch.
Also applies to: 693-705, 893-913
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/Main.ts` at line 21, The commit introduced email-link auth flow
changes into the UI-only branch; remove or move these auth-route changes into a
separate PR by reverting usage/imports of confirmEmailLink and userAuth from
Main.ts and undoing related auth/session code in the blocks containing the
confirmEmailLink and userAuth handling (also revert edits around the regions you
mentioned at the ranges referencing email-link handling and auth/session logic —
e.g., functions or handlers named confirmEmailLink, userAuth, and any added
session/auth route wiring). Ensure Main.ts imports go back to their previous
UI-only set (remove confirmEmailLink and userAuth), strip any added route
handlers or session side effects in the other referenced code regions, and
isolate the entire email-link flow into a new branch/PR so the UI fix remains
spectator/replay-safe.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
Author
|
Please assign a milestone to PR #3643 to unblock required check Has Milestone |
Contributor
|
same as this comment #3642 (comment) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
Scope
Please complete the following:
Testing
Notes