refactor(client): dissolve config store into typed query cache and validated prefs - #1297
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis change replaces the Zustand configuration store with query-backed hooks and shared typed preference persistence. Plex user data moves to dedicated query hooks. Configuration schemas and routes are updated, OpenAPI types are regenerated, and CI verifies generated artifacts. ChangesClient state and API migration
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![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: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/components/network-config-credenza.tsx`:
- Line 108: Prevent configuration-derived actions from running when useConfig
returns null. In src/client/components/network-config-credenza.tsx:108, disable
Save and guard handleSave until config exists; in
src/client/components/ui/workflow-status-badge.tsx:35, disable Start until
configuration data exists, rather than proceeding with default values.
In `@src/client/components/ui/user-multi-select.tsx`:
- Around line 37-42: Update UserMultiSelect’s loading state logic to consume the
isLoading value returned by useUserOptions(). Keep isLoading true until both
initialization/minimum delay and the user-options query have completed, so
MultiSelect remains disabled and displays “Loading users...” while the user list
is unavailable.
In `@src/client/features/approvals/index.tsx`:
- Around line 34-35: Update src/client/features/approvals/index.tsx at lines
34-35 to use a single useConfig() call that reads initialize, isInitialized, and
error, then render the existing PageError or retry UI before the loading
skeleton when configuration fails. Update
src/client/features/utilities/pages/new-user-defaults.tsx at line 97 to read
useConfig().error and handle the failure before rendering
NewUserDefaultsPageSkeleton; preserve the existing initialization and
successful-rendering behavior.
In `@src/client/features/approvals/pages/approval-settings.tsx`:
- Line 50: Update useConfig consumers to read and render the configuration error
before loading or missing-config branches. In
src/client/features/approvals/pages/approval-settings.tsx:50-50 and
src/client/features/approvals/pages/quota-settings.tsx:98-98, add the error
state and retry it with initialize(true); in
src/client/features/utilities/pages/api-keys.tsx:31-31, delete-sync.tsx:90-90,
log-viewer.tsx:181-181, plex-labels.tsx:63-64, and user-tags.tsx:55-55, handle
the error before their respective skeleton or config checks so failures expose
an error and retry action instead of remaining in loading state.
In `@src/client/features/plex/hooks/usePlexUsers.ts`:
- Around line 257-283: Update useUsersWithQuota and the user-table row
identity/selection flow so untracked synthetic users use their unique plex_uuid
instead of the shared id: 0. Ensure tracked and untracked rows consistently use
plex_uuid for row keys and selection, while preserving the existing isTracked
restriction and actions behavior.
- Around line 79-81: Update useUserList in
src/client/features/plex/hooks/usePlexUsers.ts to expose the query’s loading and
error state instead of representing failed or loading results only as null;
align src/client/hooks/useUserOptions.ts with that query-state contract. In
src/client/features/utilities/pages/watchlist-exclusions.tsx, use the user
query’s isError and refetch state to render and recover from failures rather
than leaving WatchlistExclusionsPage on the skeleton until users load
successfully.
In `@src/client/features/plex/pages/users.tsx`:
- Around line 114-115: Update the users page readiness logic around useConfig
and useUsersWithQuota to retain the user query’s isLoading state, combine it
with isInitialized before rendering UserTable, and prevent rendering while
usersWithQuota is null. Capture both configuration and user-query errors and
render the page’s existing error or retry state instead of remaining
indefinitely in loading.
- Line 258: Update the empty-state condition in the users page render to use
usersWithQuota.length === 0 when usersWithQuota is non-null, replacing the
hasUserData check. Preserve the existing loading and null-result guards so
enriched untracked rows keep the table and friendship flow visible.
In `@src/client/features/utilities/hooks/usePlexLabels.ts`:
- Line 150: Update the initialization effect around useConfig and
updateFormValues so configuration hydration is independent of the full-sync
schedule fields. Track query-loaded/error state separately, and hydrate whenever
config.plexLabelSync becomes available, including unscheduled configurations
with undefined scheduleTime and wildcard dayOfWeek; preserve the fallback
behavior for failed or unavailable queries.
- Line 23: Update usePlexNotifications in
src/client/features/utilities/hooks/usePlexNotifications.ts to expose and invoke
invalidateConfig after successful configure-notifications and
remove-notifications apiFetch operations, alongside notification-status
refreshes. Update src/client/features/utilities/hooks/usePlexLabels.ts at lines
23-23, src/client/features/notifications/components/apprise/apprise-form.tsx at
lines 26-26, and src/client/features/utilities/pages/new-user-defaults.tsx at
lines 35-36 to consume the updated hook or invalidation behavior; no direct
change is required there if the shared hook fix supplies the config refresh.
In `@src/client/features/utilities/hooks/useUserTags.ts`:
- Line 152: Update useUserTags around useConfig and the returned error state to
surface configError alongside other errors, and use that failure state to block
form submission while configuration is unavailable. Preserve writable behavior
once configuration loads successfully or the user retries and clears the error.
In `@src/client/hooks/use-table-pagination.ts`:
- Around line 28-36: Update the setPageSize callback to accept only positive
integer values from 1 through 100, rejecting fractional or otherwise invalid
page sizes before calling setPageSizePref. Preserve the existing warning
behavior for rejected values.
In `@src/client/lib/prefs.ts`:
- Around line 43-51: Update usePref so the memoized preference definition
changes reload the hook state: add an effect keyed by def that reads the current
preference via readPref(def) and updates setValue. Keep the existing setter
behavior unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ce9ed4d-f88a-40f7-8748-e5fddf77280b
⛔ Files ignored due to path filters (1)
docs/static/openapi.jsonis excluded by!docs/**
📒 Files selected for processing (86)
.github/workflows/ci.ymlsrc/client/components/AppSidebar.tsxsrc/client/components/network-config-credenza.tsxsrc/client/components/settings-provider.tsxsrc/client/components/theme-provider.tsxsrc/client/components/tmdb-content-viewer.tsxsrc/client/components/tmdb-metadata-display.tsxsrc/client/components/ui/apprise-status-badge.tsxsrc/client/components/ui/sidebar.tsxsrc/client/components/ui/streaming-provider-multi-select.tsxsrc/client/components/ui/tmdb-region-selector.tsxsrc/client/components/ui/user-multi-select.tsxsrc/client/components/ui/workflow-status-badge.tsxsrc/client/features/approvals/components/approval-actions-modal.tsxsrc/client/features/approvals/components/approval-sonarr-routing-card.tsxsrc/client/features/approvals/index.tsxsrc/client/features/approvals/pages/approval-settings.tsxsrc/client/features/approvals/pages/quota-settings.tsxsrc/client/features/approvals/store/approvalsStore.tssrc/client/features/content-router/components/accordion-route-card.tsxsrc/client/features/content-router/components/condition-input.tsxsrc/client/features/dashboard/hooks/useDashboardStats.tssrc/client/features/dashboard/hooks/useMediaViewMode.tssrc/client/features/dashboard/index.tsxsrc/client/features/notifications/components/apprise/apprise-form.tsxsrc/client/features/notifications/components/discord/discord-bot-form.tsxsrc/client/features/notifications/components/discord/discord-webhook-form.tsxsrc/client/features/notifications/components/general/general-settings-form.tsxsrc/client/features/notifications/components/plex-mobile/plex-mobile-form.tsxsrc/client/features/notifications/hooks/useNotificationsConfig.tssrc/client/features/notifications/hooks/usePublicContentNotifications.tssrc/client/features/plex/components/setup/setup-modal.tsxsrc/client/features/plex/components/user/quota-edit-modal.tsxsrc/client/features/plex/components/user/quota-status-badge.tsxsrc/client/features/plex/components/user/user-edit-modal.tsxsrc/client/features/plex/components/user/user-table.tsxsrc/client/features/plex/components/user/user-watchlist-sheet.tsxsrc/client/features/plex/hooks/useApprovalConfiguration.tssrc/client/features/plex/hooks/useBulkQuotaManagement.tssrc/client/features/plex/hooks/usePlexBulkUpdate.tssrc/client/features/plex/hooks/usePlexConnection.tssrc/client/features/plex/hooks/usePlexExistenceCheck.tssrc/client/features/plex/hooks/usePlexRssFeeds.tssrc/client/features/plex/hooks/usePlexSetup.tssrc/client/features/plex/hooks/usePlexUser.tssrc/client/features/plex/hooks/usePlexUsers.tssrc/client/features/plex/hooks/usePlexWatchlist.tssrc/client/features/plex/hooks/useQuotaManagement.tssrc/client/features/plex/pages/configuration.tsxsrc/client/features/plex/pages/users.tsxsrc/client/features/plex/store/types.tssrc/client/features/radarr/pages/radarr-content-router.tsxsrc/client/features/sonarr/components/instance/sonarr-instance-card.tsxsrc/client/features/sonarr/pages/sonarr-content-router.tsxsrc/client/features/sonarr/pages/sonarr-instances.tsxsrc/client/features/utilities/components/session-monitoring/manage-rolling-sheet.tsxsrc/client/features/utilities/components/session-monitoring/rolling-shows-sheet.tsxsrc/client/features/utilities/hooks/useApiKeys.tssrc/client/features/utilities/hooks/useDeleteSyncForm.tssrc/client/features/utilities/hooks/usePlexLabels.tssrc/client/features/utilities/hooks/usePlexNotifications.tssrc/client/features/utilities/hooks/useSchedules.tssrc/client/features/utilities/hooks/useSessionMonitoring.tssrc/client/features/utilities/hooks/useUserTags.tssrc/client/features/utilities/pages/api-keys.tsxsrc/client/features/utilities/pages/delete-sync.tsxsrc/client/features/utilities/pages/log-viewer.tsxsrc/client/features/utilities/pages/new-user-defaults.tsxsrc/client/features/utilities/pages/plex-labels.tsxsrc/client/features/utilities/pages/plex-notifications.tsxsrc/client/features/utilities/pages/plex-session-monitoring.tsxsrc/client/features/utilities/pages/user-tags.tsxsrc/client/features/utilities/pages/watchlist-exclusions.tsxsrc/client/hooks/use-table-pagination.tssrc/client/hooks/useConfig.tssrc/client/hooks/useCurrentUser.tssrc/client/hooks/useUserOptions.tssrc/client/hooks/useVersionCheck.tsxsrc/client/lib/prefs.tssrc/client/stores/configStore.tssrc/client/types/api.d.tssrc/routes/v1/config/config.tssrc/schemas/config/config.schema.tssrc/schemas/plex/label-sync-config.schema.tssrc/services/database/methods/config.tssrc/services/plex-label-sync.service.ts
💤 Files with no reviewable changes (4)
- src/services/plex-label-sync.service.ts
- src/client/stores/configStore.ts
- src/schemas/plex/label-sync-config.schema.ts
- src/services/database/methods/config.ts
Comment on lines
+79
to
+81
| export function useUserList(): UserWatchlistInfo[] | null { | ||
| const { data } = useUsers() | ||
| return data?.users ?? null |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the query wrapper and affected hooks before tracing state propagation.
ast-grep outline src/client/lib/tanstackApi.ts --items all
ast-grep outline src/client/features/plex/hooks/usePlexUsers.ts --items all
ast-grep outline src/client/hooks/useUserOptions.ts --items all
ast-grep outline src/client/features/utilities/pages/watchlist-exclusions.tsx --items all
# Verify whether an initial query error can leave data undefined, then trace
# whether callers distinguish isLoading from isError.
rg -n -C 8 '\$api\.useQuery|\buseUsers\b|\buseUserList\b|\bisLoading\b|\bisError\b|\berror\b|\bhasLoadedWatchlists\b' \
src/client/lib/tanstackApi.ts \
src/client/features/plex/hooks/usePlexUsers.ts \
src/client/hooks/useUserOptions.ts \
src/client/features/utilities/pages/watchlist-exclusions.tsxRepository: jamcalli/Pulsarr
Length of output: 36507
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- affected hook declarations ---\n'
rg -n -C 12 'export function useCreateWatchlistExclusion|export function useRemoveWatchlistExclusion|export function fetchAllWatchlistItems|export function useWatchlistExclusions|useQuery\(' \
src/client/features/utilities src/client/features \
| sed -n '1,260p'
printf '\n--- error/isError usage near watchlist exclusions ---\n'
rg -n -C 3 '\bisError\b|error|toast\.error|Failed to refresh|handleRefresh|WatchlistExclusionsSkeleton' \
src/client/features/utilities/pages/watchlist-exclusions.tsx \
src/client/features/utilities/hooks src/client/features/utilities/components
printf '\n--- dependency versions for openapi-react-query and react-query ---\n'
if [ -f package.json ]; then
node - <<'JS'
const fs = require('fs')
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'))
for (const section of ['dependencies','devDependencies','peerDependencies']) {
console.log(section)
for (const name of ['openapi-react-query','`@tanstack/react-query`','`@tanstack/react-query-devtools`']) {
if (pkg[section] && pkg[section][name]) console.log(` ${name}: ${pkg[section][name]}`)
}
}
JS
fi
if [ -f package-lock.json ]; then
node - <<'JS'
const fs = require('fs')
const lock = JSON.parse(fs.readFileSync('package-lock.json', 'utf8'))
for (const name of ['openapi-react-query','`@tanstack/react-query`']) {
const node = lock.packages && lock.packages['node_modules/'+name]
console.log(name, node ? node.version : 'missing')
}
JS
fi
printf '\n--- read-only semantic probe for useUserList derived state patterns ---\n'
python3 - <<'PY'
from pathlib import Path
import re
paths = [
'src/client/features/plex/hooks/usePlexUsers.ts',
'src/client/hooks/useUserOptions.ts',
'src/client/features/utilities/pages/watchlist-exclusions.tsx',
]
for path in paths:
text = Path(path).read_text()
print(f'--- {path} ---')
for i,line in enumerate(text.splitlines(), 1):
if any(s in line for s in ['useUsers()', 'useUserList()', 'data?'])':
pass
if any(s in line for s in ['return data?.users', 'users === null', 'users !== null', 'hasLoadedWatchlists', 'isLoading:', 'isLoading: usersQuery.isLoading']):
print(f'{i}: {line.strip()}')
PYRepository: jamcalli/Pulsarr
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- inspected watchlist exclusion hook definitions ---\n'
for f in $(git ls-files 'src/client/features/utilities/hooks/*' 'src/client/features/utilities/pages/watchlist-exclusions.tsx'); do
if rg -q 'useQuery|fetch|exclusion' "$f"; then
echo "### $f"
sed -n '1,240p' "$f"
fi
done
printf '\n--- all uses of error/isError in client query result destructuring ---\n'
rg -n -C 3 'data:\s*\{[^}]*error|from \{\s*data[^}]*error|\bisError\b|isLoading' src/client/features src/client/hooks | sed -n '1,260p'Repository: jamcalli/Pulsarr
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' "--- lock/package references for openapi-react-query and `@tanstack/react-query` ---"
rg -n '"openapi-react-query"|"`@tanstack/react-query`"|openapi-react-query|`@tanstack/react-query`' package.json package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || true
printf '%s\n' "--- imports of min-loading hook helpers ---"
rg -n "useMinLoading|MIN_LOADING|useQuery\\(|\\$(\\.|\\w+)useQuery\\(" src/client/features/plex/hooks/usePlexUsers.ts src/client/features/utilities/hooks/useWatchlistExclusions.ts src/client/features/utilities/pages/watchlist-exclusions.tsx src/client/lib src/client/features/utilities/components/watchlist-exclusions | sed -n '1,120p'Repository: jamcalli/Pulsarr
Length of output: 626
🌐 Web query:
openapi-react-query useQuery return data error isLoading status Tanstack React Query
💡 Result:
The useQuery hook from TanStack React Query (v5 and later) returns an object containing several properties to manage and reflect the state of your asynchronous data fetching [1][2]. Because libraries like openapi-react-query are thin wrappers around TanStack Query, they expose these same underlying result properties [3][4]. Key return values from useQuery include: data: The last successfully resolved data for the query [1][5]. Defaults to undefined [1][2]. error: The error object, if the query failed [1][5]. Defaults to null [1][2]. status: A string representing the query's state: pending, error, or success [6][7]. isPending: A boolean that is true when the query has no data yet (previously known as isLoading in older versions) [6][7]. isError: A boolean that is true if the query encountered an error [6][7]. isSuccess: A boolean that is true if the query was successful and data is available [6][7]. isFetching: A boolean that is true whenever a fetch is in-flight (including background refetching) [6][7]. In TanStack Query v5, the state formerly referred to as isLoading (which was true if the query was fetching and had no data) is primarily represented by isPending [1][2]. While isLoading may still exist in some older wrappers or definitions, you should prioritize using the status strings or the specific boolean flags like isPending, isError, and isSuccess to manage your UI states [6][7]. Example usage: const { data, error, isPending, isError } = useQuery({ queryKey: ['myKey'], queryFn: myFetchFunction, }); if (isPending) return
Loading...
; if (isError) return Error: {error.message}
; return {data.name}
; When using libraries like openapi-react-query, the generated hooks will return these same properties, allowing you to handle loading, error, and success states identically to standard TanStack Query hooks [3][4].
Citations:
- 1: https://tanstack.com/query/latest/docs/framework/react/reference/useQuery
- 2: https://tanstack.com/query/v5/docs/framework/react/reference/useQuery
- 3: https://www.npmjs.com/package/openapi-react-query
- 4: https://openapi-ts.dev/openapi-react-query/
- 5: https://github.com/tanstack/query/blob/main/docs/framework/react/reference/useQuery.md
- 6: https://tanstack.com/query/v5/docs/framework/react/guides/queries
- 7: https://tanstack.com/query/latest/docs/framework/react/guides/queries.md
Separate user-query failure from user loading.
$api.useQuery() can return data === undefined when the initial user query fails, so useUserList() maps that failure to null. Treat the user query result with React Query error/loading state instead of using only users === null; WatchlistExclusionsPage then needs isError/refetch for this query instead of staying on the skeleton until a successful user list arrives.
📍 Affects 3 files
src/client/features/plex/hooks/usePlexUsers.ts#L79-L81(this comment)src/client/hooks/useUserOptions.ts#L31-L31src/client/features/utilities/pages/watchlist-exclusions.tsx#L130-L135
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/client/features/plex/hooks/usePlexUsers.ts` around lines 79 - 81, Update
useUserList in src/client/features/plex/hooks/usePlexUsers.ts to expose the
query’s loading and error state instead of representing failed or loading
results only as null; align src/client/hooks/useUserOptions.ts with that
query-state contract. In
src/client/features/utilities/pages/watchlist-exclusions.tsx, use the user
query’s isError and refetch state to render and recover from failures rather
than leaving WatchlistExclusionsPage on the skeleton until users load
successfully.
| useScheduleActions, | ||
| useSchedules, | ||
| } from '@/features/utilities/hooks/useSchedules' | ||
| import { updateConfig, useConfig } from '@/hooks/useConfig' |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 \
'export (async )?function updateConfig|const updateConfig|invalidateConfig|queryClient\.(invalidateQueries|setQueryData)|configure-notifications|remove-notifications' \
.Repository: jamcalli/Pulsarr
Length of output: 47728
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the hooks and mutation code for the cited direct config changes.
for f in \
src/client/features/utilities/hooks/usePlexLabels.ts \
src/client/features/notifications/components/apprise/apprise-form.tsx \
src/client/features/utilities/pages/new-user-defaults.tsx \
src/client/features/utilities/hooks/usePlexNotifications.ts \
src/client/hooks/useConfig.ts
do
echo "===== $f ====="
wc -l "$f"
sed -n '1,260p' "$f"
done
echo "===== usePlexNotifications relevant section ====="
sed -n '2,250p' src/client/features/utilities/hooks/usePlexNotifications.ts
echo "===== apprise form imports/call sites around updateConfig ====="
rg -n -C 20 'updateConfig|resetForm|formHandler|handleSubmit' src/client/features/notifications/components/apprise/apprise-form.tsx
echo "===== new-user-defaults imports/call sites around validation/save ====="
rg -n -C 25 'updateConfig|handleSubmit|handleSave|save|success|invalid' src/client/features/utilities/pages/new-user-defaults.tsxRepository: jamcalli/Pulsarr
Length of output: 50372
Refresh configKeys.config after Plex notification mutations.
updateConfig refreshes configKeys.config, but usePlexNotifications calls configure-notifications and remove-notifications through direct apiFetch endpoints. Successful configure/remove operations refresh the notification status, but they do not refresh the shared config query, so sibling consumers can remain stale. Add invalidateConfig() for both operations.
📍 Affects 4 files
src/client/features/utilities/hooks/usePlexLabels.ts#L23-L23(this comment)src/client/features/notifications/components/apprise/apprise-form.tsx#L26-L26src/client/features/utilities/pages/new-user-defaults.tsx#L35-L36src/client/features/utilities/hooks/usePlexNotifications.ts#L11-L13
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/client/features/utilities/hooks/usePlexLabels.ts` at line 23, Update
usePlexNotifications in
src/client/features/utilities/hooks/usePlexNotifications.ts to expose and invoke
invalidateConfig after successful configure-notifications and
remove-notifications apiFetch operations, alongside notification-status
refreshes. Update src/client/features/utilities/hooks/usePlexLabels.ts at lines
23-23, src/client/features/notifications/components/apprise/apprise-form.tsx at
lines 26-26, and src/client/features/utilities/pages/new-user-defaults.tsx at
lines 35-36 to consume the updated hook or invalidation behavior; no direct
change is required there if the shared hook fix supplies the config refresh.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/components/ui/user-multi-select.tsx`:
- Line 25: Update useUserOptions and its useUserList integration to propagate
the user-query error and retry action instead of representing failed or missing
data as loading/null. In the component using useUserOptions, distinguish the
completed error state from isLoading, render an error message with the retry
action, and keep the select disabled only while the query is actually loading.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 26c355cd-f04d-4702-af6e-4687140b09cb
📒 Files selected for processing (32)
src/client/components/network-config-credenza.tsxsrc/client/components/ui/user-multi-select.tsxsrc/client/components/ui/workflow-status-badge.tsxsrc/client/features/approvals/index.tsxsrc/client/features/approvals/pages/approval-settings.tsxsrc/client/features/approvals/pages/quota-settings.tsxsrc/client/features/content-router/components/accordion-content-router-section.tsxsrc/client/features/dashboard/hooks/useChartData.tssrc/client/features/dashboard/index.tsxsrc/client/features/notifications/hooks/useNotificationsConfig.tssrc/client/features/notifications/index.tsxsrc/client/features/plex/pages/configuration.tsxsrc/client/features/plex/pages/users.tsxsrc/client/features/radarr/pages/radarr-content-router.tsxsrc/client/features/radarr/pages/radarr-instances.tsxsrc/client/features/sonarr/pages/sonarr-content-router.tsxsrc/client/features/sonarr/pages/sonarr-instances.tsxsrc/client/features/utilities/components/api-keys/api-keys-delete-confirmation-modal.tsxsrc/client/features/utilities/hooks/useApiKeys.tssrc/client/features/utilities/hooks/usePlexLabels.tssrc/client/features/utilities/hooks/useSchedules.tssrc/client/features/utilities/hooks/useUserTags.tssrc/client/features/utilities/pages/api-keys.tsxsrc/client/features/utilities/pages/delete-sync.tsxsrc/client/features/utilities/pages/log-viewer.tsxsrc/client/features/utilities/pages/new-user-defaults.tsxsrc/client/features/utilities/pages/plex-labels.tsxsrc/client/features/utilities/pages/plex-session-monitoring.tsxsrc/client/features/utilities/pages/user-tags.tsxsrc/client/features/utilities/pages/watchlist-exclusions.tsxsrc/client/hooks/use-table-pagination.tssrc/client/hooks/useConfig.ts
💤 Files with no reviewable changes (1)
- src/client/features/notifications/hooks/useNotificationsConfig.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- src/client/features/approvals/pages/approval-settings.tsx
- src/client/features/sonarr/pages/sonarr-instances.tsx
- src/client/features/radarr/pages/radarr-content-router.tsx
- src/client/features/utilities/pages/log-viewer.tsx
- src/client/features/sonarr/pages/sonarr-content-router.tsx
- src/client/features/utilities/pages/new-user-defaults.tsx
- src/client/hooks/use-table-pagination.ts
- src/client/features/utilities/hooks/useSchedules.ts
- src/client/components/network-config-credenza.tsx
- src/client/features/utilities/hooks/useApiKeys.ts
- src/client/features/utilities/hooks/usePlexLabels.ts
Owner
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/components/ui/user-multi-select.tsx`:
- Around line 59-68: Update the retry Button in UserMultiSelect’s usersError
branch to pass through the component’s disabled prop, preventing refetch when
the field is disabled.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ca43c86-c80b-40df-80ae-f7e14ad7b079
📒 Files selected for processing (2)
src/client/components/ui/user-multi-select.tsxsrc/client/hooks/useUserOptions.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/client/components/theme-provider.tsx (1)
107-115: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInline
PrefDefliterals are restated for each read and write.readPrefandwritePrefboth take a fullPrefDef, so every read/write pair repeats the key, the parser, and the serializer. The write-path literal also carries afallbackthatwritePrefnever reads, which lets the two literals drift apart for the same storage key. Declare one def per preference and pass it to both calls.
src/client/components/theme-provider.tsx#L107-L115: replace this literal and the one at lines 43-48 with a singleReact.useMemo<PrefDef<Theme | null>>def keyed onstorageKey, then callreadPref(themePref)andwritePref(themePref, newTheme).src/client/components/ui/sidebar.tsx#L86-L93: replace this literal with aReact.useMemo<PrefDef<boolean>>def keyed ondefaultOpenand pass it toreadPref.src/client/components/ui/sidebar.tsx#L104-L112: pass the same memoized def towritePref, and add it to thesetOpendependency array.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/components/theme-provider.tsx` around lines 107 - 115, The preference definitions are duplicated across read and write paths, allowing them to drift and carrying unused fallbacks. In src/client/components/theme-provider.tsx#L107-L115 and the corresponding definition at lines 43-48, create one React.useMemo<PrefDef<Theme | null>> keyed on storageKey, then pass it to readPref and writePref; in src/client/components/ui/sidebar.tsx#L86-L93, create one React.useMemo<PrefDef<boolean>> keyed on defaultOpen and reuse it for readPref at lines 86-93 and writePref at lines 104-112, adding the memoized definition to setOpen’s dependency array.src/client/lib/prefs.ts (1)
57-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for the parsers.
parseBoolean,parseOneOf,parseIntInRange, andparseBooleanRecordare pure functions, and they now guard persisted state for the theme provider, the sidebar, the settings provider, the dashboard view mode, table pagination, and the approvals store. Tests would pin the fallback contract, in particular that a non-boolean value rejects the whole record inparseBooleanRecordand that out-of-range values returnundefinedinparseIntInRange.Do you want me to generate the test file?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/lib/prefs.ts` around lines 57 - 98, Add unit tests for parseBoolean, parseOneOf, parseIntInRange, and parseBooleanRecord. Cover valid and invalid inputs, including whole-record rejection when parseBooleanRecord encounters any non-boolean value and undefined results for parseIntInRange values outside the inclusive range.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/features/plex/hooks/usePlexUsers.ts`:
- Around line 214-228: Update useUsersWithQuota and the related quota-loading
logic in src/client/features/plex/hooks/usePlexUsers.ts (lines 214-228 and
295-299) and src/client/features/plex/hooks/usePlexUser.ts (lines 4-31) so
quota-dependent data remains unavailable until the user, quota configuration,
and quota status queries are all ready. Derive isLoading from the combined query
states and do not expose userQuotas as null alongside hasUserData: true while
quota queries are pending.
In `@src/client/features/utilities/hooks/useApiKeys.ts`:
- Around line 33-34: Update the revoke success handling in initiateRevoke so it
clears showDeleteConfirmation only when the current confirmation still matches
the completed key’s id, preventing one key’s success from closing another key’s
modal. Preserve independent per-key revoke state and existing behavior for the
completed key.
---
Nitpick comments:
In `@src/client/components/theme-provider.tsx`:
- Around line 107-115: The preference definitions are duplicated across read and
write paths, allowing them to drift and carrying unused fallbacks. In
src/client/components/theme-provider.tsx#L107-L115 and the corresponding
definition at lines 43-48, create one React.useMemo<PrefDef<Theme | null>> keyed
on storageKey, then pass it to readPref and writePref; in
src/client/components/ui/sidebar.tsx#L86-L93, create one
React.useMemo<PrefDef<boolean>> keyed on defaultOpen and reuse it for readPref
at lines 86-93 and writePref at lines 104-112, adding the memoized definition to
setOpen’s dependency array.
In `@src/client/lib/prefs.ts`:
- Around line 57-98: Add unit tests for parseBoolean, parseOneOf,
parseIntInRange, and parseBooleanRecord. Cover valid and invalid inputs,
including whole-record rejection when parseBooleanRecord encounters any
non-boolean value and undefined results for parseIntInRange values outside the
inclusive range.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 32766559-6335-4984-95aa-882abcc973a7
⛔ Files ignored due to path filters (1)
docs/static/openapi.jsonis excluded by!docs/**
📒 Files selected for processing (92)
.github/workflows/ci.ymlsrc/client/components/AppSidebar.tsxsrc/client/components/network-config-credenza.tsxsrc/client/components/settings-provider.tsxsrc/client/components/theme-provider.tsxsrc/client/components/tmdb-content-viewer.tsxsrc/client/components/tmdb-metadata-display.tsxsrc/client/components/ui/apprise-status-badge.tsxsrc/client/components/ui/sidebar.tsxsrc/client/components/ui/streaming-provider-multi-select.tsxsrc/client/components/ui/tmdb-region-selector.tsxsrc/client/components/ui/user-multi-select.tsxsrc/client/components/ui/workflow-status-badge.tsxsrc/client/features/approvals/components/approval-actions-modal.tsxsrc/client/features/approvals/components/approval-sonarr-routing-card.tsxsrc/client/features/approvals/index.tsxsrc/client/features/approvals/pages/approval-settings.tsxsrc/client/features/approvals/pages/quota-settings.tsxsrc/client/features/approvals/store/approvalsStore.tssrc/client/features/content-router/components/accordion-content-router-section.tsxsrc/client/features/content-router/components/accordion-route-card.tsxsrc/client/features/content-router/components/condition-input.tsxsrc/client/features/dashboard/hooks/useChartData.tssrc/client/features/dashboard/hooks/useDashboardStats.tssrc/client/features/dashboard/hooks/useMediaViewMode.tssrc/client/features/dashboard/index.tsxsrc/client/features/notifications/components/apprise/apprise-form.tsxsrc/client/features/notifications/components/discord/discord-bot-form.tsxsrc/client/features/notifications/components/discord/discord-webhook-form.tsxsrc/client/features/notifications/components/general/general-settings-form.tsxsrc/client/features/notifications/components/plex-mobile/plex-mobile-form.tsxsrc/client/features/notifications/hooks/useNotificationsConfig.tssrc/client/features/notifications/hooks/usePublicContentNotifications.tssrc/client/features/notifications/index.tsxsrc/client/features/plex/components/setup/setup-modal.tsxsrc/client/features/plex/components/user/quota-edit-modal.tsxsrc/client/features/plex/components/user/quota-status-badge.tsxsrc/client/features/plex/components/user/user-edit-modal.tsxsrc/client/features/plex/components/user/user-table.tsxsrc/client/features/plex/components/user/user-watchlist-sheet.tsxsrc/client/features/plex/hooks/useApprovalConfiguration.tssrc/client/features/plex/hooks/useBulkQuotaManagement.tssrc/client/features/plex/hooks/usePlexBulkUpdate.tssrc/client/features/plex/hooks/usePlexConnection.tssrc/client/features/plex/hooks/usePlexExistenceCheck.tssrc/client/features/plex/hooks/usePlexRssFeeds.tssrc/client/features/plex/hooks/usePlexSetup.tssrc/client/features/plex/hooks/usePlexUser.tssrc/client/features/plex/hooks/usePlexUsers.tssrc/client/features/plex/hooks/usePlexWatchlist.tssrc/client/features/plex/hooks/useQuotaManagement.tssrc/client/features/plex/pages/configuration.tsxsrc/client/features/plex/pages/users.tsxsrc/client/features/plex/store/types.tssrc/client/features/radarr/pages/radarr-content-router.tsxsrc/client/features/radarr/pages/radarr-instances.tsxsrc/client/features/sonarr/components/instance/sonarr-instance-card.tsxsrc/client/features/sonarr/pages/sonarr-content-router.tsxsrc/client/features/sonarr/pages/sonarr-instances.tsxsrc/client/features/utilities/components/api-keys/api-keys-delete-confirmation-modal.tsxsrc/client/features/utilities/components/session-monitoring/manage-rolling-sheet.tsxsrc/client/features/utilities/components/session-monitoring/rolling-shows-sheet.tsxsrc/client/features/utilities/hooks/useApiKeys.tssrc/client/features/utilities/hooks/useDeleteSyncForm.tssrc/client/features/utilities/hooks/usePlexLabels.tssrc/client/features/utilities/hooks/usePlexNotifications.tssrc/client/features/utilities/hooks/useSchedules.tssrc/client/features/utilities/hooks/useSessionMonitoring.tssrc/client/features/utilities/hooks/useUserTags.tssrc/client/features/utilities/pages/api-keys.tsxsrc/client/features/utilities/pages/delete-sync.tsxsrc/client/features/utilities/pages/log-viewer.tsxsrc/client/features/utilities/pages/new-user-defaults.tsxsrc/client/features/utilities/pages/plex-labels.tsxsrc/client/features/utilities/pages/plex-notifications.tsxsrc/client/features/utilities/pages/plex-session-monitoring.tsxsrc/client/features/utilities/pages/user-tags.tsxsrc/client/features/utilities/pages/watchlist-exclusions.tsxsrc/client/hooks/use-table-pagination.tssrc/client/hooks/useConfig.tssrc/client/hooks/useCurrentUser.tssrc/client/hooks/useUserOptions.tssrc/client/hooks/useVersionCheck.tsxsrc/client/lib/prefs.tssrc/client/stores/configStore.tssrc/client/types/api.d.tssrc/plugins/external/swagger.tssrc/routes/v1/config/config.tssrc/schemas/config/config.schema.tssrc/schemas/plex/label-sync-config.schema.tssrc/services/database/methods/config.tssrc/services/plex-label-sync.service.ts
💤 Files with no reviewable changes (5)
- src/services/plex-label-sync.service.ts
- src/services/database/methods/config.ts
- src/client/stores/configStore.ts
- src/schemas/plex/label-sync-config.schema.ts
- src/client/features/notifications/hooks/useNotificationsConfig.ts
Comment on lines
+214
to
+228
| const enriched: UserWithQuotaInfo[] = users.map((user) => { | ||
| const userQuotaConfig = quotaConfigsByUser.get(user.id) | ||
| const userQuotas: UserQuotas | null = userQuotaConfig | ||
| ? { | ||
| userId: user.id, | ||
| movieQuota: mergeStatus( | ||
| userQuotaConfig.movieQuota, | ||
| movieStatuses.get(user.id), | ||
| ), | ||
| showQuota: mergeStatus( | ||
| userQuotaConfig.showQuota, | ||
| showStatuses.get(user.id), | ||
| ), | ||
| } | ||
| : null |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/client/features/plex/pages/users.tsx --items all
rg -n -C 8 \
'\busePlexUser\s*\(|\buseUsersWithQuota\s*\(|\buserQuotas\b|\bsaveQuota\b|\bisLoading\b|Quota' \
src/client/features/plex/pages/users.tsx \
src/client/features/plex/components/user \
src/client/features/plex/hooks/usePlexUser.ts \
src/client/features/plex/hooks/usePlexUsers.ts \
src/client/features/plex/hooks/useQuotaManagement.tsRepository: jamcalli/Pulsarr
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== usePlexUser.ts =="
sed -n '1,130p' src/client/features/plex/hooks/usePlexUser.ts
echo
echo "== usePlexUsers.ts relevant sections =="
sed -n '1,240p' src/client/features/plex/hooks/usePlexUsers.ts
sed -n '240,340p' src/client/features/plex/hooks/usePlexUsers.ts
echo
echo "== users.tsx quota handling sections =="
sed -n '110,180p' src/client/features/plex/pages/users.tsx
sed -n '228,270p' src/client/features/plex/pages/users.tsx
echo
echo "== quota UI opening / table references =="
rg -n -C 5 '\bonEditQuota|QuotaEditModal|onBulkEditQuotas|BulkQuotaEditModal|onEditQuota\s*=' src/client/features/plex src/client/features/plex/components -g '*.tsx' -g '*.ts'Repository: jamcalli/Pulsarr
Length of output: 48017
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const usersQuery = { data: { users: [{ id: 1, name: 'Test' }] }, isLoading: false, isError: false }
const quotaConfigQuery = { data: undefined, isLoading: false, isError: false }
const movieStatusQuery = { data: undefined, isLoading: true, isError: false }
const showStatusQuery = { data: undefined, isLoading: false, isError: false }
const userStatusQuery = { data: { users: [{ uuid: 'uuid', status: 'friend' }] }, isLoading: false, isError: false }
const usersWithQuota =
usersQuery.data.users.map((user) => {
const userQuotaConfig = quotaConfigQuery.data?.userQuotas?.find((config) => config.userId === user.id)
const movieStatuses = new Map(movieStatusQuery.data?.quotaStatuses?.filter((item) => item.quotaStatus).map((item) => [item.userId, item.quotaStatus]) ?? [])
const showStatuses = new Map(showStatusQuery.data?.quotaStatuses?.filter((item) => item.quotaStatus).map((item) => [item.userId, item.quotaStatus]) ?? [])
const userQuotas = userQuotaConfig
? {
userId: user.id,
movieQuota: userQuotaConfig.movieQuota || movieStatuses.get(user.id),
showQuota: userQuotaConfig.showQuota || showStatuses.get(user.id),
}
: null
return {
...user,
userQuotas,
friendStatus: userStatusQuery.data.users.find((info) => info.uuid === user.plex_uuid || info.uuid === 'uuid')?.status ?? 'friend_only',
}
})
console.log({
usersWithQuota,
hasUserData: (usersQuery.data.users.length ?? 0) > 0,
isLoading: usersQuery.isLoading,
pageRendering: true,
tableUsers: usersWithQuota || [],
onEditQuota: true,
})
const existing = usersWithQuota[0].userQuotas && (usersWithQuota[0].userQuotas.movieQuota || usersWithQuota[0].userQuotas.showQuota)
const remove = usersWithQuota[0].userQuotas && usersWithQuota[0].userQuotas.movieQuota && false
console.log({
existing,
remove,
saveQuotaWouldDelete: existing && remove,
saveQuotaMessage: existing && remove ? 'delete' : 'No quotas to remove',
})
JSRepository: jamcalli/Pulsarr
Length of output: 526
Keep quota query state behind the user-loading gate.
useUsersWithQuota() sets userQuotas: null, hasUserData: true, and isLoading: usersQuery.isLoading while quota config/status queries can still be pending. The page renders the user table in that state, so quota edit can pass userQuotas: null into saveQuota(), where a disabled quota form is treated as “No quotas to remove” instead of deleting the existing quota. Keep quota-dependent data unavailable until all required quota queries are ready, and derive the shared loading state from the combined query state.
📍 Affects 2 files
src/client/features/plex/hooks/usePlexUsers.ts#L214-L228(this comment)src/client/features/plex/hooks/usePlexUsers.ts#L295-L299src/client/features/plex/hooks/usePlexUser.ts#L4-L31
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/client/features/plex/hooks/usePlexUsers.ts` around lines 214 - 228,
Update useUsersWithQuota and the related quota-loading logic in
src/client/features/plex/hooks/usePlexUsers.ts (lines 214-228 and 295-299) and
src/client/features/plex/hooks/usePlexUser.ts (lines 4-31) so quota-dependent
data remains unavailable until the user, quota configuration, and quota status
queries are all ready. Derive isLoading from the combined query states and do
not expose userQuotas as null alongside hasUserData: true while quota queries
are pending.
Owner
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/features/plex/hooks/usePlexUsers.ts`:
- Around line 295-303: The aggregate returned by the Plex users hook must
include userStatusQuery in its loading state and combine loading, error, and
retry behavior across every required query, including quotaConfigsQuery,
movieStatusQuery, and showStatusQuery. Update PlexUsersPage and
quota-editing/saveQuota flows to block edits until the aggregate succeeds,
preserving existing quotas when quota loading fails. Add coverage for delayed
Plex status and failed quota queries.
In `@src/client/features/utilities/pages/plex-notifications.tsx`:
- Around line 27-28: Update the readiness flow in the Plex notifications page
using useConfig and useInitializeWithMinDuration so form rendering waits for
config initialization and the /v1/config query to complete after the minimum
delay. Ensure useConfig.initialize() forces the required refetch, keep the form
hidden while usePlexNotifications.config or plexToken is unavailable, and route
configError through the same readiness/error state.
In `@src/schemas/config/config.schema.ts`:
- Around line 470-472: Update the tmdbRegion validation in ConfigFullSchema to
handle existing lowercase persisted values before enforcing the uppercase
two-letter format. Normalize values consistently on read or write, or
migrate/clear stale lowercase entries, while preserving the US default and
uppercase validation for new updates.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: e20438f0-3630-499f-8fa7-a67b2ba49f6f
⛔ Files ignored due to path filters (1)
docs/static/openapi.jsonis excluded by!docs/**
📒 Files selected for processing (92)
.github/workflows/ci.ymlsrc/client/components/AppSidebar.tsxsrc/client/components/network-config-credenza.tsxsrc/client/components/settings-provider.tsxsrc/client/components/theme-provider.tsxsrc/client/components/tmdb-content-viewer.tsxsrc/client/components/tmdb-metadata-display.tsxsrc/client/components/ui/apprise-status-badge.tsxsrc/client/components/ui/sidebar.tsxsrc/client/components/ui/streaming-provider-multi-select.tsxsrc/client/components/ui/tmdb-region-selector.tsxsrc/client/components/ui/user-multi-select.tsxsrc/client/components/ui/workflow-status-badge.tsxsrc/client/features/approvals/components/approval-actions-modal.tsxsrc/client/features/approvals/components/approval-sonarr-routing-card.tsxsrc/client/features/approvals/index.tsxsrc/client/features/approvals/pages/approval-settings.tsxsrc/client/features/approvals/pages/quota-settings.tsxsrc/client/features/approvals/store/approvalsStore.tssrc/client/features/content-router/components/accordion-content-router-section.tsxsrc/client/features/content-router/components/accordion-route-card.tsxsrc/client/features/content-router/components/condition-input.tsxsrc/client/features/dashboard/hooks/useChartData.tssrc/client/features/dashboard/hooks/useDashboardStats.tssrc/client/features/dashboard/hooks/useMediaViewMode.tssrc/client/features/dashboard/index.tsxsrc/client/features/notifications/components/apprise/apprise-form.tsxsrc/client/features/notifications/components/discord/discord-bot-form.tsxsrc/client/features/notifications/components/discord/discord-webhook-form.tsxsrc/client/features/notifications/components/general/general-settings-form.tsxsrc/client/features/notifications/components/plex-mobile/plex-mobile-form.tsxsrc/client/features/notifications/hooks/useNotificationsConfig.tssrc/client/features/notifications/hooks/usePublicContentNotifications.tssrc/client/features/notifications/index.tsxsrc/client/features/plex/components/setup/setup-modal.tsxsrc/client/features/plex/components/user/quota-edit-modal.tsxsrc/client/features/plex/components/user/quota-status-badge.tsxsrc/client/features/plex/components/user/user-edit-modal.tsxsrc/client/features/plex/components/user/user-table.tsxsrc/client/features/plex/components/user/user-watchlist-sheet.tsxsrc/client/features/plex/hooks/useApprovalConfiguration.tssrc/client/features/plex/hooks/useBulkQuotaManagement.tssrc/client/features/plex/hooks/usePlexBulkUpdate.tssrc/client/features/plex/hooks/usePlexConnection.tssrc/client/features/plex/hooks/usePlexExistenceCheck.tssrc/client/features/plex/hooks/usePlexRssFeeds.tssrc/client/features/plex/hooks/usePlexSetup.tssrc/client/features/plex/hooks/usePlexUser.tssrc/client/features/plex/hooks/usePlexUsers.tssrc/client/features/plex/hooks/usePlexWatchlist.tssrc/client/features/plex/hooks/useQuotaManagement.tssrc/client/features/plex/pages/configuration.tsxsrc/client/features/plex/pages/users.tsxsrc/client/features/plex/store/types.tssrc/client/features/radarr/pages/radarr-content-router.tsxsrc/client/features/radarr/pages/radarr-instances.tsxsrc/client/features/sonarr/components/instance/sonarr-instance-card.tsxsrc/client/features/sonarr/pages/sonarr-content-router.tsxsrc/client/features/sonarr/pages/sonarr-instances.tsxsrc/client/features/utilities/components/api-keys/api-keys-delete-confirmation-modal.tsxsrc/client/features/utilities/components/session-monitoring/manage-rolling-sheet.tsxsrc/client/features/utilities/components/session-monitoring/rolling-shows-sheet.tsxsrc/client/features/utilities/hooks/useApiKeys.tssrc/client/features/utilities/hooks/useDeleteSyncForm.tssrc/client/features/utilities/hooks/usePlexLabels.tssrc/client/features/utilities/hooks/usePlexNotifications.tssrc/client/features/utilities/hooks/useSchedules.tssrc/client/features/utilities/hooks/useSessionMonitoring.tssrc/client/features/utilities/hooks/useUserTags.tssrc/client/features/utilities/pages/api-keys.tsxsrc/client/features/utilities/pages/delete-sync.tsxsrc/client/features/utilities/pages/log-viewer.tsxsrc/client/features/utilities/pages/new-user-defaults.tsxsrc/client/features/utilities/pages/plex-labels.tsxsrc/client/features/utilities/pages/plex-notifications.tsxsrc/client/features/utilities/pages/plex-session-monitoring.tsxsrc/client/features/utilities/pages/user-tags.tsxsrc/client/features/utilities/pages/watchlist-exclusions.tsxsrc/client/hooks/use-table-pagination.tssrc/client/hooks/useConfig.tssrc/client/hooks/useCurrentUser.tssrc/client/hooks/useUserOptions.tssrc/client/hooks/useVersionCheck.tsxsrc/client/lib/prefs.tssrc/client/stores/configStore.tssrc/client/types/api.d.tssrc/plugins/external/swagger.tssrc/routes/v1/config/config.tssrc/schemas/config/config.schema.tssrc/schemas/plex/label-sync-config.schema.tssrc/services/database/methods/config.tssrc/services/plex-label-sync.service.ts
💤 Files with no reviewable changes (6)
- src/services/database/methods/config.ts
- src/services/plex-label-sync.service.ts
- src/client/features/notifications/hooks/useNotificationsConfig.ts
- src/schemas/plex/label-sync-config.schema.ts
- src/client/stores/configStore.ts
- src/client/features/plex/hooks/usePlexSetup.ts
Comment on lines
+295
to
+303
| return { | ||
| usersWithQuota, | ||
| hasUserData: (users?.length ?? 0) > 0, | ||
| // pending quota data is indistinguishable from a user having no quotas | ||
| isLoading: | ||
| usersQuery.isLoading || | ||
| quotaConfigsQuery.isLoading || | ||
| movieStatusQuery.isLoading || | ||
| showStatusQuery.isLoading, |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not expose a partial query aggregate as ready.
usersWithQuota depends on every query above. Lines 299-303 omit userStatusQuery, so a pending status query produces rows without friend status and omits untracked Plex users.
A failed quota query also produces userQuotas: null. PlexUsersPage handles only usersQuery.isError, so saveQuota can treat existing quotas as absent.
Return combined loading, error, and retry state for every required query. Block quota editing until the aggregate succeeds. Add tests for delayed Plex status and failed quota queries.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/client/features/plex/hooks/usePlexUsers.ts` around lines 295 - 303, The
aggregate returned by the Plex users hook must include userStatusQuery in its
loading state and combine loading, error, and retry behavior across every
required query, including quotaConfigsQuery, movieStatusQuery, and
showStatusQuery. Update PlexUsersPage and quota-editing/saveQuota flows to block
edits until the aggregate succeeds, preserving existing quotas when quota
loading fails. Add coverage for delayed Plex status and failed quota queries.
Comment on lines
+470
to
472
| .regex(/^[A-Z]{2}$/, { | ||
| error: 'Region must be exactly 2 uppercase letters (A-Z)', | ||
| }) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 'tmdbRegion|tmdb-region-selector|iso_3166_1' \
src/client src/routes src/services src/schemas || trueRepository: jamcalli/Pulsarr
Length of output: 25219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== config schema around tmdbRegion defaults/update/read =="
sed -n '150,195p' src/schemas/config/config.schema.ts
sed -n '456,474p' src/schemas/config/config.schema.ts
rg -n "ConfigFullSchema|ConfigSchema|ConfigUpdateSchema|defaultConfig|ConfigWithDefaults|tmdbRegion:.*'US'|tmdbRegion:.*US" src/schemas src/services src/routes -C 3 || true
echo "== locate migrations/config init files =="
git ls-files | rg '(^|/)(migrations|migration|prisma|knex|.*config.*\.(sql|ts|js))(?<!\.spec\.ts)(?<!\.test\.ts)$|src/services/database/methods/config\.ts' | sort
echo "== database methods config relevant sections =="
sed -n '200,260p' src/services/database/methods/config.ts
sed -n '380,415p' src/services/database/methods/config.ts
sed -n '540,570p' src/services/database/methods/config.ts
echo "== all lowercase/uppercase region-ish literals =="
rg -n -i '\b[a-z]{2}\b|TMDB|tmdb|PROVIDER|provider|availableProviders|getAvailableProviders' src/routes src/services src/schemas src/client -C 2 | sed -n '1,220p' || trueRepository: jamcalli/Pulsarr
Length of output: 24831
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate config/migration files =="
git ls-files | grep -E '(^|/)(migrations|migration|prisma|knex|.*config.*\.(sql|ts|js))\.?(spec\.ts|test\.ts)?$' | grep -vE '\.(spec\.ts|test\.ts)$' | sort || true
echo "== tmdb-region-selector implementation =="
sed -n '1,130p' src/client/components/ui/tmdb-region-selector.tsx
echo "== route config validation and update handling =="
sed -n '1,230p' src/routes/v1/config/config.ts
echo "== all direct tmdbRegion references outside generated type file =="
rg -n '\b(tmdbRegion|tmdb-region-selector)\b' src --glob '!src/client/types/api.d.ts' || true
echo "== simple deterministic zod schema behavior probe =="
node - <<'JS'
const z = require('zod')
const ConfigFullSchema = z.object({ tmdbRegion: z.string() })
const ConfigUpdateSchema = z.object({
tmdbRegion: z.string().trim().regex(/^[A-Z]{2}$/, { error: 'Region must be exactly 2 uppercase letters (A-Z)' }).optional(),
})
const values = ['US', 'us', 'uk', 'de', 'aB']
for (const v of values) {
try { console.log('full', v, ConfigFullSchema.parse({ tmdbRegion: v }).tmdbRegion) }
catch (e) { console.log('full', v, 'ERR') }
try { console.log('update', v, ConfigUpdateSchema.parse({ tmdbRegion: v ? v : undefined }).tmdbRegion) }
catch (e) { console.log('update', v, 'ERR') }
}
JSRepository: jamcalli/Pulsarr
Length of output: 15642
Check for existing lowercase tmdbRegion values before enforcing uppercase.
tmdbRegion is now ^[A-Z]{2}$ on updates, defaults to US, and existing database values are not normalized. A lowercase persisted value such as us can be returned by ConfigFullSchema but will fail the next update; keep normalization on write/read or migrate/clear stale lowercase values.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/schemas/config/config.schema.ts` around lines 470 - 472, Update the
tmdbRegion validation in ConfigFullSchema to handle existing lowercase persisted
values before enforcing the uppercase two-letter format. Normalize values
consistently on read or write, or migrate/clear stale lowercase entries, while
preserving the US default and uppercase validation for new updates.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
Related Issues
Type of Change
Testing Performed
Screenshots
Checklist
Summary by CodeRabbit