Skip to content

fix(dashboard): Replace Select with Combobox for inline role editing#6621

Draft
ssiyad wants to merge 2 commits into
developfrom
feat/team_invite/role
Draft

fix(dashboard): Replace Select with Combobox for inline role editing#6621
ssiyad wants to merge 2 commits into
developfrom
feat/team_invite/role

Conversation

@ssiyad

@ssiyad ssiyad commented Jun 4, 2026

Copy link
Copy Markdown
Member

Problem

The Role column in the team member list used the legacy Select component for inline role editing, while the invite dialog already used the newer Combobox. Additionally, when the get_roles API loaded asynchronously, the list cells never re-rendered because Vue couldn't track dependencies referenced inside closures (the column component functions).

Changes

dashboard: TeamInviteDialog — Replaced FormControl[type="select"] with <Combobox> with open-on-focus.

dashboard: Team.vue — Key changes:

  • Replaced Select with Combobox for role editing in both member and invitation rows
  • Moved column definitions to a computed() to fix a reactivity hole: when roles.data loaded asynchronously, the cell component functions were never re-invoked, so the guard always showed a Badge. Now columns is reactive — when roleOptions changes, the parent re-renders, giving cells fresh component functions with actual role data.
  • Added roleOptions computed to normalize API data to {label, value} format (matching the dialog)
  • Two-stage guard: non-admins see a Badge (no edit), loading state shows a Badge until roles arrive, then auto-transitions to Combobox
  • Added openOnFocus and openOnClick to match read-only select behavior

Testing

Reload the Team settings page. While roles are loading, the Role column shows blue Badges. Once loaded, they should transition to Comboboxes. Non-admins see only Badges.

- Replace FormControl[type='select'] with Combobox in TeamInviteDialog
- Replace Select with Combobox in Team member/invitation list
- Move column definitions to computed() to fix reactivity hole where
  cells never re-rendered when roles loaded asynchronously
- Add roleOptions computed to normalize data format (same as dialog)
- Two-stage guard: permission check first, then loading state
  (shows Badge until roles arrive, then auto-transitions to Combobox)
- Add openOnFocus and openOnClick to match read-only select behavior
@ssiyad ssiyad requested review from shadrak98 and siduck as code owners June 4, 2026 10:41
@greptile-apps

greptile-apps Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces the legacy Select component with Combobox for inline role editing on the Team settings page and fixes a reactivity hole that caused role cells to stay as Badges after async role data loaded. It also adds a missing cross-team ownership check in send_invitation and de-duplicates get_roles fetches across related methods.

  • Team.vue: Columns moved to a computed, Select replaced with Combobox; a roleOptions computed ensures the async-loaded role list is properly reactive and available to cell render functions.
  • TeamInviteDialog.vue: FormControl[type=\"select\"] replaced with Combobox and a manually added label element.
  • team.py: send_invitation now validates that an Account Request belongs to the current team before updating it; _validate_role and _set_invitation_role accept a pre-fetched all_roles list to avoid redundant DB round-trips.

Confidence Score: 5/5

Safe to merge — the backend security fix is correct and the frontend migration is a straightforward component swap.

The Python changes are additive guards with no regressions. The frontend changes are a clean Select→Combobox replacement with a loading-state guard. The columns computed's reactive mechanism is slightly different from what the author describes, but functionally the cells re-render correctly when roles load via ObjectList's own reactive render context.

No files require special attention.

Important Files Changed

Filename Overview
dashboard/src/components/settings/Team.vue Migrated inline Select to Combobox with a computed columns definition and a roleOptions computed; the reactive mechanism is slightly misrepresented but functionally correct in practice.
dashboard/src/components/settings/TeamInviteDialog.vue Replaced FormControl select with Combobox; manual label element is missing a for/id association with the combobox input.
press/press/doctype/team/team.py Added cross-team ownership check in send_invitation (security fix) and refactored _validate_role/_set_invitation_role to accept a pre-fetched all_roles list, avoiding redundant DB calls.

Sequence Diagram

sequenceDiagram
    participant U as User (Admin)
    participant TV as Team.vue
    participant CB as Combobox cell
    participant API as Backend (team.py)

    TV->>API: get_roles (auto)
    TV->>API: get_members (auto)
    API-->>TV: members.data (rows)
    Note over CB: roleOptions = [] → Badge shown
    API-->>TV: roles.data → roleOptions computed updates
    Note over CB: roleOptions.value.length > 0 → Combobox shown

    U->>CB: Select new role
    CB->>API: update_invitation_role(account_request, role)
    API->>API: get_roles() → _validate_role(role, all_roles)
    API->>API: "check d.team == self.name"
    API->>API: _set_invitation_role(d, role, all_roles)
    API-->>TV: updated members list
    TV->>CB: members.setData(data) → row re-renders
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
dashboard/src/components/settings/TeamInviteDialog.vue:68-77
The `<label>` element has no `for` attribute, so it is not programmatically associated with the `Combobox` input. Clicking the label won't focus the picker, and screen readers won't announce "Role" when the combobox is focused — this is a regression from `FormControl[type="select"]` which handled association automatically.

```suggestion
				<div v-if="roleOptions.length > 0">
					<label
						for="invite-role-combobox"
						class="block text-xs leading-4 text-ink-gray-5 mb-1"
					>
						Role
					</label>
					<Combobox
						id="invite-role-combobox"
						v-model="selectedRole"
						:options="roleOptions"
						open-on-focus
					/>
				</div>
```

### Issue 2 of 2
dashboard/src/components/settings/Team.vue:120-230
**`columns` computed has no reactive dependencies**

The `computed(() => [...])` factory only constructs the array and function literals — it never executes the component function bodies, so `roleOptions.value` is never read during the factory run. Vue therefore records zero dependencies, meaning `columns` will never recompute and the parent will never re-render when roles load asynchronously. The fix actually works via a different path: `roleOptions.value` is read inside the closures when ObjectList calls `column.component({ row })` within its own reactive render context, which IS tracked correctly. Consider touching `roleOptions.value` at the top of the factory (e.g. `const opts = roleOptions.value; return [...]`) to make the dependency explicit and self-documenting.

Reviews (3): Last reviewed commit: "fix(team): Misc fixes" | Re-trigger Greptile

Comment thread dashboard/src/components/settings/TeamInviteDialog.vue Outdated
@codecov-commenter

codecov-commenter commented Jun 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 11.11111% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.17%. Comparing base (2f32dbb) to head (64e23d3).
⚠️ Report is 28 commits behind head on develop.

Files with missing lines Patch % Lines
press/press/doctype/team/team.py 11.11% 16 Missing ⚠️

❌ Your patch status has failed because the patch coverage (11.11%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6621      +/-   ##
===========================================
+ Coverage    42.54%   50.17%   +7.62%     
===========================================
  Files          984      990       +6     
  Lines        82492    82971     +479     
  Branches       523      523              
===========================================
+ Hits         35099    41633    +6534     
+ Misses       47361    41306    -6055     
  Partials        32       32              
Flag Coverage Δ
dashboard 62.80% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ssiyad ssiyad closed this Jun 5, 2026
@ssiyad ssiyad reopened this Jun 5, 2026
@ssiyad ssiyad marked this pull request as draft June 5, 2026 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants