feat: add probe filtering by adoption status for admin users #133
Conversation
WalkthroughThis PR adds admin-facing adoption filtering and new filter UI components: AdminFilterSettings.vue and ProbeListFilters.vue, and updates MobileProbeListFilters.vue to use the shared useProbeFilters composable with debounced search and Directus-backed status counts. useProbeFilters gains AdoptionOption/ADOPTION_OPTIONS, an adoption field on Filter/DEFAULT_FILTER, and new helpers getDirectusFilter and isDefault; it applies admin-aware query construction. pages/probes/index.vue now uses the new filter components, fetches probe user info, and relies on filtered counts. A new type ProbeWithUser is introduced. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests
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}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
components/probe/ProbeFilters/ProbeListFilters.vue (2)
47-55: Add accessible name to the icon-only admin buttonThe icon-only button lacks an accessible label. Add an aria-label (and optionally aria-haspopup/expanded) for screen readers.
Apply this diff:
<Button class="relative hover:bg-white focus:ring-primary dark:hover:bg-dark-900 dark:focus:ring-primary" severity="secondary" size="small" text + aria-label="Admin filters" + aria-haspopup="dialog" @click="adminOptsRef.toggle($event)">
41-43: Harden search input UX for free-text queriesDisable spellcheck and auto-correction to avoid noisy search queries.
Apply this diff:
- <InputText v-model="searchInput" class="m-0 h-full min-w-[280px]" placeholder="Filter by name, location, or tags" @input="onFilterChangeDebounced"/> + <InputText + v-model="searchInput" + class="m-0 h-full min-w-[280px]" + placeholder="Filter by name, location, or tags" + spellcheck="false" + autocapitalize="off" + autocorrect="off" + @input="onFilterChangeDebounced" + />composables/useProbeFilters.ts (2)
9-10: Make ADOPTION_OPTIONS type-safe and derive AdoptionOption from itCurrent
ADOPTION_OPTIONS: string[]defeats theas consttuple narrowing. Derive the union from the tuple to improve type-safety acrossincludeschecks and props.Apply this diff:
-export type AdoptionOption = 'all' | 'adopted' | 'non-adopted'; +export const ADOPTION_OPTIONS = [ 'all', 'adopted', 'non-adopted' ] as const; +export type AdoptionOption = typeof ADOPTION_OPTIONS[number];-export const ADOPTION_OPTIONS: string[] = [ 'all', 'adopted', 'non-adopted' ] as const; +// moved above to derive AdoptionOption; keep single source of truthAlso applies to: 37-38
165-169: Sanitize adoption param for non-adminsIf a non-admin opens a URL with
?adoption=..., the filter resets locally to default but the URL param stays. Consider strippingadoptionfrom the URL for non-admins to avoid confusing, inert params.Proposed snippet (inside the watcher after setting defaults):
if (!auth.isAdmin && 'adoption' in route.query) { const { adoption: _omit, ...rest } = route.query as Record<string, any>; navigateTo({ query: rest }); }types.ts (1)
124-125: Narrow the user shape to match selected fields (and nullability)You only fetch
{ id, github_username }andusercan be null for non‑adopted probes. Narrowing the type avoids over‑promising fields and matches template usageuser?.github_username.Apply this diff:
-type ProbeWithUser<TCountry extends string = string> = Probe<TCountry> & { user: Partial<User> }; +type ProbeWithUser<TCountry extends string = string> = Probe<TCountry> & { + user: Pick<User, 'id' | 'github_username'> | null; +};pages/probes/index.vue (1)
301-303: Align selection type with loaded items
probesareProbeWithUser[], butselectedProbesis typed asProbe[]. Align types to avoid subtle runtime/TS mismatches when passing selection to other components.Apply this diff:
-const selectedProbes = ref<Probe[]>([]); +const selectedProbes = ref<ProbeWithUser[]>([]);components/probe/ProbeFilters/MobileProbeListFilters.vue (3)
113-115: Cancel the debounced handler on unmount to avoid late updates.Prevent updates firing after component disposal.
Add:
onScopeDispose(() => { onFilterChangeDebounced.cancel?.(); });
133-134: Avoid refetching counts on unrelated filter changes.Watching the entire draftFilterDeps re-queries on sort/desc/etc. If counts only depend on search/adoption (and ignore status), narrow the watch source.
Example:
const statusCountDeps = computed(() => ({ search: draftFilter.value.search, adoption: draftFilter.value.adoption, // if applicable // add other relevant fields only })); watch: [ statusCountDeps ],Based on learnings.
88-89: Ensure AdminFilterSettings doesn’t mutate props directly.If the child mutates draftFilter via prop, that’s an anti-pattern. Prefer emitting updates or accepting a change handler.
If needed, switch to an explicit update contract, e.g.:
- Child: emit('update:adoption', value) or emit('batch-change', partial)
- Parent: <AdminFilterSettings v-if="auth.isAdmin" :filter="draftFilter" @batch-change="Object.assign(draftFilter, $event)" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/probe/ProbeFilters/AdminFilterSettings.vue(1 hunks)components/probe/ProbeFilters/MobileProbeListFilters.vue(3 hunks)components/probe/ProbeFilters/ProbeListFilters.vue(1 hunks)composables/useProbeFilters.ts(8 hunks)pages/probes/index.vue(13 hunks)types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
types.tscomponents/probe/ProbeFilters/ProbeListFilters.vuecomponents/probe/ProbeFilters/MobileProbeListFilters.vuecomposables/useProbeFilters.tspages/probes/index.vuecomponents/probe/ProbeFilters/AdminFilterSettings.vue
🧠 Learnings (3)
📓 Common learnings
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
📚 Learning: 2025-07-20T06:27:45.919Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: pages/probes/index.vue:33-34
Timestamp: 2025-07-20T06:27:45.919Z
Learning: In the GlobalPing Dashboard, getSortSettings() from useProbeFilters composable returns sort fields formatted specifically for the Directus SDK API calls (e.g., '-name' for descending), while PrimeVue DataTable components should use filter.by and filter.desc for their :sort-field and :sort-order props respectively. These serve different purposes and should not be mixed.
Applied to files:
composables/useProbeFilters.ts
📚 Learning: 2025-07-17T14:33:26.596Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: pages/probes/index.vue:416-428
Timestamp: 2025-07-17T14:33:26.596Z
Learning: In the GlobalPing Dashboard probes listing page, there's a performance optimization where an initial lightweight probe count check is performed in onMounted() using only getUserFilter('userId'). This avoids running the more expensive loadLazyData() function (which includes complex filtering and status aggregation) when a user has no probes at all. The loadLazyData() function is only called if the initial count check confirms probes exist.
Applied to files:
pages/probes/index.vue
🧬 Code graph analysis (1)
composables/useProbeFilters.ts (1)
store/auth.ts (1)
useAuth(23-175)
⏰ 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: Run e2e tests
🔇 Additional comments (2)
pages/probes/index.vue (1)
359-371: Use stricter field typing for user relationYou rely on aliasing
user: 'userId'with limited fields. EnsureProbeWithUserreflectsuser | null(see types.ts suggestion) and consider guarding against missing relation responses.Please verify the Directus SDK actually returns
user: null(notundefined) whenuserIdis null. If it returnsundefined, update the Nullish coalescing in the template accordingly.components/probe/ProbeFilters/MobileProbeListFilters.vue (1)
124-129: Confirm getDirectusFilter arg semantics and ref vs value usage.You pass draftFilter (a Ref) and a string array. If getDirectusFilter expects the raw object or an “exclude keys” list, this may be wrong and could drop search/adoption filters from counts.
- Verify getDirectusFilter signature: does it accept Ref or Filter?
- Verify the second parameter meaning: include-only vs exclude keys.
- If it expects a plain object, pass draftFilter.value.
Use the script in the previous comment to locate the signature.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/probe/ProbeFilters/MobileProbeListFilters.vue (1)
124-124: Type AsyncData result for statusCounts.Improve TS safety by specifying the transformed data shape.
Apply this diff:
- const { data: statusCounts, error: statusCntError } = await useLazyAsyncData( + const { data: statusCounts, error: statusCntError } = await useLazyAsyncData<Record<StatusCode, number>>(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/probe/ProbeFilters/AdminFilterSettings.vue(1 hunks)components/probe/ProbeFilters/MobileProbeListFilters.vue(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/probe/ProbeFilters/AdminFilterSettings.vue
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
components/probe/ProbeFilters/MobileProbeListFilters.vue
🧠 Learnings (2)
📓 Common learnings
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
📚 Learning: 2025-07-17T21:20:55.175Z
Learnt from: MartinKolarik
PR: jsdelivr/globalping-dash#103
File: components/DeleteProbes.vue:34-39
Timestamp: 2025-07-17T21:20:55.175Z
Learning: In the GlobalPing Dashboard codebase, prefer using `type: Array as PropType<Probe[]>` for prop definitions without importing PropType from 'vue'. This maintains runtime type checking while avoiding extra imports.
Applied to files:
components/probe/ProbeFilters/MobileProbeListFilters.vue
⏰ 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: Run e2e tests
- GitHub Check: Run e2e tests
🔇 Additional comments (3)
components/probe/ProbeFilters/MobileProbeListFilters.vue (3)
92-92: No change required here.Template ref unwrapping passes
draftFilter.valuetoonBatchChange; calling withdraftFilteris fine.
44-44: Fix invalid InputText event; debounce never fires.PrimeVue InputText doesn’t emit
value-change. Use@update:modelValue(or watchsearchInput) so the debounced sync runs.Apply this diff:
- <InputText v-model="searchInput" class="m-0 size-full" placeholder="Filter by name, location, or tags" @value-change="onFilterChangeDebounced"/> + <InputText v-model="searchInput" class="m-0 size-full" placeholder="Filter by name, location, or tags" @update:modelValue="onFilterChangeDebounced"/>Alternatively, drop the template event and wire a watcher:
-const onFilterChangeDebounced = debounce(() => draftFilter.value.search = searchInput.value, 300); +const onFilterChangeDebounced = debounce(() => { draftFilter.value.search = searchInput.value; }, 300); +watch(searchInput, () => onFilterChangeDebounced());
102-102: Import missing Status type and fix aggregate response typing.
Statusis used but not imported, and the generic[{ ... }]is a tuple, not an array.Apply these diffs:
- import { type StatusCode, SORTABLE_FIELDS, STATUS_MAP, useProbeFilters } from '~/composables/useProbeFilters'; + import { type Status, type StatusCode, SORTABLE_FIELDS, STATUS_MAP, useProbeFilters } from '~/composables/useProbeFilters';- () => $directus.request<[{ count: number; status: Status; isOutdated: boolean }]>(aggregate('gp_probes', { + () => $directus.request<Array<{ count: number; status: Status; isOutdated: boolean }>>(aggregate('gp_probes', {If
Statusisn’t exported, replace withstringor export it from the composable.Also applies to: 125-125
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)
pages/tokens.vue (1)
270-271: Guard transform against undefined/empty aggregate result to avoid runtime error.Accessing
data[0]can throw if the response is empty/undefined.Apply:
- default: () => 0, transform: data => data[0].count ?? 0, + default: () => 0, transform: data => data?.[0]?.count ?? 0,
🧹 Nitpick comments (2)
components/probe/ProbeFilters/ProbeListFilters.vue (2)
8-9: Remove redundant option-label for string options.
STATUS_CODESis a string array;option-label="code"is ineffective and misleading when rendering via slots.Apply:
- :pt="{ listContainer: { class: '!max-h-64' } }" - option-label="code" + :pt="{ listContainer: { class: '!max-h-64' } }"
84-92: Prefer Array<> generic for Directus request typing.Use
Array<{ ... }>for clarity over a tuple-like type.Apply:
- () => $directus.request<[{ count: number; status: Status; isOutdated: boolean }]>(aggregate('gp_probes', { + () => $directus.request<Array<{ count: number; status: Status; isOutdated: boolean }>>(aggregate('gp_probes', {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/probe/ProbeFilters/MobileProbeListFilters.vue(3 hunks)components/probe/ProbeFilters/ProbeListFilters.vue(1 hunks)pages/notifications.vue(2 hunks)pages/probes/index.vue(14 hunks)pages/tokens.vue(2 hunks)
👮 Files not reviewed due to content moderation or server errors (1)
- pages/notifications.vue
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
pages/notifications.vuecomponents/probe/ProbeFilters/ProbeListFilters.vuepages/tokens.vuecomponents/probe/ProbeFilters/MobileProbeListFilters.vuepages/probes/index.vue
🧠 Learnings (5)
📓 Common learnings
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
📚 Learning: 2025-07-17T21:20:55.175Z
Learnt from: MartinKolarik
PR: jsdelivr/globalping-dash#103
File: components/DeleteProbes.vue:34-39
Timestamp: 2025-07-17T21:20:55.175Z
Learning: In the GlobalPing Dashboard codebase, prefer using `type: Array as PropType<Probe[]>` for prop definitions without importing PropType from 'vue'. This maintains runtime type checking while avoiding extra imports.
Applied to files:
components/probe/ProbeFilters/MobileProbeListFilters.vue
📚 Learning: 2025-07-18T09:21:36.375Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: composables/useProbeFilters.ts:1-13
Timestamp: 2025-07-18T09:21:36.375Z
Learning: In the GlobalPing Dashboard codebase, all types are exported globally and available without explicit imports. The Status type and other types from types.ts can be used directly without import statements due to Nuxt's auto-import configuration.
Applied to files:
components/probe/ProbeFilters/MobileProbeListFilters.vue
📚 Learning: 2025-07-17T14:33:26.596Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: pages/probes/index.vue:416-428
Timestamp: 2025-07-17T14:33:26.596Z
Learning: In the GlobalPing Dashboard probes listing page, there's a performance optimization where an initial lightweight probe count check is performed in onMounted() using only getUserFilter('userId'). This avoids running the more expensive loadLazyData() function (which includes complex filtering and status aggregation) when a user has no probes at all. The loadLazyData() function is only called if the initial count check confirms probes exist.
Applied to files:
pages/probes/index.vue
📚 Learning: 2025-07-18T09:57:23.099Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: components/DeleteProbes.vue:24-26
Timestamp: 2025-07-18T09:57:23.099Z
Learning: In the GlobalPing Dashboard DeleteProbes component, buttons should use :aria-disabled="loading" instead of :disabled="loading" during loading states. This provides accessibility information without completely preventing interaction, which is the preferred approach for this component.
Applied to files:
pages/probes/index.vue
⏰ 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: Run e2e tests
- GitHub Check: Run e2e tests
🔇 Additional comments (5)
components/probe/ProbeFilters/MobileProbeListFilters.vue (4)
44-45: Fix invalid InputText event; debounce never runs.Apply:
- <InputText v-model="searchInput" class="m-0 size-full" placeholder="Filter by name, location, or tags" @value-change="onFilterChangeDebounced"/> + <InputText v-model="searchInput" class="m-0 size-full" placeholder="Filter by name, location, or tags" @update:modelValue="onFilterChangeDebounced"/>
92-93: Pass the plain filter, not the Ref, to onBatchChange.Apply:
- <Button label="Apply" @click="onBatchChange(draftFilter); emit('apply')"/> + <Button label="Apply" @click="onBatchChange(draftFilter.value); emit('apply')"/>
125-129: Unwrap draftFilter before building Directus filter.
getDirectusFiltershould receive a plain object.Apply:
- filter: getDirectusFilter(draftFilter, [ 'status' ]), + filter: getDirectusFilter(draftFilter.value, [ 'status' ]),
125-132: Use Array<> generic for aggregate response typing.Apply:
- () => $directus.request<[{ count: number; status: Status; isOutdated: boolean }]>(aggregate('gp_probes', { + () => $directus.request<Array<{ count: number; status: Status; isOutdated: boolean }>>(aggregate('gp_probes', {pages/probes/index.vue (1)
365-371: Verify Directus alias mapping for relationaluserfield.This unconventional
fields+aliascombo has no precedent in the codebase and may leaveprobe.userundefined at runtime—confirm against the Directus SDK/API docs or test its behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pages/probes/index.vue (1)
383-393: Use aggregate() for filtered count to avoid unsupported readItems+aggregate comboUsing readItems with aggregate is brittle. You already use aggregate() above for probeCount; reuse it for filteredProbeCount for consistency and to avoid SDK/REST mismatches.
Apply this diff:
- const { data: filteredProbeCount, error: filteredProbeCountError, refresh: refreshFilteredProbeCount, pending: filteredProbeCountLoading } = await useLazyAsyncData( - () => $directus.request<[{ count: number }]>(readItems('gp_probes', { - filter: getCurrentFilter(), - aggregate: { count: '*' }, - })), + const { data: filteredProbeCount, error: filteredProbeCountError, refresh: refreshFilteredProbeCount, pending: filteredProbeCountLoading } = await useLazyAsyncData( + () => $directus.request<[{ count: number }]>(aggregate('gp_probes', { + query: { filter: getDirectusFilter() }, + aggregate: { count: '*' }, + })), { watch: [ filterDeps ], default: () => 0, transform: data => data?.[0]?.count ?? 0, }, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pages/probes/index.vue(14 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
pages/probes/index.vue
🧠 Learnings (3)
📓 Common learnings
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
📚 Learning: 2025-07-17T14:33:26.596Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: pages/probes/index.vue:416-428
Timestamp: 2025-07-17T14:33:26.596Z
Learning: In the GlobalPing Dashboard probes listing page, there's a performance optimization where an initial lightweight probe count check is performed in onMounted() using only getUserFilter('userId'). This avoids running the more expensive loadLazyData() function (which includes complex filtering and status aggregation) when a user has no probes at all. The loadLazyData() function is only called if the initial count check confirms probes exist.
Applied to files:
pages/probes/index.vue
📚 Learning: 2025-07-18T09:57:23.099Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: components/DeleteProbes.vue:24-26
Timestamp: 2025-07-18T09:57:23.099Z
Learning: In the GlobalPing Dashboard DeleteProbes component, buttons should use :aria-disabled="loading" instead of :disabled="loading" during loading states. This provides accessibility information without completely preventing interaction, which is the preferred approach for this component.
Applied to files:
pages/probes/index.vue
⏰ 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: Run e2e tests
- GitHub Check: Run e2e tests
🔇 Additional comments (1)
pages/probes/index.vue (1)
363-366: Remove suggested change: getCurrentFilter already delegates to getDirectusFilter
getCurrentFilter() simply calls getDirectusFilter() under the hood, so replacing it is redundant.Likely an incorrect or invalid review comment.
Closes #128