feat: sort journeys in order #8608
Open
Conversation
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
Contributor
WalkthroughInitializes and syncs JourneyList sortOrder from URL query ( Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant JourneyList
participant Router
participant LocalStorage
participant JourneyListView
Browser->>JourneyList: Mount component (URL query?)
JourneyList->>LocalStorage: read stored sortOrder
alt URL has sortBy
JourneyList->>JourneyList: set sortOrder from URL
else stored value only
JourneyList->>Router: shallow push with sortBy (persist URL)
Router-->>Browser: update URL (shallow)
end
JourneyList->>LocalStorage: persist current sortOrder
JourneyList->>JourneyListView: pass sortOrder and handleSetSortOrder
JourneyListView->>JourneyList: user changes sortOrder (via handler)
JourneyList->>Router: shallow push updated sortBy
JourneyList->>LocalStorage: persist updated sortOrder
Router-->>Browser: update URL (shallow)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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 |
…er-last-selection-and-not-switch
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run-many --target=deploy --projects=journeys... |
❌ Failed | 1m 7s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-01-22 01:24:13 UTC
github-actions
bot
requested a deployment
to
Preview - journeys-admin
Contributor
|
The latest updates on your projects.
|
![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: 1
🤖 Fix all issues with AI agents
In `@apps/journeys-admin/src/components/JourneyList/JourneyList.tsx`:
- Around line 45-65: The current useEffect uses router.query.sortBy with "||"
which allows an invalid value or string[] to override localStorage; change the
logic in the useEffect to first validate router.query.sortBy is a single string
and a member of SortOrder before using it, otherwise fall back to the
localStorage value from localStorage.getItem('journeyListSortBy'); then call
setSortOrder only with a validated SortOrder value, and only write to
localStorage or call router.replace when the validated source (query or storage)
is used; reference router.query.sortBy, SortOrder, setSortOrder,
localStorage.getItem('journeyListSortBy'), and router.replace to locate and
update the logic.
🧹 Nitpick comments (1)
apps/journeys-admin/src/components/JourneyList/JourneyList.tsx (1)
67-83: Avoid mimicking functional-updater semantics if you don’t implement them.
handleSetSortOrderaccepts a functional updater but evaluates it against the render-timesortOrder, which can be stale if multiple updates are queued. IfJourneyListViewnever uses the function form, consider simplifying the signature; otherwise, move the router/localStorage sync to auseEffectdriven bysortOrderand pass the updater through untouched.
Comment on lines
+45
to
+65
| useEffect(() => { | ||
| if (!router.isReady) return | ||
| const sortByFromQuery = router.query.sortBy as string | ||
| const sortByFromStorage = | ||
| typeof window !== 'undefined' | ||
| ? localStorage.getItem('journeyListSortBy') | ||
| : null | ||
| const sortBy = sortByFromQuery || sortByFromStorage | ||
| if (sortBy && Object.values(SortOrder).includes(sortBy as SortOrder)) { | ||
| setSortOrder((prev) => (prev !== sortBy ? (sortBy as SortOrder) : prev)) | ||
| if (sortByFromQuery && typeof window !== 'undefined') { | ||
| localStorage.setItem('journeyListSortBy', sortByFromQuery) | ||
| } else if (sortByFromStorage && !sortByFromQuery) { | ||
| void router.replace( | ||
| { query: { ...router.query, sortBy: sortByFromStorage } }, | ||
| undefined, | ||
| { shallow: true } | ||
| ) | ||
| } | ||
| } | ||
| }, [router.isReady, router.query.sortBy, router]) |
Contributor
There was a problem hiding this comment.
Validate the query param before it overrides stored sort.
sortByFromQuery can be a string[] or an invalid value. Because it’s used with ||, an invalid query string (or array) blocks a valid localStorage fallback, leaving sortOrder unset. Consider validating the query first, and only falling back to storage when the query is absent/invalid.
🛠️ Suggested fix
- const sortByFromQuery = router.query.sortBy as string
+ const rawSortBy = router.query.sortBy
+ const sortByFromQuery = Array.isArray(rawSortBy) ? rawSortBy[0] : rawSortBy
const sortByFromStorage =
typeof window !== 'undefined'
? localStorage.getItem('journeyListSortBy')
: null
- const sortBy = sortByFromQuery || sortByFromStorage
- if (sortBy && Object.values(SortOrder).includes(sortBy as SortOrder)) {
- setSortOrder((prev) => (prev !== sortBy ? (sortBy as SortOrder) : prev))
+ const isValidSort = (value?: string | null): value is SortOrder =>
+ !!value && Object.values(SortOrder).includes(value as SortOrder)
+ const sortBy = isValidSort(sortByFromQuery)
+ ? sortByFromQuery
+ : isValidSort(sortByFromStorage)
+ ? sortByFromStorage
+ : undefined
+ if (sortBy) {
+ setSortOrder((prev) => (prev !== sortBy ? sortBy : prev))
if (sortByFromQuery && typeof window !== 'undefined') {
localStorage.setItem('journeyListSortBy', sortByFromQuery)
} else if (sortByFromStorage && !sortByFromQuery) {🤖 Prompt for AI Agents
In `@apps/journeys-admin/src/components/JourneyList/JourneyList.tsx` around lines
45 - 65, The current useEffect uses router.query.sortBy with "||" which allows
an invalid value or string[] to override localStorage; change the logic in the
useEffect to first validate router.query.sortBy is a single string and a member
of SortOrder before using it, otherwise fall back to the localStorage value from
localStorage.getItem('journeyListSortBy'); then call setSortOrder only with a
validated SortOrder value, and only write to localStorage or call router.replace
when the validated source (query or storage) is used; reference
router.query.sortBy, SortOrder, setSortOrder,
localStorage.getItem('journeyListSortBy'), and router.replace to locate and
update the logic.
…er-last-selection-and-not-switch
…er-last-selection-and-not-switch
…er-last-selection-and-not-switch
…er-last-selection-and-not-switch
github-actions
bot
requested a deployment
to
Preview - journeys-admin
…er-last-selection-and-not-switch
github-actions
bot
had a problem deploying
to
Preview - journeys-admin
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.