Fix: don't show "Join discord" for discord users#3111
Fix: don't show "Join discord" for discord users#3111internet-addict-xyz wants to merge 1 commit intoopenfrontio:mainfrom
Conversation
WalkthroughThe WinModal component now checks if a user is already a Discord member. When the random branch would display the Discord promo, it shows a pattern button instead if the user has a Discord account linked. The state is populated from user data during pattern content loading. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
Thanks! Though, them having a logged-in Discord account doesn't mean they are member of the Openfront discord server, or does it? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/client/graphics/layers/WinModal.ts`:
- Around line 166-169: The current loadPatternContent() sets this.hasDiscord
using me.user.discord !== undefined which only detects a linked Discord account,
not server membership; update loadPatternContent (and related logic that uses
this.hasDiscord) to verify actual Openfront server membership instead of just a
linked account — either by reading a dedicated membership flag returned by
getUserMe() (e.g., me.user.isOpenfrontMember or similar) or by adding a new API
call that checks membership (e.g., checkDiscordServerMembership(userId) or a
backend endpoint) and set this.hasDiscord only when that membership check
succeeds; ensure you adjust any downstream UI usage to rely on the membership
check rather than me.user.discord.
🧹 Nitpick comments (1)
src/client/graphics/layers/WinModal.ts (1)
169-169: Add optional chaining for safer property access.If
me.useris ever undefined (API edge case or future change), this line throws. Using optional chaining is a small defensive improvement.Suggested fix
- this.hasDiscord = me !== false && me.user.discord !== undefined; + this.hasDiscord = me !== false && me.user?.discord !== undefined;
| async loadPatternContent() { | ||
| const me = await getUserMe(); | ||
| const patterns = await fetchCosmetics(); | ||
| this.hasDiscord = me !== false && me.user.discord !== undefined; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check if the file exists and read context around the specific lines
echo "=== Reading WinModal.ts context (lines 160-175) ==="
wc -l src/client/graphics/layers/WinModal.ts
sed -n '160,175p' src/client/graphics/layers/WinModal.ts
echo -e "\n=== Searching for Discord server/membership logic ==="
rg -n "discord" --type=ts -i | grep -i "member\|guild\|server\|openfront" | head -20
echo -e "\n=== Searching for broader patterns around hasDiscord usage ==="
rg -n "hasDiscord" --type=ts -C3
echo -e "\n=== Looking for guild or server membership checks ==="
rg -n "guild\|membership\|member.*server" --type=ts -i | head -20Repository: openfrontio/OpenFrontIO
Length of output: 2346
🏁 Script executed:
#!/bin/bash
# Check issue `#3093` references or comments in code
echo "=== Searching for issue `#3093` or related promo/membership logic ==="
rg -n "3093\|Openfront.*community\|community.*server" --type=ts -i
echo -e "\n=== Checking how getUserMe() is used and what it returns ==="
rg -n "getUserMe|user\.discord" --type=ts -B2 -A2 | head -40Repository: openfrontio/OpenFrontIO
Length of output: 2804
The code checks only if Discord is linked, not if the user is a server member.
The current implementation at line 169 sets hasDiscord based on me.user.discord !== undefined, which indicates a linked Discord account. However, issue #3093 requires hiding the promo if the user "is already a member of the Openfront community server." These are not the same—linking Discord does not guarantee server membership.
No explicit server membership verification exists in the codebase. Either:
- The app's flow implicitly guarantees that linking Discord means the user is already in the server, or
- An explicit membership check is needed (e.g., via Discord API or stored membership flag)
Clarify which behavior is intended and update the implementation if needed.
🤖 Prompt for AI Agents
In `@src/client/graphics/layers/WinModal.ts` around lines 166 - 169, The current
loadPatternContent() sets this.hasDiscord using me.user.discord !== undefined
which only detects a linked Discord account, not server membership; update
loadPatternContent (and related logic that uses this.hasDiscord) to verify
actual Openfront server membership instead of just a linked account — either by
reading a dedicated membership flag returned by getUserMe() (e.g.,
me.user.isOpenfrontMember or similar) or by adding a new API call that checks
membership (e.g., checkDiscordServerMembership(userId) or a backend endpoint)
and set this.hasDiscord only when that membership check succeeds; ensure you
adjust any downstream UI usage to rely on the membership check rather than
me.user.discord.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
Yeah... I’m dumbass. Oops, sorry. |
github-project-automation
bot
moved this from Development
to Complete
in OpenFront Release Management
No, mistakes happen, thanks again |
2 participants
Resolves #3093
Description:
If the user is logged in with Discord, then don't promote it at the end of a match.
Please complete the following:
I have added screenshots for all UI updates
I process any text displayed to the user through translateText() and I've added it to the en.json file
I have added relevant tests to the test directory
I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced
Please put your Discord username so you can be contacted if a bug or regression is found:
internet_addict_