Bug-3566: Add searchable project group picker and TDEI fixes#47
Merged
Bug-3566: Add searchable project group picker and TDEI fixes#47
Conversation
Replace simple select with a searchable, keyboard-navigable ProjectGroupPicker (infinite scroll, debounced search, click-outside handling, highlighting). Add scoped styles and accessibility improvements. Update services/tdei.ts with stronger TypeScript annotations, Promise return types, optional chaining, safer JWT field casts, and paginated/searchable getMyProjectGroups API. Apply related type and runtime guards in pages/workspace/create/tdei.vue (typed refs, map removal guard, bounds check, and safer import parameter casts). Also add "qrcode" to package.json dependencies.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces a static project-group select with a searchable, paginated dropdown (keyboard + infinite-scroll). Strengthens TypeScript typings for TDEI services and page code, adds new TDEI API response and UI types, improves map null-safety, and adds the Changes
Sequence DiagramsequenceDiagram
actor User
participant Picker as ProjectGroupPicker
participant API as Backend API
User->>Picker: Focus / Type search
activate Picker
Picker->>Picker: Debounce input
Picker->>API: loadGroups(pageNo, searchText)
activate API
API-->>Picker: paginated groups
deactivate API
Picker->>Picker: Render items / highlight
deactivate Picker
User->>Picker: Arrow / Enter / Click item
activate Picker
Picker->>Picker: Update highlighted / select
Picker->>Picker: Update model & close
deactivate Picker
User->>Picker: Scroll near bottom
activate Picker
Picker->>API: loadGroups(nextPage, searchText)
activate API
API-->>Picker: next page
deactivate API
Picker->>Picker: Append items
deactivate Picker
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 47 minutes and 58 seconds.Comment |
jeffmaki
reviewed
Apr 28, 2026
jeffmaki
reviewed
Apr 28, 2026
jeffmaki
reviewed
Apr 28, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/tdei.ts (1)
17-27:⚠️ Potential issue | 🟠 MajorDecode JWT payload as base64url before
atob.At line 26,
atob(body)assumes standard base64. JWT payloads are base64url-encoded per RFC 7519 and can include-and_characters. These will produce incorrect decoding withatob()and cause JSON parse failures for valid tokens.🔧 Proposed fix
function getJwtBody(accessToken: string): any { const bodyStart = accessToken.indexOf('.'); const bodyEnd = accessToken.indexOf('.', bodyStart + 1); if (bodyStart === -1 || bodyEnd === -1) { throw new Error('Error parsing JWT body'); } - let body = accessToken.substring(bodyStart + 1, bodyEnd); - body = JSON.parse(atob(body)) + const body = accessToken.substring(bodyStart + 1, bodyEnd); + const normalized = body.replace(/-/g, '+').replace(/_/g, '/'); + const padded = normalized + '='.repeat((4 - normalized.length % 4) % 4); + const parsed = JSON.parse(atob(padded)); - return body; + return parsed; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/tdei.ts` around lines 17 - 27, getJwtBody currently runs atob on the JWT payload which is base64url-encoded; update getJwtBody to normalize the extracted payload string (variable body) from base64url to standard base64 by replacing '-' -> '+' and '_' -> '/' and adding '=' padding to reach a length multiple of 4, then call atob on the normalized string and JSON.parse the result; keep the existing JWT part extraction and existing error throw for malformed tokens and ensure JSON.parse runs on the decoded payload.
🧹 Nitpick comments (1)
pages/workspace/create/tdei.vue (1)
232-235: Prefer explicit null guards overas stringcasts increate().At Lines 232-233, casts can mask null runtime values. Since this payload is forwarded to import/create calls, a defensive guard here will prevent invalid requests.
🔧 Proposed refactor
async function create() { + if (!tdeiRecordId.value || !projectGroupId.value) { + return + } + const workspaceId = await importer.import({ title: workspaceTitle.value, type: record.data_type, - tdeiRecordId: tdeiRecordId.value as string, - tdeiProjectGroupId: projectGroupId.value as string, + tdeiRecordId: tdeiRecordId.value, + tdeiProjectGroupId: projectGroupId.value, tdeiServiceId: record.service?.tdei_service_id || '', tdeiMetadata: JSON.stringify(record), })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/workspace/create/tdei.vue` around lines 232 - 235, In the create() payload construction the fields tdeiRecordId: tdeiRecordId.value as string and tdeiProjectGroupId: projectGroupId.value as string use unsafe casts; replace these casts with explicit null/undefined guards inside the create() function (e.g., check tdeiRecordId.value and projectGroupId.value before building the payload), and either throw a descriptive error or return early if they are missing so you never send an invalid request; update the code paths that call the import/create API to rely on these validated values (references: tdeiRecordId, projectGroupId, create()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ProjectGroupPicker.vue`:
- Around line 61-70: The loadGroups function currently bails out immediately
when loading.value is true, discarding concurrent watcher-triggered calls; add a
small retry queue by introducing a pendingLoad flag (and optionally
pendingReset) so that if loadGroups(reset) is invoked while loading.value is
true it records pendingLoad = true (and pendingReset = pendingReset || reset)
then returns; at the end of the actual network flow inside loadGroups
(success/failure/finally) check pendingLoad and if set call
loadGroups(pendingReset) and clear pendingLoad/pendingReset. Update the watcher
(the one that currently calls loadGroups on search/page changes) to simply call
loadGroups() as before (it will now enqueue instead of drop), ensuring you
reference loadGroups, loading.value, pendingLoad, and pendingReset when making
the change.
- Around line 215-218: The current logic in ProjectGroupPicker.vue overwrites a
preexisting selection by setting model.value to projectGroups.value[0]?.id when
model.value is truthy but not found in the currently loaded page; change this so
we only assign the first project group when there is no existing selection (use
a null/undefined check or !model.value strictly for empty), i.e. remove the
branch that replaces a non-null model when it isn't in projectGroups.value and
only set model.value = projectGroups.value[0]?.id when model.value is
null/undefined; reference the projectGroups.value and model.value checks in the
component to implement this guard.
- Around line 3-10: ProjectGroupPicker is not declaring or binding a disabled
prop so the parent's :disabled ends up on the wrapper div instead of the actual
<input>; declare a prop named disabled (Boolean) on the component and bind it to
the input element (e.g., add :disabled="disabled" to the <input> that uses
v-model="searchText"), or alternatively set inheritAttrs: false and explicitly
forward attributes to the input; update the component's props block (or script
setup) and the input element (where onFocus/onKeydown are defined) to ensure the
disabled state is applied to the actual input.
In `@services/tdei.ts`:
- Around line 386-388: The assignments to this.#auth.subject,
this.#auth.displayName, and this.#auth.email currently use casts
(jwt.sub/name/email as unknown as string) without runtime checks; update the
code (where jwt is handled and before these assignments) to validate that
jwt.sub, jwt.name, and jwt.email are strings (and non-empty if required) and
only then assign them to this.#auth.subject/displayName/email, otherwise set a
safe fallback (e.g., empty string) or throw an explicit error; also ensure any
code that builds URLs in getMyProjectGroups defensively handles missing/invalid
this.#auth values so it won't produce fields like project-group-roles/undefined.
---
Outside diff comments:
In `@services/tdei.ts`:
- Around line 17-27: getJwtBody currently runs atob on the JWT payload which is
base64url-encoded; update getJwtBody to normalize the extracted payload string
(variable body) from base64url to standard base64 by replacing '-' -> '+' and
'_' -> '/' and adding '=' padding to reach a length multiple of 4, then call
atob on the normalized string and JSON.parse the result; keep the existing JWT
part extraction and existing error throw for malformed tokens and ensure
JSON.parse runs on the decoded payload.
---
Nitpick comments:
In `@pages/workspace/create/tdei.vue`:
- Around line 232-235: In the create() payload construction the fields
tdeiRecordId: tdeiRecordId.value as string and tdeiProjectGroupId:
projectGroupId.value as string use unsafe casts; replace these casts with
explicit null/undefined guards inside the create() function (e.g., check
tdeiRecordId.value and projectGroupId.value before building the payload), and
either throw a descriptive error or return early if they are missing so you
never send an invalid request; update the code paths that call the import/create
API to rely on these validated values (references: tdeiRecordId, projectGroupId,
create()).
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 005d350a-6a48-47a7-9fcf-3e20686d3301
📒 Files selected for processing (4)
components/ProjectGroupPicker.vuepackage.jsonpages/workspace/create/tdei.vueservices/tdei.ts
jeffmaki
requested changes
Apr 28, 2026
This was referenced Apr 28, 2026
Add explicit TDEI API and normalized types and update service clients to use them. services/tdei.ts: import and apply new types, add return types, safer JSON parsing and JWT field checks, and better error handling in getMyProjectGroups. types/tdei.ts: introduce API response shapes and normalized TdeiProjectGroup/TdeiService/TdeiDatasetSummary interfaces. components/ProjectGroupPicker.vue: add disabled prop, implement pendingReset logic to queue resets while loading, and simplify initial selection logic. pages/workspace/create/tdei.vue: replace manual property copy with Object.assign and guard against null dataset info.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/workspace/create/tdei.vue (1)
184-195:⚠️ Potential issue | 🔴 CriticalReturn early before dereferencing
record.project_group.
if (!info) returnonly exits theloading.wrapcallback, so the function still falls through tonextTick()and line 194 can throw when the API returns no dataset. Bail out fromgetDatasetInfo()itself here, or reset the form state and stop processing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/workspace/create/tdei.vue` around lines 184 - 195, The code currently only returns from the loading.wrap callback when client.getDatasetInfo(id) yields no info, but execution continues and dereferences record.project_group (used to set projectGroupId.value) causing a crash; update the flow so that after awaiting loading.wrap(...) you check whether the record was populated (e.g., verify record.metadata or record.project_group or a boolean flag set inside the callback) and if not, bail out from the enclosing function (or reset form state) before calling nextTick() and before setting workspaceTitle.value, projectGroupId.value, and tdeiRecordId.value; alternatively, move the early return out of the loading.wrap and return from the outer async function right when client.getDatasetInfo(id) returns no info (referencing loading.wrap, tdeiClient, client.getDatasetInfo, record, nextTick, workspaceTitle.value, projectGroupId.value, tdeiRecordId.value).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pages/workspace/create/tdei.vue`:
- Around line 184-188: The code currently shallow-merges API result into the
existing record with Object.assign(record, info), which can leave stale keys;
instead, when handling the getDatasetInfo response inside the loading.wrap
callback (the client from tdeiClient and the variables record and info), replace
the record wholesale or clear it before copying: either remove all existing keys
from record (e.g., delete each key) and then Object.assign(record, info), or set
record to the new info object/reactive copy so no old fields remain; update the
logic in the loading.wrap(...) callback where client.getDatasetInfo(id) is
handled to perform this replacement rather than a shallow merge.
---
Outside diff comments:
In `@pages/workspace/create/tdei.vue`:
- Around line 184-195: The code currently only returns from the loading.wrap
callback when client.getDatasetInfo(id) yields no info, but execution continues
and dereferences record.project_group (used to set projectGroupId.value) causing
a crash; update the flow so that after awaiting loading.wrap(...) you check
whether the record was populated (e.g., verify record.metadata or
record.project_group or a boolean flag set inside the callback) and if not, bail
out from the enclosing function (or reset form state) before calling nextTick()
and before setting workspaceTitle.value, projectGroupId.value, and
tdeiRecordId.value; alternatively, move the early return out of the loading.wrap
and return from the outer async function right when client.getDatasetInfo(id)
returns no info (referencing loading.wrap, tdeiClient, client.getDatasetInfo,
record, nextTick, workspaceTitle.value, projectGroupId.value,
tdeiRecordId.value).
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2c52b9a1-a7bd-4112-b0e9-aad064f2e498
📒 Files selected for processing (4)
components/ProjectGroupPicker.vuepages/workspace/create/tdei.vueservices/tdei.tstypes/tdei.ts
✅ Files skipped from review due to trivial changes (1)
- types/tdei.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- components/ProjectGroupPicker.vue
- services/tdei.ts
When loading dataset info in getDatasetInfo, clear any existing keys on the shared record object before Object.assign. This prevents stale/orphaned fields from a previously loaded dataset from persisting when switching datasets.
jeffmaki
approved these changes
Apr 29, 2026
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
2 participants
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.
DevBoard Task
https://dev.azure.com/TDEI-UW/TDEI/_workitems/edit/3566
Changes Implemented:
Impacted Areas For Testing:
Project Group Picker
Test Scenarios
Screenshots:
Overview
This PR replaces the simple project-group with a searchable, keyboard-navigable ProjectGroupPicker and tightens TDEI-related TypeScript and runtime guards. It also fixes a missing runtime dependency by adding "qrcode". Changes by File components/ProjectGroupPicker.vue Replaces fully-fetched with an async, searchable dropdown supporting:
services/tdei.ts
types/tdei.ts
pages/workspace/create/tdei.vue
package.json
Test Coverage / QA Scenarios
Impacted Pages