Open
Conversation
|
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:
📝 WalkthroughWalkthroughAdds workspace role types and a composable for role queries; gates many settings, team, and member UIs by role; introduces a members management page; extends TDEI and workspace service clients with project-group and member endpoints; refactors project-group selection to use TDEI IDs and provides workspace via provide/inject. Changes
Sequence DiagramsequenceDiagram
actor User
participant Page as Workspace Page
participant WS as Services (Workspaces / TDEI)
participant Provide as provide('workspace')
participant Composable as useWorkspaceRole
participant UI as Component UI
User->>Page: open workspace/settings (or review/export) page
Page->>WS: fetch workspace + project-groups/users (parallel)
WS-->>Page: workspace + groups/users
Page->>Provide: provide('workspace', workspace)
UI->>Composable: useWorkspaceRole()
Composable->>Provide: inject('workspace')
Provide-->>Composable: workspace
Composable->>UI: { role, isLead, isValidator }
UI->>UI: enable/disable controls and show badges based on role flags
User->>UI: attempt action (save/assign/remove/export)
alt isLead / isValidator
UI->>Page: invoke action
Page->>WS: call service endpoint (assignRole/update/save/export)
WS-->>Page: success / error
Page-->>UI: result -> update UI / toast
else not authorized
UI-->>User: disabled controls / informational alert
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 7
🧹 Nitpick comments (6)
types/tdei.ts (1)
6-8: ReuseTdeiRoleAssignmentinTdeiProjectGrouptoo.
TdeiProjectGroup.rolesandTdeiUserItem.rolesrepresent the same concept, but only one side uses the alias. Making bothTdeiRoleAssignment[]keeps future narrowing/refactors in one place.Also applies to: 15-24
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@types/tdei.ts` around lines 6 - 8, Replace the plain string-array typing with the shared alias: change TdeiProjectGroup.roles (on the TdeiProjectGroup interface that extends TdeiProjectGroupItem) from string[] to TdeiRoleAssignment[] so it matches TdeiUserItem.roles; also update any other declarations in this file that define roles as string[] (the other project/group/user types) to use TdeiRoleAssignment[] to centralize role shape and enable single-point narrowing/refactors.composables/useWorkspaceRole.ts (2)
20-25: RenameisContributoror make it role-exact.Right now
isContributoris true forleadandvalidatortoo. In an access-control helper, that name is misleading and easy to gate the wrong UI with later. Either change it torole.value === 'contributor'or rename it to something likeisMember/hasWorkspaceRole.Possible direction
const role = computed<WorkspaceRole | undefined>(() => workspace?.role); const isLead = computed(() => role.value === 'lead'); const isValidator = computed(() => role.value === 'validator' || isLead.value); -const isContributor = computed(() => !!role.value); +const isContributor = computed(() => role.value === 'contributor'); return { role, isLead, isValidator, isContributor };If the current behavior is intentional, keep the implementation and rename the helper instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composables/useWorkspaceRole.ts` around lines 20 - 25, The name isContributor is misleading because it returns true for lead/validator; either make it role-exact by changing isContributor to computed(() => role.value === 'contributor') or, if the broad truthiness is intentional, rename the identifier (and its exports/usages) to a clearer name such as isMember or hasWorkspaceRole; update references to isContributor accordingly and ensure this aligns with the existing computed properties role, isLead and isValidator.
11-17: Create a sharedInjectionKey<Workspace>to replace the duplicated'workspace'string.The raw string
'workspace'is duplicated across 12 files (2 providers inpages/workspace/[id]/settings.vueandpages/workspace/[id]/review.vue, and 10 consumers throughoutcomponents/andcomposables/). A single exportedInjectionKeywill prevent silent failures from typos and provide type safety end-to-end.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composables/useWorkspaceRole.ts` around lines 11 - 17, Create and export a single typed InjectionKey for the workspace (e.g. export const WORKSPACE_KEY: InjectionKey<Workspace> = Symbol('workspace')) and replace all raw string usages with it: change inject<Workspace>('workspace') in composables like useWorkspaceRole.ts to inject(WORKSPACE_KEY) and update all providers (pages/workspace/[id]/settings.vue and pages/workspace/[id]/review.vue) to call provide(WORKSPACE_KEY, workspace). Import WORKSPACE_KEY where needed and keep the Workspace type the same so consumers/provide calls are type-safe and resilient to typos.components/dashboard/WorkspaceItem.vue (1)
13-24: UseROLE_LABELShere instead of duplicating the badge copy.These strings now duplicate the new role-label mapping in
util/roles.ts, so the dashboard badges can drift from the members/settings UI if wording changes.Possible direction
+import { ROLE_LABELS } from '~/util/roles'; + <span v-if="workspace.role === 'lead'" class="badge bg-dark ms-2" > - <app-icon variant="star" /> Owner + <app-icon variant="star" /> {{ ROLE_LABELS.lead }} </span> <span v-else-if="workspace.role === 'validator'" class="badge bg-dark ms-2" > - <app-icon variant="task_alt" /> Validator + <app-icon variant="task_alt" /> {{ ROLE_LABELS.validator }} </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/dashboard/WorkspaceItem.vue` around lines 13 - 24, Replace the hardcoded badge text in WorkspaceItem.vue with the canonical labels from the ROLE_LABELS mapping: import ROLE_LABELS from util/roles and use ROLE_LABELS[workspace.role] when rendering the badge (still conditionally showing the badge based on workspace.role like in the existing v-if / v-else-if logic, or refactor to a single conditional that checks if ROLE_LABELS[workspace.role] exists). This ensures the displayed label is pulled from the single source of truth (ROLE_LABELS) rather than duplicated strings.services/tdei.ts (1)
485-493: Missingawaitbeforeresponse.json().For consistency with other methods in this class (e.g.,
getMyProjectGroups), explicitly await the JSON parsing.♻️ Proposed fix for consistency
async getProjectGroupUsers(projectGroupId: string): Promise<TdeiUserItem[]> { const params = new URLSearchParams(); params.append('page_no', '1'); params.append('page_size', '10000'); const response = await this._get(`project-group/${projectGroupId}/users?${params}`); - return response.json(); + return await response.json(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/tdei.ts` around lines 485 - 493, In getProjectGroupUsers, the code returns response.json() without awaiting it; change to await the JSON parsing so the method returns the resolved TdeiUserItem[] (i.e., replace the final return with awaiting response.json()). Update the return statement in getProjectGroupUsers to use await response.json() for consistency with other methods like getMyProjectGroups.pages/dashboard.vue (1)
126-139: Non-null assertion oncurrentWorkspace.valuecould be fragile.Line 138 uses
currentWorkspace.value!.center = center. While this is currently safe becauseonCenterLoadedis emitted from a component inside thev-if="currentWorkspace"block, a future refactor could introduce a race condition if the workspace is deselected while the map is still emitting events.Consider a defensive guard:
🛡️ Proposed defensive check
function onCenterLoaded(center) { - currentWorkspace.value!.center = center; + if (currentWorkspace.value) { + currentWorkspace.value.center = center; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/dashboard.vue` around lines 126 - 139, The assignment in onCenterLoaded uses a non-null assertion on currentWorkspace.value which can be brittle; update onCenterLoaded to defensively check that currentWorkspace.value exists (and optionally that it matches the expected workspace or has a center property) before assigning center, e.g. early-return if currentWorkspace.value is undefined, to avoid a race when the map emits after deselection; reference currentWorkspace.value and onCenterLoaded (and if relevant, selectWorkspace/onCurrentWorkspacesChange) so you locate the handler and replace the non-null assertion with a safe guard or optional chaining assignment.
🤖 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/dashboard/DetailsTable.vue`:
- Line 80: The import in DetailsTable.vue should use the extensionless module
path to match the codebase convention: change the statement importing Workspace
(the import that currently reads "import type { Workspace } from
'~/types/workspaces.ts'") to import from '~/types/workspaces' so other modules
resolve consistently.
In `@components/review/Toolbar.vue`:
- Around line 48-52: The native disabled button prevents the title tooltip from
showing; update the Toolbar.vue rendering so the explanatory tooltip/label is
attached to a non-disabled wrapper (e.g., a span or div) surrounding the button
or render an inline explanation when props.item.isFeedback is true and
isValidator is false; specifically, keep the button with
:disabled="!isValidator" but move the :title and tooltip logic off the button
and onto the wrapper (or add an aria-describedby/tooltip element) so users can
see "Only validators and owners can resolve feedback" even when the button is
disabled.
In `@components/settings/panel/Delete.vue`:
- Around line 22-25: The confirm button and handlers need an isLead guard:
update the confirmation button's disabled binding so it also checks isLead
(e.g., disabled when !isLead or when accepted is false) and add early-return
checks at the top of both acceptDelete and submitDelete methods to short-circuit
if !isLead; this ensures the UI button is disabled and the methods cannot be
invoked when the user is not a lead. Refer to the Delete.vue component, the
acceptDelete and submitDelete methods, and the initial acknowledgment and final
confirm button elements when making these changes.
In `@components/settings/panel/Imagery.vue`:
- Around line 61-64: The save flow in Imagery.vue currently calls
updateWorkspace() without awaiting it, causing the UI to show a success state
even if the request fails; update the handler that runs when the submit button
(type="submit", :disabled="!isLead") is clicked to await updateWorkspace() and
wrap it in try/catch so the success state is only set after await resolves and
errors are handled in the catch; apply the same fix to the other identical save
block referenced (lines ~128-133) so both submit handlers await
updateWorkspace() and only set "Changes saved" after a successful await.
In `@pages/workspace/`[id]/settings/members.vue:
- Around line 206-219: The variable accessDenied is a plain let and not reactive
so template/computed won't update when it's set; change accessDenied to a Vue
ref (e.g., const accessDenied = ref(false)) and assign to accessDenied.value in
the catch block where you check for TdeiUserClientError and e.response.status
=== 403, and update any template or computed references to use
accessDenied.value (also ensure projectGroupUsers remains reactive if needed or
convert to ref/ reactive similarly if the template depends on it); keep the
try/catch around
tdeiUserClient.getProjectGroupUsers(workspace.tdeiProjectGroupId) and only set
accessDenied.value = true on the 403 path.
- Around line 221-243: localRoleMap is populated with workspace member auth_uids
but MemberEntry only stores TDEI user_id, so lookup and subsequent API calls
fail; fix by building a join between projectGroupUsers and workspace members
(e.g., match on email or another shared field) when creating members: call
workspacesClient.getWorkspaceMembers(...) to build a map from a shared
identifier (email) to auth_uid and role, then when mapping projectGroupUsers
include auth_uid in each MemberEntry and set localRole using
localRoleMap.get(auth_uid); finally, update usages of member.user_id in
setUserRole/removeUserRole to pass member.auth_uid so the API receives the
correct identifier (if no shared field exists, add a small resolver call to
translate user_id -> auth_uid before populating MemberEntry).
In `@services/tdei.ts`:
- Around line 470-476: The response JSON in getMyRolesForProjectGroup is being
cast to TdeiProjectGroup[] but the API returns project_group_name (not name);
update getMyRolesForProjectGroup to parse the raw response (e.g., any[] or a
dedicated DTO) and map each item to TdeiProjectGroup by copying
tdei_project_group_id and roles and mapping project_group_name -> name before
running find(), so references to TdeiProjectGroup.name remain correct; adjust
the local variable (currently pgs) to hold the mapped array.
---
Nitpick comments:
In `@components/dashboard/WorkspaceItem.vue`:
- Around line 13-24: Replace the hardcoded badge text in WorkspaceItem.vue with
the canonical labels from the ROLE_LABELS mapping: import ROLE_LABELS from
util/roles and use ROLE_LABELS[workspace.role] when rendering the badge (still
conditionally showing the badge based on workspace.role like in the existing
v-if / v-else-if logic, or refactor to a single conditional that checks if
ROLE_LABELS[workspace.role] exists). This ensures the displayed label is pulled
from the single source of truth (ROLE_LABELS) rather than duplicated strings.
In `@composables/useWorkspaceRole.ts`:
- Around line 20-25: The name isContributor is misleading because it returns
true for lead/validator; either make it role-exact by changing isContributor to
computed(() => role.value === 'contributor') or, if the broad truthiness is
intentional, rename the identifier (and its exports/usages) to a clearer name
such as isMember or hasWorkspaceRole; update references to isContributor
accordingly and ensure this aligns with the existing computed properties role,
isLead and isValidator.
- Around line 11-17: Create and export a single typed InjectionKey for the
workspace (e.g. export const WORKSPACE_KEY: InjectionKey<Workspace> =
Symbol('workspace')) and replace all raw string usages with it: change
inject<Workspace>('workspace') in composables like useWorkspaceRole.ts to
inject(WORKSPACE_KEY) and update all providers
(pages/workspace/[id]/settings.vue and pages/workspace/[id]/review.vue) to call
provide(WORKSPACE_KEY, workspace). Import WORKSPACE_KEY where needed and keep
the Workspace type the same so consumers/provide calls are type-safe and
resilient to typos.
In `@pages/dashboard.vue`:
- Around line 126-139: The assignment in onCenterLoaded uses a non-null
assertion on currentWorkspace.value which can be brittle; update onCenterLoaded
to defensively check that currentWorkspace.value exists (and optionally that it
matches the expected workspace or has a center property) before assigning
center, e.g. early-return if currentWorkspace.value is undefined, to avoid a
race when the map emits after deselection; reference currentWorkspace.value and
onCenterLoaded (and if relevant, selectWorkspace/onCurrentWorkspacesChange) so
you locate the handler and replace the non-null assertion with a safe guard or
optional chaining assignment.
In `@services/tdei.ts`:
- Around line 485-493: In getProjectGroupUsers, the code returns response.json()
without awaiting it; change to await the JSON parsing so the method returns the
resolved TdeiUserItem[] (i.e., replace the final return with awaiting
response.json()). Update the return statement in getProjectGroupUsers to use
await response.json() for consistency with other methods like
getMyProjectGroups.
In `@types/tdei.ts`:
- Around line 6-8: Replace the plain string-array typing with the shared alias:
change TdeiProjectGroup.roles (on the TdeiProjectGroup interface that extends
TdeiProjectGroupItem) from string[] to TdeiRoleAssignment[] so it matches
TdeiUserItem.roles; also update any other declarations in this file that define
roles as string[] (the other project/group/user types) to use
TdeiRoleAssignment[] to centralize role shape and enable single-point
narrowing/refactors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 26cdf998-ae76-4820-81bf-a16cef856472
📒 Files selected for processing (22)
components/ProjectGroupPicker.vuecomponents/dashboard/DetailsTable.vuecomponents/dashboard/Toolbar.vuecomponents/dashboard/WorkspaceItem.vuecomponents/review/Toolbar.vuecomponents/settings/Nav.vuecomponents/settings/panel/Apps.vuecomponents/settings/panel/Delete.vuecomponents/settings/panel/General.vuecomponents/settings/panel/Imagery.vuecomponents/teams/Item.vuecomposables/useWorkspaceRole.tspages/dashboard.vuepages/workspace/[id]/export/tdei.vuepages/workspace/[id]/review.vuepages/workspace/[id]/settings/members.vuepages/workspace/[id]/settings/teams/index.vueservices/tdei.tsservices/workspaces.tstypes/tdei.tstypes/workspaces.tsutil/roles.ts
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
pages/workspace/[id]/settings/members.vue (1)
206-219: Consider usingreffor consistency, though current usage works.
accessDeniedis declared withlet(line 207), which makes it non-reactive. However, since it's set synchronously during the top-levelawaitbefore the template renders, the current implementation functions correctly.For consistency with Vue 3 patterns and to avoid confusion if future changes require reactivity, consider using
ref:♻️ Optional refactor for consistency
-let projectGroupUsers: TdeiUserItem[] = []; -let accessDenied = false; +const projectGroupUsers = ref<TdeiUserItem[]>([]); +const accessDenied = ref(false); try { - projectGroupUsers = await tdeiUserClient.getProjectGroupUsers(workspace.tdeiProjectGroupId); + projectGroupUsers.value = await tdeiUserClient.getProjectGroupUsers(workspace.tdeiProjectGroupId); } catch (e) { if (e instanceof TdeiUserClientError && e.response.status === 403) { - accessDenied = true; + accessDenied.value = true; }Then update template references to
accessDeniedand usages ofprojectGroupUsersaccordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/workspace/`[id]/settings/members.vue around lines 206 - 219, Replace the plain let bindings with Vue refs to make the state reactive: change accessDenied to a ref (e.g., accessDenied = ref(false)) and optionally convert projectGroupUsers to a ref (e.g., projectGroupUsers = ref<TdeiUserItem[]>([])), then update the try/catch that calls tdeiUserClient.getProjectGroupUsers(workspace.tdeiProjectGroupId) to assign into .value (projectGroupUsers.value = ...) and set accessDenied.value = true when catching the TdeiUserClientError with response.status === 403; also update any template and usages to read the .value (or use unref) so reactivity is consistent with Vue 3 patterns.
🤖 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/dashboard/DetailsTable.vue`:
- Around line 80-84: The prop Workspace type is inconsistent:
workspace.tdeiMetadata is declared as string in types/workspaces.ts but used as
a nested object in DetailsTable.vue (accessing
workspace.tdeiMetadata?.metadata?.dataset_detail?.version); fix by either
updating the Workspace.tdeiMetadata type to a proper object/interface (e.g.,
define and export a TdeiMetadata type with metadata.dataset_detail.version and
use it in types/workspaces.ts and the Props interface) or, if the backend truly
returns a JSON string, parse it once in DetailsTable.vue (create a computed
property like parsedTdei = workspace.tdeiMetadata ?
JSON.parse(workspace.tdeiMetadata) : undefined and use parsedTdei.metadata... in
the template) and update the Props type to string accordingly so the type and
usage match.
- Around line 28-33: The template currently collapses missing workspace.role
into "Member" via a v-else branch; update the conditional to explicitly check
for workspace.role === 'contributor' (render the <span class="badge bg-secondary
text-uppercase"> with <app-icon variant="person" /> Member only for that case)
and change the former v-else branch to render a neutral/fallback (empty or a
non-assertive placeholder) when workspace.role is undefined or null so you don't
display a role that wasn't provided.
In `@components/ProjectGroupPicker.vue`:
- Around line 25-26: projectGroups is being assigned once using props.options or
an awaited tdeiUserClient.getMyProjectGroups() call without error handling and
not reactive to props changes; change projectGroups to a computed that returns
props.options if present else loads the API result (use an internal async loader
or a reactive ref for the fetched list) and wrap the API call
tdeiUserClient.getMyProjectGroups() in a try-catch to handle errors (log or set
an empty array/fallback) and then sort the result with compareStringAsc(g =>
g.name); ensure you reference the computed projectGroups wherever used so
updates to props.options or the fetched data reactively update the component.
In `@components/settings/panel/Delete.vue`:
- Around line 40-43: Add a "pending" flag to guard the destructive delete flow:
update the button disabled binding to include pending (e.g., :disabled="!isLead
|| attestation !== 'delete' || pending") and modify submitDelete to return early
if pending is true, set pending = true before calling deleteWorkspace(), wrap
the call in try/catch/finally, navigate only on success, surface the caught
error to the user (e.g., set an error message or emit a toast) and set pending =
false in finally so the button is re-enabled after failure; reference the
existing submitDelete method, the deleteWorkspace call inside it, and the
isLead/attestation button bindings when making changes.
In `@components/teams/Item.vue`:
- Around line 25-26: The lead-only explanation is currently only in the button
title and becomes inaccessible when the button is disabled; update the UI so the
explanatory hint is discoverable and keyboard-accessible: wrap the button(s)
that use props.isLead (the rename/delete team controls) in a focusable wrapper
(e.g., a span/div with tabindex="0") and move the tooltip/title to that wrapper
(or render a small inline hint text when !props.isLead), ensure the wrapper
remains focusable/hoverable and preserves the button semantics (keep :disabled
on the button but add aria-disabled on the wrapper if needed), and update any
places referencing props.isLead for both the rename control and the delete
control to use this pattern so non-leads can discover the message.
---
Nitpick comments:
In `@pages/workspace/`[id]/settings/members.vue:
- Around line 206-219: Replace the plain let bindings with Vue refs to make the
state reactive: change accessDenied to a ref (e.g., accessDenied = ref(false))
and optionally convert projectGroupUsers to a ref (e.g., projectGroupUsers =
ref<TdeiUserItem[]>([])), then update the try/catch that calls
tdeiUserClient.getProjectGroupUsers(workspace.tdeiProjectGroupId) to assign into
.value (projectGroupUsers.value = ...) and set accessDenied.value = true when
catching the TdeiUserClientError with response.status === 403; also update any
template and usages to read the .value (or use unref) so reactivity is
consistent with Vue 3 patterns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 19af7f40-51eb-4767-9f22-69f2559c667c
📒 Files selected for processing (22)
components/ProjectGroupPicker.vuecomponents/dashboard/DetailsTable.vuecomponents/dashboard/Toolbar.vuecomponents/dashboard/WorkspaceItem.vuecomponents/review/Toolbar.vuecomponents/settings/Nav.vuecomponents/settings/panel/Apps.vuecomponents/settings/panel/Delete.vuecomponents/settings/panel/General.vuecomponents/settings/panel/Imagery.vuecomponents/teams/Item.vuecomposables/useWorkspaceRole.tspages/dashboard.vuepages/workspace/[id]/export/tdei.vuepages/workspace/[id]/review.vuepages/workspace/[id]/settings/members.vuepages/workspace/[id]/settings/teams/index.vueservices/tdei.tsservices/workspaces.tstypes/tdei.tstypes/workspaces.tsutil/roles.ts
✅ Files skipped from review due to trivial changes (1)
- components/dashboard/Toolbar.vue
🚧 Files skipped from review as they are similar to previous changes (7)
- util/roles.ts
- components/review/Toolbar.vue
- pages/workspace/[id]/review.vue
- types/tdei.ts
- pages/workspace/[id]/export/tdei.vue
- composables/useWorkspaceRole.ts
- components/settings/Nav.vue
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/workspace/[id]/export/tdei.vue (1)
165-165:⚠️ Potential issue | 🟡 MinorFix typo: "sucessfully" → "successfully"
Minor typo in the toast message.
✏️ Proposed fix
- toast.info(`TDEI import job ${jobId} created sucessfully.`); + toast.info(`TDEI import job ${jobId} created successfully.`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/workspace/`[id]/export/tdei.vue at line 165, Typo in the toast message: update the toast.info call that constructs the TDEI job creation message (the expression using jobId) to replace "sucessfully" with the correct spelling "successfully" so the user-facing notification reads "TDEI import job ${jobId} created successfully.".
🧹 Nitpick comments (2)
services/tdei.ts (1)
470-480: Minor: Consider simplifying the search approach.The method uses
searchText: pgNameto filter results but then finds byprojectGroupId. If the API supports searching directly by project group ID, that would be more efficient and avoid potential edge cases where name-based search returns unexpected results.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/tdei.ts` around lines 470 - 480, getMyRolesForProjectGroup currently searches by name (pgName) via searchText then filters by projectGroupId, which is inefficient and brittle; change the request to query or path the API using the projectGroupId directly (use projectGroupId instead of pgName in the URLSearchParams or call project-group-roles/{projectGroupId}/{this.#auth.subject} if the API supports it) so the server returns only the targeted group; update the URL construction in getMyRolesForProjectGroup and remove the follow-up .find by id (keep the final roles fallback logic) to simplify and ensure correct results.components/ProjectGroupPicker.vue (1)
25-34: Consider adding error handling for the API call.If
tdeiUserClient.getMyProjectGroups()fails, the component will throw and the error will propagate to the nearest Suspense boundary. While this may be acceptable for your error handling strategy, you might want to catch the error and display a user-friendly message or fallback UI.The auto-selection logic (lines 28-34) is well-implemented, correctly defaulting to the first project group when the current model value is missing or invalid.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ProjectGroupPicker.vue` around lines 25 - 34, Wrap the call to tdeiUserClient.getMyProjectGroups() in a try/catch so API failures don’t throw into the Suspense boundary; if the call fails, fall back to props.options or an empty array for the projectGroups variable, set an error flag/message in component state (e.g. projectGroupsError) and render a user-friendly fallback UI or message, and ensure the existing auto-selection logic that updates model.value (referencing projectGroups and model.value) still runs against the fallback array; also consider emitting an error event if upstream handling is required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@composables/useWorkspaceRole.ts`:
- Around line 20-23: The computed flag isContributor misclassifies members
because it uses !!role.value; update the isContributor computed getter
(alongside role, isLead and isValidator) so that workspace.role === undefined is
treated as a contributor (e.g., return true when role.value is undefined or
equals 'contributor'), leaving isLead and isValidator logic unchanged.
In `@services/tdei.ts`:
- Around line 489-497: The getProjectGroupUsers method returns response.json()
without validating or mapping to TdeiUserItem; update getProjectGroupUsers (and
its use of this._get) to parse the JSON, verify required fields (user_id,
first_name, last_name, email, username, phone, roles) and types for each item,
and map/normalize fields into the same shape produced by
getMyProjectGroups/getMyServices in TdeiUserClient (or throw a clear error if
any item is missing/invalid). Follow the existing validation/mapping pattern
used by getMyProjectGroups/getMyServices: iterate the response array, validate
each record's fields and types, construct TdeiUserItem objects, and return the
mapped array. Ensure the method name getProjectGroupUsers and type TdeiUserItem
are the targets for your changes and reuse existing helper/validator logic if
present.
---
Outside diff comments:
In `@pages/workspace/`[id]/export/tdei.vue:
- Line 165: Typo in the toast message: update the toast.info call that
constructs the TDEI job creation message (the expression using jobId) to replace
"sucessfully" with the correct spelling "successfully" so the user-facing
notification reads "TDEI import job ${jobId} created successfully.".
---
Nitpick comments:
In `@components/ProjectGroupPicker.vue`:
- Around line 25-34: Wrap the call to tdeiUserClient.getMyProjectGroups() in a
try/catch so API failures don’t throw into the Suspense boundary; if the call
fails, fall back to props.options or an empty array for the projectGroups
variable, set an error flag/message in component state (e.g. projectGroupsError)
and render a user-friendly fallback UI or message, and ensure the existing
auto-selection logic that updates model.value (referencing projectGroups and
model.value) still runs against the fallback array; also consider emitting an
error event if upstream handling is required.
In `@services/tdei.ts`:
- Around line 470-480: getMyRolesForProjectGroup currently searches by name
(pgName) via searchText then filters by projectGroupId, which is inefficient and
brittle; change the request to query or path the API using the projectGroupId
directly (use projectGroupId instead of pgName in the URLSearchParams or call
project-group-roles/{projectGroupId}/{this.#auth.subject} if the API supports
it) so the server returns only the targeted group; update the URL construction
in getMyRolesForProjectGroup and remove the follow-up .find by id (keep the
final roles fallback logic) to simplify and ensure correct results.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 44e5140c-0f73-45ac-9295-e2bd79620bc6
📒 Files selected for processing (23)
components/ProjectGroupPicker.vuecomponents/dashboard/DetailsTable.vuecomponents/dashboard/Toolbar.vuecomponents/dashboard/WorkspaceItem.vuecomponents/review/Toolbar.vuecomponents/settings/Nav.vuecomponents/settings/panel/Apps.vuecomponents/settings/panel/Delete.vuecomponents/settings/panel/General.vuecomponents/settings/panel/Imagery.vuecomponents/teams/Item.vuecomponents/teams/MembersDialog.vuecomposables/useWorkspaceRole.tspages/dashboard.vuepages/workspace/[id]/export/tdei.vuepages/workspace/[id]/review.vuepages/workspace/[id]/settings/members.vuepages/workspace/[id]/settings/teams/index.vueservices/tdei.tsservices/workspaces.tstypes/tdei.tstypes/workspaces.tsutil/roles.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- components/review/Toolbar.vue
- services/workspaces.ts
- components/dashboard/WorkspaceItem.vue
- components/settings/panel/Delete.vue
- components/settings/Nav.vue
- types/tdei.ts
- types/workspaces.ts
- components/dashboard/DetailsTable.vue
- components/settings/panel/Apps.vue
- components/settings/panel/General.vue
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
pages/dashboard.vue (1)
69-69: Remove the SSR runtime concern; browser compatibility depends on your target browser set.This page uses
Map.groupBy(), an ES2024 API newly available in modern browsers (Chrome/Edge 117+, Firefox 119+, Safari 17.4+). Since yournuxt.config.tsexplicitly disables SSR (ssr: false), there is no Node.js runtime concern. However, without an explicit.browserslistrcor browser target configuration, it's unclear whether your supported browsers include older versions (e.g., older Safari, IE) that don't supportMap.groupBy().If you need to support older browsers, consider this compatibility-safe alternative:
⬇️ Compatibility-safe alternative
-const workspacesByProjectGroup = Map.groupBy(workspaces, w => w.tdeiProjectGroupId); +const workspacesByProjectGroup = workspaces.reduce((groups, workspace) => { + const groupId = workspace.tdeiProjectGroupId; + const group = groups.get(groupId); + + if (group) { + group.push(workspace); + } + else { + groups.set(groupId, [workspace]); + } + + return groups; +}, new Map<string, Workspace[]>());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/dashboard.vue` at line 69, The use of the ES2024 API Map.groupBy for computing workspacesByProjectGroup may not be supported in some target browsers; either add explicit browser targets (create/update .browserslistrc or adjust build target in nuxt.config.ts) to guarantee Map.groupBy availability, or replace Map.groupBy(workspaces, w => w.tdeiProjectGroupId) with a compatibility-safe grouping implementation (e.g., iterate over the workspaces array and build a Map keyed by tdeiProjectGroupId using Array.prototype.reduce or a simple forEach) so the grouping logic (workspaces, workspacesByProjectGroup) works reliably across your supported browsers.
🤖 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/review/Toolbar.vue`:
- Around line 55-65: The "Mark as Resolved" button is missing its click handler
and emit; add an `@click` on the button (e.g. `@click`="markAsResolved") and
implement a corresponding method (markAsResolved) that emits an event to the
parent with the item (e.g. this.$emit('resolve', props.item) for Options API or
const emit = defineEmits(['resolve']); emit('resolve', props.item) in
setup/Composition API), following the same naming/emit pattern used by other
buttons in the component.
In `@pages/dashboard.vue`:
- Around line 64-68: The roles lookup can reject the top-level Promise.all and
prevent the dashboard from rendering; change the
tdeiUserClient.getMyProjectGroups() call inside Promise.all to gracefully handle
errors (e.g., append a .catch(() => []) or equivalent) so it returns an empty
array on failure, then build rolesByProjectGroup from that safe result (using
myProjectGroups.map(...) as before) so the page renders with an empty roles map
instead of blank-screening; keep workspacesClient.getMyWorkspaces().then(ws =>
ws.sort(compareWorkspaceCreatedAtDesc)) unchanged.
In `@pages/workspace/`[id]/export/tdei.vue:
- Around line 137-140: When auto-falling back to the first eligible project
group in the branch that sets workspace.tdeiProjectGroupId (the if checking
canExport && !eligibleProjectGroups.some(...)), also clear any saved service
selection by setting workspace.tdeiServiceId to null/undefined so a stale
service id from the previous group won't be submitted by upload(); update the
same block that assigns workspace.tdeiProjectGroupId to also reset
workspace.tdeiServiceId and ensure upload() continues to send the updated
fields.
In `@services/tdei.ts`:
- Around line 470-479: getMyRolesForProjectGroup is calling the paginated
endpoint project-group-roles with only searchText, which can be truncated by the
backend; change it to paginate instead of single-requesting: call
project-group-roles with page_no/page_size (or loop requests starting at
page_no=1) and fetch subsequent pages until you find the entry where
tdei_project_group_id === projectGroupId or no more results, then return its
roles (mirroring how getMyProjectGroups handles pagination) so you don’t
incorrectly return [] when the group is on a different page.
---
Nitpick comments:
In `@pages/dashboard.vue`:
- Line 69: The use of the ES2024 API Map.groupBy for computing
workspacesByProjectGroup may not be supported in some target browsers; either
add explicit browser targets (create/update .browserslistrc or adjust build
target in nuxt.config.ts) to guarantee Map.groupBy availability, or replace
Map.groupBy(workspaces, w => w.tdeiProjectGroupId) with a compatibility-safe
grouping implementation (e.g., iterate over the workspaces array and build a Map
keyed by tdeiProjectGroupId using Array.prototype.reduce or a simple forEach) so
the grouping logic (workspaces, workspacesByProjectGroup) works reliably across
your supported browsers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f5c6cfc-b0a3-439f-8ec8-51ae9634391d
📒 Files selected for processing (23)
components/ProjectGroupPicker.vuecomponents/dashboard/DetailsTable.vuecomponents/dashboard/Toolbar.vuecomponents/dashboard/WorkspaceItem.vuecomponents/review/Toolbar.vuecomponents/settings/Nav.vuecomponents/settings/panel/Apps.vuecomponents/settings/panel/Delete.vuecomponents/settings/panel/General.vuecomponents/settings/panel/Imagery.vuecomponents/teams/Item.vuecomponents/teams/MembersDialog.vuecomposables/useWorkspaceRole.tspages/dashboard.vuepages/workspace/[id]/export/tdei.vuepages/workspace/[id]/review.vuepages/workspace/[id]/settings/members.vuepages/workspace/[id]/settings/teams/index.vueservices/tdei.tsservices/workspaces.tstypes/tdei.tstypes/workspaces.tsutil/roles.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- components/settings/panel/General.vue
- util/roles.ts
- components/dashboard/WorkspaceItem.vue
- components/settings/panel/Apps.vue
- components/dashboard/Toolbar.vue
- components/teams/Item.vue
- pages/workspace/[id]/settings/teams/index.vue
- types/workspaces.ts
- components/teams/MembersDialog.vue
- components/settings/panel/Delete.vue
- pages/workspace/[id]/review.vue
- components/settings/Nav.vue
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
components/settings/panel/Apps.vue (1)
209-211: Consider adding an early guard for defense-in-depth.While the submit button is disabled for non-leads and backend authorization should enforce this, adding an explicit check at the function entry prevents accidental or programmatic form submission.
🛡️ Proposed guard
async function saveExternalAppConfiguration() { + if (!isLead.value) return; + clearExternalAppMessages();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/settings/panel/Apps.vue` around lines 209 - 211, Add a defense-in-depth early guard at the top of saveExternalAppConfiguration: check the current user's lead/permission flag (e.g., isLead, is_admin, or the same prop used to disable the submit button) and immediately return (and optionally set an error message via clearExternalAppMessages()/setExternalAppError) if the user is not authorized; update saveExternalAppConfiguration to perform this check before any other actions so programmatic or accidental submissions are blocked even if the UI is bypassed.
🤖 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/settings/Nav.vue`:
- Around line 9-14: Members nav link is visible to all users but should be shown
only to leads; wrap the <settings-nav-link> with a conditional that checks the
workspace lead flag (use the existing isLead boolean/computed) so the Members
link is rendered only when isLead is true (i.e., add a v-if or equivalent around
the settings-nav-link in Nav.vue that uses isLead).
In `@pages/dashboard.vue`:
- Around line 71-72: The cached ID variables should allow undefined/null to
match runtime assignments: update the declarations for lastProjectGroupId and
lastWorkspaceId to be nullable (e.g., string | undefined or string | null) and,
if needed, make currentProjectGroup and currentWorkspace refs explicitly
nullable (e.g., ref<string | undefined>() and ref<Workspace | undefined>()) so
that assignments like val?.id and val compile cleanly and the subsequent truthy
checks remain correct; locate and update the declarations of lastProjectGroupId,
lastWorkspaceId, currentProjectGroup, and currentWorkspace accordingly.
In `@services/tdei.ts`:
- Around line 450-451: The clone method on TdeiUserClient currently returns a
new TdeiUserClient but reuses the original private `#tdeiClient`, which keeps the
original AbortSignal; update clone(signal?: AbortSignal): TdeiUserClient to also
clone the nested TdeiClient (e.g., const clonedTdei =
this.#tdeiClient.clone(signal ?? this._abortSignal)) and pass that cloned
instance into the new TdeiUserClient constructor (new
TdeiUserClient(this._baseUrl, clonedTdei, signal ?? this._abortSignal)) so that
_send() and auth refreshes use the cloned client's signal rather than the
original.
- Around line 5-9: The import line using the explicit .ts extension causes
TypeScript to fail; remove the file extension so the import reads from
'~/types/tdei' instead of '~/types/tdei.ts' (update the import that brings in
TdeiFeedback, TdeiProjectGroup, TdeiUserItem) to match other extensionless
imports like '~/services/http' and '~/services/loading'.
---
Nitpick comments:
In `@components/settings/panel/Apps.vue`:
- Around line 209-211: Add a defense-in-depth early guard at the top of
saveExternalAppConfiguration: check the current user's lead/permission flag
(e.g., isLead, is_admin, or the same prop used to disable the submit button) and
immediately return (and optionally set an error message via
clearExternalAppMessages()/setExternalAppError) if the user is not authorized;
update saveExternalAppConfiguration to perform this check before any other
actions so programmatic or accidental submissions are blocked even if the UI is
bypassed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc7adaa7-25eb-41a9-88a1-11eafc83b8b3
📒 Files selected for processing (23)
components/ProjectGroupPicker.vuecomponents/dashboard/DetailsTable.vuecomponents/dashboard/Toolbar.vuecomponents/dashboard/WorkspaceItem.vuecomponents/review/Toolbar.vuecomponents/settings/Nav.vuecomponents/settings/panel/Apps.vuecomponents/settings/panel/Delete.vuecomponents/settings/panel/General.vuecomponents/settings/panel/Imagery.vuecomponents/teams/Item.vuecomponents/teams/MembersDialog.vuecomposables/useWorkspaceRole.tspages/dashboard.vuepages/workspace/[id]/export/tdei.vuepages/workspace/[id]/review.vuepages/workspace/[id]/settings/members.vuepages/workspace/[id]/settings/teams/index.vueservices/tdei.tsservices/workspaces.tstypes/tdei.tstypes/workspaces.tsutil/roles.ts
✅ Files skipped from review due to trivial changes (1)
- components/dashboard/Toolbar.vue
🚧 Files skipped from review as they are similar to previous changes (8)
- components/teams/MembersDialog.vue
- pages/workspace/[id]/settings/teams/index.vue
- pages/workspace/[id]/settings/members.vue
- util/roles.ts
- types/tdei.ts
- composables/useWorkspaceRole.ts
- pages/workspace/[id]/review.vue
- components/dashboard/DetailsTable.vue
Comment on lines
+9
to
+14
| <settings-nav-link | ||
| to="/members" | ||
| icon="admin_panel_settings" | ||
| > | ||
| Members | ||
| </settings-nav-link> |
There was a problem hiding this comment.
Gate the Members nav item to leads.
This link is currently visible to everyone, but the destination page only loads workspace member-role data for leads (pages/workspace/[id]/settings/members.vue:203-228). Non-leads can still reach the page and only get an informational alert (pages/workspace/[id]/settings/members.vue:1-12), so this adds a dead-end/degraded settings entry for users who cannot use it. Please hide the link behind isLead.
Suggested fix
<template>
<nav class="list-group mb-4">
<settings-nav-link
to=""
icon="settings"
>
General
</settings-nav-link>
- <settings-nav-link
+ <settings-nav-link
+ v-if="isLead"
to="/members"
icon="admin_panel_settings"
>
Members
</settings-nav-link>
<settings-nav-link
to="/teams"
icon="diversity_3"
>
Teams
</settings-nav-link>
</nav>
</template>
<script setup lang="ts">
+const { isLead } = useWorkspaceRole();
</script>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/settings/Nav.vue` around lines 9 - 14, Members nav link is visible
to all users but should be shown only to leads; wrap the <settings-nav-link>
with a conditional that checks the workspace lead flag (use the existing isLead
boolean/computed) so the Members link is rendered only when isLead is true
(i.e., add a v-if or equivalent around the settings-nav-link in Nav.vue that
uses isLead).
Comment on lines
+71
to
+72
| const currentProjectGroup = ref<string>(); | ||
| const currentWorkspace = ref<Workspace>(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
printf '== dashboard snippet ==\n'
sed -n '52,100p' pages/dashboard.vue
printf '\n== tsconfig files ==\n'
fd 'tsconfig.*json$' . -x sh -c 'echo "\n--- $1 ---"; sed -n "1,220p" "$1"'
printf '\n== strict nullability settings ==\n'
rg -n '"strict"|"strictNullChecks"' .Repository: TaskarCenterAtUW/workspaces-frontend
Length of output: 1892
🏁 Script executed:
#!/bin/bash
# Find tsconfig more carefully
find . -name 'tsconfig*.json' -type f 2>/dev/null | head -5
# Check each config file's strict settings
for f in $(find . -name 'tsconfig*.json' -type f 2>/dev/null | head -5); do
echo "=== $f ==="
cat "$f" | jq '.compilerOptions | {strict, strictNullChecks}' 2>/dev/null || cat "$f"
done
# Search for usage of lastWorkspaceId and lastProjectGroupId
echo -e "\n=== Usage of lastWorkspaceId and lastProjectGroupId ==="
rg 'lastWorkspaceId|lastProjectGroupId' pages/dashboard.vue -n -A 2 -B 1Repository: TaskarCenterAtUW/workspaces-frontend
Length of output: 940
Make the cached IDs nullable to match their runtime usage.
The watchers at lines 91–92 assign val?.id and val (both potentially undefined) into lastProjectGroupId and lastWorkspaceId, which are then checked for truthiness at lines 111 and 121 before being used. The types should reflect this:
🛠️ Minimal fix
-let lastProjectGroupId: string;
-let lastWorkspaceId: number;
+let lastProjectGroupId: string | undefined;
+let lastWorkspaceId: number | undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pages/dashboard.vue` around lines 71 - 72, The cached ID variables should
allow undefined/null to match runtime assignments: update the declarations for
lastProjectGroupId and lastWorkspaceId to be nullable (e.g., string | undefined
or string | null) and, if needed, make currentProjectGroup and currentWorkspace
refs explicitly nullable (e.g., ref<string | undefined>() and ref<Workspace |
undefined>()) so that assignments like val?.id and val compile cleanly and the
subsequent truthy checks remain correct; locate and update the declarations of
lastProjectGroupId, lastWorkspaceId, currentProjectGroup, and currentWorkspace
accordingly.
Comment on lines
+5
to
+9
| import type { | ||
| TdeiFeedback, | ||
| TdeiProjectGroup, | ||
| TdeiUserItem, | ||
| } from '~/types/tdei.ts'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
printf '== tsconfig files ==\n'
fd 'tsconfig.*json$' . -x sh -c 'echo "\n--- $1 ---"; sed -n "1,220p" "$1"'
printf '\n== explicit .ts imports in source ==\n'
rg -n --type=ts --type=vue "from ['\"][^'\"]+\\.ts['\"]"
printf '\n== TypeScript settings related to .ts imports ==\n'
rg -n '"allowImportingTsExtensions"|"moduleResolution"|"noEmit"|"emitDeclarationOnly"' .Repository: TaskarCenterAtUW/workspaces-frontend
Length of output: 341
🏁 Script executed:
#!/bin/bash
echo "=== Repository structure ==="
ls -la | head -20
echo -e "\n=== Finding tsconfig files ==="
find . -maxdepth 3 -name 'tsconfig*.json' -type f 2>/dev/null
echo -e "\n=== Content of tsconfig.json if found ==="
if [ -f 'tsconfig.json' ]; then
cat tsconfig.json
fi
echo -e "\n=== Checking services/tdei.ts (lines 1-20) ==="
if [ -f 'services/tdei.ts' ]; then
head -20 services/tdei.ts
fi
echo -e "\n=== Searching for .ts imports in source files ==="
rg "from ['\"].*\.ts['\"]" --type=ts -n 2>/dev/null | head -20Repository: TaskarCenterAtUW/workspaces-frontend
Length of output: 2200
🏁 Script executed:
#!/bin/bash
echo "=== nuxt.config.ts ==="
cat nuxt.config.ts
echo -e "\n=== Checking if ~/types/tdei.ts exists (as types/tdei.ts) ==="
if [ -f 'types/tdei.ts' ]; then
echo "File exists"
wc -l types/tdei.ts
else
echo "File NOT found at types/tdei.ts"
fi
echo -e "\n=== Checking for allowImportingTsExtensions in all configs ==="
grep -r "allowImportingTsExtensions" . 2>/dev/null || echo "Not found"
echo -e "\n=== Checking biome.json for TypeScript settings ==="
cat biome.json
echo -e "\n=== Other imports in services/tdei.ts (checking import patterns) ==="
head -5 services/tdei.tsRepository: TaskarCenterAtUW/workspaces-frontend
Length of output: 3727
Drop the explicit .ts extension in the import.
Line 9 adds an import with ~/types/tdei.ts. Without allowImportingTsExtensions enabled (which is not configured in this project), TypeScript will reject this import. All other imports in the file use extensionless paths (~/services/http, ~/services/loading), and this is the only .ts import in the codebase.
🛠️ Minimal fix
import type {
TdeiFeedback,
TdeiProjectGroup,
TdeiUserItem,
-} from '~/types/tdei.ts';
+} from '~/types/tdei';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/tdei.ts` around lines 5 - 9, The import line using the explicit .ts
extension causes TypeScript to fail; remove the file extension so the import
reads from '~/types/tdei' instead of '~/types/tdei.ts' (update the import that
brings in TdeiFeedback, TdeiProjectGroup, TdeiUserItem) to match other
extensionless imports like '~/services/http' and '~/services/loading'.
Comment on lines
+450
to
+451
| clone(signal?: AbortSignal): TdeiUserClient { | ||
| return new TdeiUserClient(this._baseUrl, this.#tdeiClient, signal ?? this._abortSignal); |
There was a problem hiding this comment.
Clone the nested TdeiClient too.
TdeiUserClient._send() still refreshes auth through this.#tdeiClient. Reusing the original instance here means a cloned user client keeps the old AbortSignal for refresh calls, so canceling one request can poison later clones or leave refreshes uncancelable.
🛠️ Minimal fix
clone(signal?: AbortSignal): TdeiUserClient {
- return new TdeiUserClient(this._baseUrl, this.#tdeiClient, signal ?? this._abortSignal);
+ const nextSignal = signal ?? this._abortSignal;
+ return new TdeiUserClient(
+ this._baseUrl,
+ this.#tdeiClient.clone(nextSignal),
+ nextSignal,
+ );
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/tdei.ts` around lines 450 - 451, The clone method on TdeiUserClient
currently returns a new TdeiUserClient but reuses the original private
`#tdeiClient`, which keeps the original AbortSignal; update clone(signal?:
AbortSignal): TdeiUserClient to also clone the nested TdeiClient (e.g., const
clonedTdei = this.#tdeiClient.clone(signal ?? this._abortSignal)) and pass that
cloned instance into the new TdeiUserClient constructor (new
TdeiUserClient(this._baseUrl, clonedTdei, signal ?? this._abortSignal)) so that
_send() and auth refreshes use the cloned client's signal rather than the
original.
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.
This sets up the UI for role management and integrates access controls into the Workspaces frontend.
Backend PR: TaskarCenterAtUW/workspaces-backend#4
Summary by CodeRabbit
New Features
UI/UX Improvements / Access Controls
Types & Utilities
Service & Backend Integration
Notable Component Changes
Overall, this PR implements the initial permissions/roles system end-to-end: types and service methods, a composable for role derivation, UI wiring across dashboard and workspace settings, a members management page, and permission-aware flows for TDEI export and settings management.