-
Notifications
You must be signed in to change notification settings - Fork 167
fix: replace my site urls and use env based redirection #1381
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThree test files add fake timers for deterministic time-based assertions. The URL constants file is refactored to support environment-aware prefixes for staging and production. The Dropdown component migrates from MY_SITE_URL to MAIN_SITE_URL. New URL constants for items, tags, levels, and tasks are introduced. Events URL changes from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
__tests__/Unit/Components/Tasks/Card.test.tsx (1)
539-555: Fix timer management conflict with suite-level fake timers.This test calls
jest.useFakeTimers()andjest.setSystemTime()within a specific test, then restores real timers withjest.useRealTimers()at line 554. However, the entire test suite already uses fake timers (set at line 76), and many other tests rely onjest.advanceTimersByTime()(e.g., lines 156, 172, 357, etc.). Restoring real timers here will break those tests if they run after this one.🔧 Proposed fix
Since the suite-level fake timers are already active, only set the system time without redundantly calling
useFakeTimers(), and remove theuseRealTimers()call:it('renders "Started" with a specific date if status is not AVAILABLE', () => { - jest.useFakeTimers(); jest.setSystemTime(new Date('2025-01-01T00:00:00Z')); const { getByTestId } = renderWithRouter( <Provider store={store()}> <Card content={CONTENT[2]} shouldEdit={true} onContentChange={jest.fn()} /> </Provider>, {} ); const spanElement = screen.getByTestId('started-on'); expect(spanElement).toHaveTextContent('Started 4 years ago'); // Mocked date from moment - jest.useRealTimers(); });
🤖 Fix all issues with AI agents
In @src/constants/url.ts:
- Around line 5-6: The staging detection using
process.env.NEXT_PUBLIC_BASE_URL?.includes('staging') is too loose; replace it
with a precise check such as parsing NEXT_PUBLIC_BASE_URL hostname (or using a
regex that matches subdomain prefix like /^staging(\.|-)/) or, even better, rely
on an explicit env flag (e.g., process.env.NEXT_PUBLIC_ENV === 'staging');
update the isStaging variable to use that precise check and keep envPrefix =
isStaging ? 'staging-' : '' so downstream code uses the correct prefix.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
__tests__/Unit/Components/ExtensionRequest/ExtensionStatusModal.test.tsx__tests__/Unit/Components/Navbar/Navbar.test.tsx__tests__/Unit/Components/Tasks/Card.test.tsxsrc/components/Dropdown/Dropdown.tsxsrc/constants/url.ts
🧰 Additional context used
🧬 Code graph analysis (2)
__tests__/Unit/Components/ExtensionRequest/ExtensionStatusModal.test.tsx (1)
src/components/ExtensionRequest/ExtensionStatusModal.tsx (1)
formatToRelativeTime(24-27)
src/components/Dropdown/Dropdown.tsx (1)
src/constants/url.ts (1)
MAIN_SITE_URL(35-35)
⏰ 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). (1)
- GitHub Check: build (18.x)
🔇 Additional comments (5)
__tests__/Unit/Components/ExtensionRequest/ExtensionStatusModal.test.tsx (1)
122-133: LGTM! Fake timers ensure deterministic time-based assertions.The use of fake timers with a fixed system time ensures the
formatToRelativeTimetest produces consistent, predictable output. The cleanup withjest.useRealTimers()properly prevents timer leakage to other tests.__tests__/Unit/Components/Navbar/Navbar.test.tsx (1)
90-93: LGTM! Events URL updated to match the new endpoint.The test expectation correctly reflects the removal of the
.htmlextension from the Events URL, aligning with the updatedEVENTS_URLconstant insrc/constants/url.ts.src/components/Dropdown/Dropdown.tsx (1)
1-52: LGTM! MY_SITE_URL successfully migrated to MAIN_SITE_URL.All dropdown navigation links now use
MAIN_SITE_URL, which supports environment-aware routing (staging vs. production). The migration is complete and consistent across all external links (Status, Profile, Identity), while the new Tasks link correctly uses an internal route.src/constants/url.ts (2)
5-36: LGTM! Environment-aware URL configuration implemented correctly.The refactoring successfully introduces environment-based URL configuration using
envPrefix, enabling seamless navigation between staging and production environments. The removal ofMY_SITE_URLand introduction ofMAIN_SITE_URLwith new API endpoint constants (ITEMS_URL, ALL_TAGS_URL, etc.) aligns well with the PR objectives.Key improvements:
- All user-facing URLs now support staging environment
- New API endpoint constants follow consistent patterns
- EVENTS_URL path updated from
.../events.htmlto.../events
14-14: Verify GitHub OAuth app configuration for staging redirect URIs.The
STATUS_URLis correctly environment-aware and will resolve tohttps://staging-status.realdevsquad.comin staging andhttps://status.realdevsquad.comin production. However, verify that the GitHub OAuth application has been configured to accept the staging callback URL (https://staging-status.realdevsquad.com) in addition to the production URL. This requires checking the GitHub OAuth app settings, which is not visible in the frontend code.
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.
this change is done because with the year change, the test was expecting "4 years ago"...jest is used to freeze time for previous year
| const spanElement = screen.getByTestId('started-on'); | ||
| expect(spanElement).toHaveTextContent('Not started'); | ||
| }); | ||
|
|
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.
this change is done because with the year change expectation was being changed...jest is used to freeze time for previous year without updating test expectation
Date: 09-01-2026
Developer Name: @MayankBansal12
Issue Ticket Number
Description
(Prod urls were being used for navigation before)
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
demo
Screencast.from.2026-01-10.00-05-58.mp4
Test Coverage
coverage