Skip to content

Add initial permissions system #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cyrossignol wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add initial permissions system #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01e4929
Add initial permissions system
cyrossignol Jan 1, 2026
cb510ec
Fix spelling
cyrossignol Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 36 additions & 8 deletions components/ProjectGroupPicker.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,50 @@
class="project-group-picker form-select"
aria-label="Project Group Selection"
>
<option v-for="pg in projectGroups" :key="pg.id" :value="pg.id">
<option
v-for="pg in projectGroups"
:key="pg.tdei_project_group_id"
:value="pg.tdei_project_group_id"
>
{{ pg.name }}
</option>
</select>
</template>

<script setup lang="ts">
import { tdeiUserClient } from '~/services/index'
import { tdeiUserClient } from '~/services/index';
import type { TdeiProjectGroupItem } from '~/types/tdei';
import { compareStringAsc } from '~/util/compare';

const model = defineModel({ required: true });
const projectGroups = (await tdeiUserClient.getMyProjectGroups())
.sort((a, b) => a.name.localeCompare(b.name));
const model = defineModel<string | undefined>({ required: true });
const props = defineProps<{ options?: TdeiProjectGroupItem[] }>();

if (projectGroups.length > 0) {
if (!model.value || !projectGroups.some(pg => pg.id === model.value)) {
model.value = projectGroups[0].id;
const fetchedGroups = ref<TdeiProjectGroupItem[]>([]);

if (!props.options) {
try {
fetchedGroups.value = (await tdeiUserClient.getMyProjectGroups()).sort(
compareStringAsc(g => g.name),
);
}
catch {
fetchedGroups.value = [];
}
}

const projectGroups = computed(() => props.options ?? fetchedGroups.value);

watch(
projectGroups,
(groups) => {
if (groups.length > 0) {
const pgId = model.value;

if (!pgId || !groups.some(pg => pg.tdei_project_group_id === pgId)) {
model.value = groups[0]?.tdei_project_group_id;
}
}
},
{ immediate: true },
);
</script>
58 changes: 50 additions & 8 deletions components/dashboard/DetailsTable.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>
<div class="table-responsive border-top">
<table class="table table-striped mb-0">
<div class="table-responsive border-top mb-0">
<table class="table table-striped">
<tbody>
<tr>
<th><app-icon variant="schedule" />Created At</th>
Expand All @@ -10,6 +10,41 @@
<th><app-icon variant="person_outline" />Created By</th>
<td>{{ workspace.createdByName }}</td>
</tr>
<tr>
<th><app-icon variant="badge" />My Role</th>
<td>
<span
v-if="workspace.role === 'lead'"
class="badge bg-dark text-uppercase"
>
<app-icon variant="star" /> Owner
</span>
<span
v-else-if="workspace.role === 'validator'"
class="badge bg-dark text-uppercase"
>
<app-icon variant="task_alt" /> Validator
</span>
<span
v-else
class="badge bg-secondary text-uppercase"
>
<app-icon variant="person" /> Member
</span>
Comment thread
cyrossignol marked this conversation as resolved.
<span
v-if="isPoc"
class="badge bg-warning text-dark text-uppercase ms-1"
>
<app-icon variant="local_police" /> POC
</span>
<span
v-else-if="isDataGenerator"
class="badge bg-warning text-dark text-uppercase ms-1"
>
<app-icon variant="offline_bolt" /> Data Generator
</span>
</td>
</tr>
<tr>
<th><app-icon variant="phonelink_setup" />App Access</th>
<td>
Expand Down Expand Up @@ -42,12 +77,19 @@
</template>

<script setup lang="ts">
const props = defineProps({
workspace: {
type: Object,
required: true
}
});
import type { Workspace } from '~/types/workspaces';

interface Props {
workspace: Workspace;
myTdeiRoles: string[];
Comment thread
cyrossignol marked this conversation as resolved.
}

const props = defineProps<Props>();

const isPoc = computed(() => props.myTdeiRoles.includes('poc'));
const isDataGenerator = computed(() =>
props.myTdeiRoles.includes(`${props.workspace.type}_data_generator`),
);
</script>

<style lang="scss">
Expand Down
4 changes: 2 additions & 2 deletions components/dashboard/Toolbar.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
<app-icon variant="settings" size="24" no-margin />
<span class="d-none d-sm-inline ms-2">Settings</span>
</nuxt-link>
</div>
</div>
</div><!-- .btn-group -->
</div><!-- .btn-toolbar -->
</template>

<script setup lang="ts">
Expand Down
15 changes: 15 additions & 0 deletions components/dashboard/WorkspaceItem.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,25 @@
<app-icon v-else variant="lock" />
App
</span>

<span
v-if="workspace.role === 'lead'"
class="badge bg-dark ms-2"
>
<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" /> {{ ROLE_LABELS.validator }}
</span>
</button>
</template>

<script setup lang="ts">
import { ROLE_LABELS } from '~/util/roles';

const props = defineProps({
workspace: {
type: Object,
Expand Down
30 changes: 21 additions & 9 deletions components/review/Toolbar.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,27 @@
{{ props.item.commentCount }}
</span>
</button>
<button
v-show="props.item.isFeedback"
class="btn btn-sm btn-success ms-2"
<BPopover
content="Only validators and owners can resolve feedback"
placement="bottom"
:manual="isValidator"
>
<app-icon
variant="check"
no-margin
/>
<span class="d-none d-sm-inline ms-2">Mark as Resolved</span>
</button>
<template #target>
<div class="d-inline-block ms-2">
<button
v-show="props.item.isFeedback && !props.item.isResolved"
class="btn btn-sm btn-success"
:disabled="!isValidator"
>
<app-icon
variant="check"
no-margin
/>
<span class="d-none d-sm-inline ms-2">Mark as Resolved</span>
</button>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
</div>
</template>
</BPopover>
</nav>
</template>

Expand All @@ -66,6 +77,7 @@ interface Props {
}

const props = defineProps<Props>();
const { isValidator } = useWorkspaceRole();

const emit = defineEmits(['edit']);
const showDetails = defineModel<boolean>('showDetails');
Expand Down
8 changes: 7 additions & 1 deletion components/settings/Nav.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,15 @@
>
General
</settings-nav-link>
<settings-nav-link
to="/members"
icon="admin_panel_settings"
>
Members
</settings-nav-link>
Comment on lines +9 to +14
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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).

Copy link
Copy Markdown
Contributor

@jeffmaki jeffmaki Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on this one? Seems relevant?

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeffmaki, I'd lean toward keeping the link visible to everyone and not gating it to leads only. Here's the reasoning:

The Members page isn't actually a dead-end for non-leads — per the members.vue implementation, non-leads still get a read-only view of the workspace member list with role badges (Owner/Validator/Member). Only the role-change dropdowns are gated to leads. That's a meaningful, informational page that any team member might reasonably want to visit to see who's in the workspace and what their roles are.

The original review comment ("Non-leads can still reach the page and only get an informational alert") slightly mischaracterizes the page — that 403-based info alert only appears when the project group user fetch fails (an error path), not as the standard non-lead experience.

In short:

  • Hiding the link = non-leads lose a useful read-only member directory view
  • Showing the link = consistent with the already-implemented read-only UX for non-leads on that page

The current behavior (link visible, page shows read-only for non-leads) is actually good layered UX. I'd resolve this comment as intentional.

ʕ •`ᴥ´• ʔ

🧠 Learnings used 
Learnt from: cyrossignol
Repo: TaskarCenterAtUW/workspaces-frontend PR: 44
File: pages/dashboard.vue:64-68
Timestamp: 2026-03-09T06:25:53.226Z
Learning: In Vue single-file components, when using Promise.all for multiple asynchronous fetches that gate rendering, do not attach a .catch() that silently handles or suppresses errors. If any critical promise rejects, let the error propagate so the page does not render partial or degraded UI. Mark which promises are critical and ensure failures surface to either the user or the surrounding rendering guard. This guideline applies broadly to all .vue components with Promise.all usage that affects render eligibility.

Learnt from: cyrossignol
Repo: TaskarCenterAtUW/workspaces-frontend PR: 44
File: components/dashboard/DetailsTable.vue:28-33
Timestamp: 2026-03-08T07:14:48.103Z
Learning: In TaskarCenterAtUW/workspaces-frontend, the `contributor` WorkspaceRole is never explicitly assigned. Membership in the TDEI project group implicitly grants a contributor role. Therefore, `workspace.role === undefined` and `workspace.role === 'contributor'` are semantically equivalent, and collapsing both into a single "Member" fallback (v-else) in the UI is intentional and correct. Do not flag this pattern as a bug.

Learnt from: cyrossignol
Repo: TaskarCenterAtUW/workspaces-frontend PR: 43
File: pages/help.vue:39-43
Timestamp: 2026-02-23T22:07:09.080Z
Learning: Review anchor tags in Vue components (e.g., <a href=... target="_blank">). Modern browsers treat target="_blank" as having implicit noopener behavior, so explicitly adding rel="noopener" is not necessary for security against tabnabbing. Use rel="noreferrer" only when you need to prevent the Referer header from being sent. Apply this guidance broadly to Vue files (all .vue), unless a specific need dictates otherwise.

Learnt from: cyrossignol
Repo: TaskarCenterAtUW/workspaces-frontend PR: 44
File: pages/workspace/[id]/settings/members.vue:206-219
Timestamp: 2026-03-08T07:05:10.800Z
Learning: In Vue 3 with <script setup>, values assigned inside top-level await blocks (e.g., within a try/catch around awaited API calls) do not need to be Vue refs. The component suspends via <Suspense> until the async setup completes, so plain let/const variables reach their final values before the template renders. Only mark a variable as reactive (ref) if it can change after the initial render. When reviewing code, verify that top-level awaited results used in the template are not unnecessarily made reactive, and ensure reactivity is introduced only for values that may change at runtime after mount.

<settings-nav-link
to="/teams"
icon="group"
icon="diversity_3"
>
Teams
</settings-nav-link>
Expand Down
17 changes: 17 additions & 0 deletions components/settings/panel/Apps.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@
External Apps
</h3>

<b-alert
v-if="!isLead"
variant="info"
show
class="mb-3"
>
<app-icon variant="info" />
Only workspace owners can change external app settings.
</b-alert>

<div class="form-check form-switch">
<label class="form-check-label">
<input
Expand All @@ -16,6 +26,7 @@
class="form-check-input"
:true-value="1"
:false-value="0"
:disabled="!isLead"
>
Publish this workspace for external apps
</label>
Expand All @@ -36,6 +47,7 @@
type="radio"
name="longFormQuestType"
value="JSON"
:disabled="!isLead"
>
</label>
</div>
Expand All @@ -48,6 +60,7 @@
type="radio"
name="longFormQuestType"
value="URL"
:disabled="!isLead"
>
</label>
</div>
Expand All @@ -61,6 +74,7 @@
:class="{ 'drag-over': isDraggingQuest }"
rows="5"
placeholder="Optional"
:disabled="!isLead"
@dragover.prevent="isDraggingQuest = true"
@dragleave.prevent="isDraggingQuest = false"
@drop.prevent="onQuestFileDrop"
Expand Down Expand Up @@ -96,6 +110,7 @@
type="text"
class="form-control"
placeholder="https://..."
:disabled="!isLead"
>
</label>
<div
Expand Down Expand Up @@ -131,6 +146,7 @@
<button
type="submit"
class="btn btn-primary"
:disabled="!isLead"
>
Save
</button>
Expand All @@ -157,6 +173,7 @@ const longFormQuestSchemaUrl = import.meta.env.VITE_LONG_FORM_QUEST_SCHEMA;
const longFormQuestExampleUrl = import.meta.env.VITE_LONG_FORM_QUEST_EXAMPLE_URL;

const workspace = inject<Workspace>('workspace')!;
const { isLead } = useWorkspaceRole();

const [longFormQuestSettings] = await Promise.all([
workspacesClient.getLongFormQuestSettings(workspace.id),
Expand Down
23 changes: 21 additions & 2 deletions components/settings/panel/Delete.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,24 @@
Delete Workspace
</h3>

<b-alert
v-if="!isLead"
variant="info"
show
class="mb-3"
>
<app-icon variant="info" />
Only workspace owners can delete the workspace.
</b-alert>

<p>
Deleting a workspace is permanent. This action will not remove any
TDEI datasets outside of Workspaces.
</p>

<button
class="btn btn-outline-danger mb-3"
:disabled="accepted"
:disabled="!isLead || accepted"
@click="acceptDelete"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
>
I understand, and I want to delete this workspace
Expand All @@ -30,7 +40,7 @@

<button
class="btn btn-danger"
:disabled="attestation !== 'delete'"
:disabled="!isLead || attestation !== 'delete'"
@click="submitDelete"
Comment thread
cyrossignol marked this conversation as resolved.
>
Delete this workspace
Expand All @@ -48,18 +58,27 @@ import { workspacesClient } from '~/services/index';
import type { Workspace } from '~/types/workspaces';

const workspace = inject<Workspace>('workspace')!;
const { isLead } = useWorkspaceRole();

const accepted = ref(false);
const attestation = ref('');
const input = useTemplateRef<HTMLInputElement>('input');

async function acceptDelete() {
if (!isLead.value) {
return;
}

accepted.value = true;
await nextTick();
input.value!.focus();
}

async function submitDelete() {
if (!isLead.value) {
return;
}

await workspacesClient.deleteWorkspace(workspace.id);
navigateTo('/dashboard');
}
Expand Down
Loading
Loading