-
Notifications
You must be signed in to change notification settings - Fork 0
Update links to be acm.gg instead of go.acm #365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughExternal links across API and UI components are updated from Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
💰 Infracost reportMonthly estimate generatedThis comment will be updated when code changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
src/api/functions/ses.ts(1 hunks)src/ui/components/AppShell/index.tsx(1 hunks)src/ui/components/AuthGuard/index.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build Application
- GitHub Check: Run Unit Tests
🔇 Additional comments (2)
src/ui/components/AuthGuard/index.tsx (1)
214-215: LGTM! Access request link properly updated.The access request form link has been correctly migrated to the new
acm.ggdomain and is now rendered inline within the message.src/ui/components/AppShell/index.tsx (1)
124-124: LGTM! External navigation links properly updated.Both the reimbursement request and pizza request form links have been correctly migrated to the new
acm.ggdomain.Also applies to: 130-130
| </p> | ||
| <div style="text-align: center; margin-top: 20px;"> | ||
| <a href="https://go.acm.illinois.edu/discord" class="button">Join our Discord</a> | ||
| <a href="https://acm.gg/discord" class="button">Join our Discord</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
LGTM! Domain migration looks correct.
The Discord link has been properly updated to the new acm.gg domain.
Verify that the new URL is accessible and redirects correctly:
🏁 Script executed:
#!/bin/bash
# Description: Verify the new Discord link is accessible and check for any other instances of the old domain
# Check if the new URL is accessible
echo "Testing new Discord URL..."
curl -I -L -s -o /dev/null -w "%{http_code}" https://acm.gg/discord
# Search for any remaining instances of the old domain in the codebase
echo -e "\nSearching for remaining instances of go.acm.illinois.edu..."
rg -n "go\.acm\.illinois\.edu" --type-add 'web:*.{ts,tsx,js,jsx,html}' --type webLength of output: 180
Update test file to reflect domain migration.
The new Discord URL is accessible (HTTP 200), but the domain migration is incomplete. The test file still references the old domain:
git/tests/unit/organizations.test.ts:55: Update"https://go.acm.illinois.edu/discord"to"https://acm.gg/discord"
🤖 Prompt for AI Agents
In src/api/functions/ses.ts around line 229 the HTML anchor uses the new Discord
URL but the unit test git/tests/unit/organizations.test.ts (line ~55) still
expects "https://go.acm.illinois.edu/discord"; update that test expectation to
"https://acm.gg/discord" (or adjust the fixture string) so the test matches the
migrated domain; run the unit tests to verify and commit the updated test file.
Summary by CodeRabbit