fix: unarchive option in archived template library #8585
Conversation
jaycnz
added
effort: 2
type: fix
WalkthroughRenames the URL query parameter key from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
View your CI Pipeline Execution ↗ for commit 1587c94
☁️ Nx Cloud last updated this comment at |
github-actions
Bot
requested a deployment
to
Preview - journeys-admin
jaycnz
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.tsx (1)
63-65: Bug: Incomplete migration fromtabtostatusquery parameter.Line 64 still reads from
router?.query?.tabwhile all other references have been updated torouter?.query?.status. This breaks initial tab selection from URL - users navigating to?status=archivedwill see the Active tab instead.🐛 Proposed fix
const tabIndex = - journeyStatusTabs.find((status) => status.queryParam === router?.query?.tab) + journeyStatusTabs.find((status) => status.queryParam === router?.query?.status) ?.tabIndex ?? 0apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.spec.tsx (4)
16-19: Incomplete migration: Default router mock still usestab.The default mock should use
statusto align with the component's updated query parameter. Whileundefinedmay work, it's better to be consistent.🐛 Proposed fix
jest.mock('next/router', () => ({ __esModule: true, - useRouter: jest.fn(() => ({ query: { tab: undefined } })) + useRouter: jest.fn(() => ({ query: { status: undefined } })) }))
94-120: Test will fail: Mock usestabbut component readsstatus.This test verifies URL-based tab selection, but the mock still uses
tab: 'archived'while the component now reads fromrouter?.query?.status. The test will pass incorrectly (defaulting to Active tab) instead of verifying the Archived tab is selected.🐛 Proposed fix
it('should set active tab based on url query params', () => { mockedUseRouter.mockReturnValue({ query: { - tab: 'archived' + status: 'archived' } } as unknown as NextRouter)
122-154: Test mock inconsistency: Usestabinstead ofstatus.The mock at line 127 uses
tab: 'archived'but should usestatus: 'archived'for consistency with the component's updated query parameter reading.🐛 Proposed fix
mockedUseRouter.mockReturnValue({ query: { - tab: 'archived' + status: 'archived' }, push } as unknown as NextRouter)
156-190: Test mock inconsistency: Usestabinstead ofstatus.The mock at line 161 uses
tab: 'active'but should usestatus: 'active'for consistency.🐛 Proposed fix
mockedUseRouter.mockReturnValue({ query: { - tab: 'active' + status: 'active' }, push } as unknown as NextRouter)
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.spec.tsxapps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.
Files:
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.tsxapps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.spec.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
Define a type if possible.
Files:
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.tsxapps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.spec.tsx
apps/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/apps.mdc)
apps/**/*.{js,jsx,ts,tsx}: Always use MUI over HTML elements; avoid using CSS or tags.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Files:
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.tsxapps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.spec.tsx
🧠 Learnings (6)
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Co-locate React Testing Library specs under `*.spec.ts(x)` and mock network traffic with MSW handlers in `apps/watch/test`.
Applied to files:
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.spec.tsx
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Enclose SWR-based hooks in `TestSWRConfig` (`apps/watch/test/TestSWRConfig.tsx`) to isolate cache state between assertions in tests.
Applied to files:
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.spec.tsx
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Applies to apps/resources/**/*.spec.ts?(x) : Enclose SWR-based hooks in `TestSWRConfig` (`apps/resources/test/TestSWRConfig.tsx`) to isolate cache state between assertions
Applied to files:
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.spec.tsx
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Applies to apps/resources/**/*.spec.ts?(x) : Co-locate React Testing Library specs under `*.spec.ts(x)` and mock network traffic with MSW handlers in `apps/resources/test`
Applied to files:
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.spec.tsx
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Use the shared Jest setup in `apps/watch/setupTests.tsx` which already boots MSW, Next router mock, and has a longer async timeout.
Applied to files:
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.spec.tsx
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Applies to apps/resources/**/*.spec.ts?(x) : Wrap component specs with `MockedProvider`, `VideoProvider`, and `WatchProvider` when the unit touches those contexts—`NewVideoContentPage.spec.tsx` shows the expected harness
Applied to files:
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.spec.tsx
⏰ 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). (7)
- GitHub Check: Deploy Preview (journeys-admin, 8585/merge, pull_request, 22)
- GitHub Check: build (22)
- GitHub Check: test (22, 2/3)
- GitHub Check: test (22, 1/3)
- GitHub Check: test (22, 3/3)
- GitHub Check: lint (22)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.tsx (2)
89-95: LGTM!The router.push call correctly uses the new
statusquery parameter.
146-171: LGTM!The
unmountUntilVisibleconditions for all three TabPanel instances correctly referencerouter?.query?.status.apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.spec.tsx (1)
192-226: LGTM!This test correctly uses
status: 'active'in the mock and verifies the push call withstatus: 'trashed'.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.tsx (1)
63-65: Bug: Incomplete refactor - still usingtabinstead ofstatus.The initial
tabIndexcalculation still referencesrouter?.query?.tab, butrouter.pushon line 91 now writes tostatus, and theunmountUntilVisiblechecks also usestatus. This causes the initial tab selection to fail when navigating directly to a URL like?status=archived.🐛 Proposed fix
const tabIndex = - journeyStatusTabs.find((status) => status.queryParam === router?.query?.tab) + journeyStatusTabs.find((status) => status.queryParam === router?.query?.status) ?.tabIndex ?? 0
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.spec.tsxapps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.spec.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.
Files:
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
Define a type if possible.
Files:
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.tsx
apps/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/apps.mdc)
apps/**/*.{js,jsx,ts,tsx}: Always use MUI over HTML elements; avoid using CSS or tags.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Files:
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.tsx
⏰ 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). (5)
- GitHub Check: Deploy Preview (journeys-admin, 8585/merge, pull_request, 22)
- GitHub Check: test (22, 2/3)
- GitHub Check: build (22)
- GitHub Check: lint (22)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/journeys-admin/src/components/StatusTabPanel/StatusTabPanel.tsx (2)
89-95: LGTM!The query parameter rename from
tabtostatusin the router push is correct.
150-172: LGTM!The
unmountUntilVisiblelogic is consistent:
- Active panel mounts when
statusis undefined or'active'(default behavior)- Archived/Trashed panels mount only when their respective
statusvalues are setThis correctly implements the status-based tab visibility.
|
The latest updates on your projects.
|
edmonday
changed the title
…in-template-library
jaycnz
deleted the
jaychen/nes-1186-check-for-unarchive-option-in-template-library
branch
2 participants
Summary by CodeRabbit
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.