Add Organizations' User Invitation UI - #6126
Conversation
|
👋 Hi @yasinelmi, thanks for contributing! For the review process to begin, please verify that the following is satisfied:
Also check that issue requirements are satisfied & you ran Pull requests that don't follow the guidelines will be closed. Reviewer assignment can take up to 2 weeks. |
1b1371e to
d7e3b63
Compare
|
📢✨ Before we assign a reviewer, we'll turn on |
🟡 Waiting for changesLast updated: 2026-09-08 19:33 UTC |
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #6126 — CI passing; manual QA did not run.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran a phased review pipeline over the pull request diff:
- Classified the diff to select review passes (core, frontend, backend) and whether manual QA was required
- Core review pass checked correctness, design, architecture, testing, completeness, and DRY/SRP/Rule-of-Three principles
- Specialized frontend/backend review passes applied framework-specific lenses where those files changed
- For UI changes: manual QA and an accessibility audit against a live dev server, when available
- Checked CI status and linked issue acceptance criteria
- Synthesized one review from those passes and chose the verdict from the findings, CI status, and QA evidence
| (SessionAuthentication, BasicAuthentication, TokenAuthentication) | ||
| ) | ||
| @permission_classes((IsAuthenticated,)) | ||
| def send_organization_invitation_email(request): |
There was a problem hiding this comment.
✅ Resolved — addressed in the current code.
blocking: No send_mail — nobody is notified; Resend invitation mails nothing. Mirror templates/permissions/permissions_email*.txt.
| const organizations = ref([]); | ||
|
|
||
| onMounted(() => { | ||
| Organization.fetchCollection({ page_size: MAX_PAGE_SIZE }) |
There was a problem hiding this comment.
✅ Resolved — addressed in the current code.
blocking: Unfiltered — filter_view_queryset (models.py:1969) returns public=True orgs too. Add a member filter to OrganizationFilter.
| self.assertEqual(response.status_code, 400, response.content) | ||
| invitation.refresh_from_db() | ||
| self.assertFalse(invitation.accepted) | ||
| self.assertFalse(self.channel.editors.filter(pk=self.invited_user.id).exists()) |
There was a problem hiding this comment.
✅ Resolved — addressed in the current code.
blocking: Restore — assertFalse(invitation.accepted) alone misses the guard's effect.
| * Pass a falsy organizationId to use this in "create a new organization" mode: | ||
| * the fetch is skipped and `create` becomes usable instead of `update`. | ||
| */ | ||
| export function useOrganization(organizationId) { |
There was a problem hiding this comment.
✅ Resolved — addressed in the current code.
blocking: Captured at setup(); RouterView reuses the instance, so Save after Create calls Organization.update('', data). Take a getter.
| email, | ||
| shareMode: this.role.value, | ||
| }) | ||
| .then(() => { |
There was a problem hiding this comment.
✅ Resolved — addressed in the current code.
blocking: No .catch here or at OrganizationDetailsTab.vue:139, OrganizationUsersTable.vue:169,199, useOrganizationInvitations.js:34,40; shared/client.js re-rejects — use handleMembershipError.
| v-if="!isNew" | ||
| class="tabs-nav" | ||
| > | ||
| <KTabsList |
There was a problem hiding this comment.
✅ Resolved — addressed in the current code.
blocking: No KTabsPanel, so aria-controls (KTabsList.vue:201) targets a missing id.
| if not invitation: | ||
| invitation = Invitation.objects.create(**fields) | ||
|
|
||
| invitation.share_mode = share_mode |
There was a problem hiding this comment.
suggestion: Unvalidated share_mode → 500 at accept (models.py:3920).
| INVITATION, | ||
| {"revoked": True}, | ||
| channel_id=invitation.channel_id, | ||
| user_id=request.user.id, |
There was a problem hiding this comment.
suggestion: user_id is the revoker; the invitee never receives this.
| > | ||
| <template #menu> | ||
| <KDropdownMenu | ||
| :options="menuOptions(rows[rowIndex][3])" |
There was a problem hiding this comment.
suggestion: rowIndex indexes KTable's sortedRows, not rows — wrong target once sortable.
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #6126 — 6 of 9 prior findings resolved; 3 open, 2 new (1 blocking).
CI pending; manual QA did not run.
Still open, see existing threads: unvalidated share_mode (views/users.py:155); revoke notifies the revoker (viewsets/invitation.py:268); rowIndex indexes sortedRows (OrganizationUsersTable.vue:29).
Prior-finding status
RESOLVED — contentcuration/contentcuration/views/users.py:122 — Resend invitation mails nothing
RESOLVED — contentcuration/contentcuration/frontend/channelList/composables/useOrganizationList.js:14 — unfiltered org list
RESOLVED — contentcuration/contentcuration/tests/viewsets/test_invitation.py:802 — editors assertion
RESOLVED — contentcuration/contentcuration/frontend/channelList/composables/useOrganization.js:10 — organizationId captured at setup()
RESOLVED — contentcuration/contentcuration/frontend/channelList/views/Organization/InviteOrganizationUserForm.vue:92 — missing .catch
RESOLVED — contentcuration/contentcuration/frontend/channelList/views/Organization/OrganizationEditPage.vue:17 — KTabsPanel aria-controls
UNADDRESSED — contentcuration/contentcuration/views/users.py:155 — unvalidated share_mode
UNADDRESSED — contentcuration/contentcuration/viewsets/invitation.py:268 — revoke notifies the revoker
UNADDRESSED — contentcuration/contentcuration/frontend/channelList/views/Organization/OrganizationUsersTable.vue:29 — rowIndex indexes sortedRows
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
|
|
||
| {% if user and user.is_active %} | ||
| {% translate "Please sign in to Kolibri Studio to accept or decline your invitation:" %} | ||
| {{ domain }}{% url 'channels' %} |
There was a problem hiding this comment.
✅ Resolved — addressed in the current code.
blocking: {% url 'channels' %} lands on CHANNELS_EDITABLE; the org invitation banner only exists at /my-organizations ({% else %} branch redirects to base, same problem).
Org invitations also reach that page as broken channel invitations: filter_invited (viewsets/invitation.py:142) matches email alone, loadInvitationList (vuex/channelList/actions.js:45) returns them, StudioMyChannels/index.vue:119 filters only share_mode === 'edit' — so ChannelInvitation.vue renders a null channel name, ChannelListIndex.vue:196 inflates the badge, and accepting dispatches channel/loadChannel(null).
Both branches should link {{ domain }}{% url 'channels' %}#/my-organizations, and loadInvitationList should drop organization-bearing rows, as useOrganizationInvitations.js:20 already does.
| @@ -0,0 +1,5 @@ | |||
| export function getApiErrorMessage(error, fallback) { | |||
There was a problem hiding this comment.
✅ Resolved — addressed in the current code.
suggestion: OrganizationUsersTable.handleMembershipError (211-215) is still the verbatim body of this util — 3 of 4 call sites converted.
| }, | ||
| mixins: [routerMixin], | ||
| setup(props) { | ||
| const { loading, organization, update, create } = useOrganization(() => props.organizationId); |
There was a problem hiding this comment.
suggestion: Untested. OrganizationEditPage.spec.js passes static props, so it passes either way; a repro needs RouterView with props: true, then Save on the details tab asserting Organization.update gets org-2.
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #6126 — 7 of 12 prior findings resolved; 5 still open.
CI pending; manual QA did not run.
Still open:
views/users.py:155—share_modeunvalidatedviewsets/invitation.py:268— mails revokerOrganizationUsersTable.vue:29—rowIndex/sortedRowschannelList/utils.js:1— duplicatehandleMembershipErrorOrganizationEditPage.vue:77— untested
Prior-finding status
RESOLVED — contentcuration/contentcuration/views/users.py:122 — send_mail
RESOLVED — contentcuration/contentcuration/frontend/channelList/composables/useOrganizationList.js — public=True
RESOLVED — contentcuration/contentcuration/tests/viewsets/test_invitation.py:802 — assertFalse guard
RESOLVED — contentcuration/contentcuration/frontend/channelList/composables/useOrganization.js — stale id
RESOLVED — contentcuration/contentcuration/frontend/channelList/views/Organization/InviteOrganizationUserForm.vue:92 — .catch
RESOLVED — contentcuration/contentcuration/frontend/channelList/views/Organization/OrganizationEditPage.vue:17 — aria-controls
RESOLVED — contentcuration/contentcuration/templates/permissions/organization_permissions_email.txt — link target
UNADDRESSED — contentcuration/contentcuration/views/users.py:155 — share_mode
UNADDRESSED — contentcuration/contentcuration/viewsets/invitation.py:268 — user_id
UNADDRESSED — contentcuration/contentcuration/frontend/channelList/views/Organization/OrganizationUsersTable.vue:29 — rowIndex
UNADDRESSED — contentcuration/contentcuration/frontend/channelList/utils.js:1 — duplicate
UNADDRESSED — contentcuration/contentcuration/frontend/channelList/views/Organization/OrganizationEditPage.vue:77 — untested
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
| {{ domain }}{% url 'channels' %}#/my-organizations | ||
| {% else %} | ||
| {% translate "Please follow this link to create an account (you must be signed in to accept or decline invitations):" %} | ||
| {{ domain }}{% url 'accept_invitation_and_registration' email %}#/my-organizations |
There was a problem hiding this comment.
✅ Resolved — addressed in the current code.
suggestion: The redirect chain drops this fragment: new_user_redirect (views/users.py:444,459) sends new invitees to /accounts/#/create?email=..., and a UA keeps the original fragment only when Location has none (RFC 7231 §7.1.2). Drop it, or pass the target as a query param.
|
|
||
| self.send_invitation("active-invitee@example.com", "edit") | ||
|
|
||
| self.assertIn("#/my-organizations", mail.outbox[0].body) |
There was a problem hiding this comment.
✅ Resolved — addressed in the current code.
suggestion: Both branches carry #/my-organizations, so the active-User setup buys nothing and the registration branch stays untested. Assert /channels/#/my-organizations plus a no-User case.
| ]); | ||
| }); | ||
| }); | ||
| it('should exclude organization invitations', () => { |
There was a problem hiding this comment.
✅ Resolved — addressed in the current code.
praise: Asserts getter output, not the filter.
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #6126 — all 7 prior blocking findings resolved; 7 earlier suggestion threads remain open on code untouched by this delta.
CI pending. Manual QA did not run, so nothing here is visually verified. One new suggestion inline on the sole-admin guard.
Prior-finding status
RESOLVED — contentcuration/contentcuration/views/users.py:122 — no send_mail on resend
RESOLVED — contentcuration/contentcuration/frontend/channelList/composables/useOrganizationList.js — unfiltered org queryset
RESOLVED — contentcuration/contentcuration/tests/viewsets/test_invitation.py:802 — restore guard assertion
RESOLVED — contentcuration/contentcuration/frontend/channelList/composables/useOrganization.js — id captured at setup()
RESOLVED — contentcuration/contentcuration/frontend/channelList/views/Organization/InviteOrganizationUserForm.vue:92 — missing .catch handlers
RESOLVED — contentcuration/contentcuration/frontend/channelList/views/Organization/OrganizationEditPage.vue:17 — aria-controls targets a missing id
RESOLVED — contentcuration/contentcuration/templates/permissions/organization_permissions_email.txt — wrong invitation link target
UNADDRESSED — contentcuration/contentcuration/views/users.py:155 — unvalidated share_mode
UNADDRESSED — contentcuration/contentcuration/viewsets/invitation.py:268 — revoke change addressed to the revoker
UNADDRESSED — contentcuration/contentcuration/frontend/channelList/views/Organization/OrganizationUsersTable.vue:29 — rowIndex indexes sortedRows
UNADDRESSED — contentcuration/contentcuration/frontend/channelList/utils.js:1 — handleMembershipError duplicates getApiErrorMessage
UNADDRESSED — contentcuration/contentcuration/frontend/channelList/views/Organization/OrganizationEditPage.vue:77 — organizationId reactivity untested
UNADDRESSED — contentcuration/contentcuration/templates/permissions/organization_permissions_email.txt:19 — redirect chain drops the fragment
UNADDRESSED — contentcuration/contentcuration/tests/test_organization_invitation.py:66 — both branches assert the same URL fragment
ACKNOWLEDGED — contentcuration/contentcuration/frontend/channelList/vuex/channelList/tests/module.spec.js:55 — praise
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
| if (target.type !== 'member') { | ||
| return false; | ||
| } | ||
| const activeAdmins = this.members.filter(member => member.role === OrganizationRoles.ADMIN); |
There was a problem hiding this comment.
suggestion: members is one page, not the org.
useOrganizationMembers.js:18 fetches with page_size: 100 against a viewset capped at max_page_size = 100 (viewsets/organization.py:131) and never pages further. In an org with >100 active members whose second admin falls outside page 1, activeAdmins.length === 1 is true for an admin who is not the sole admin: the options button disappears and there is no way to demote or remove them, silently. The server counts all active admins, so it would have allowed the action.
Derive the flag from a count the server supplies, or page the fetch to completion.
Adds the frontend for issue learningequality#6088: a My Organizations landing page (pending-invitations banner + org card grid), an organization edit page with Details and Sharing tabs, an invite-by-email form, and a unified members/pending-invites table with role change, remove, resend, and revoke actions. Reuses StudioImmersiveModal for the edit page so it matches the channel edit/details page pattern, including its focus trap and dark banner header. Also supports creating a new organization through the same page. All new state is Composition API composables (no new Vuex), and every new component uses Kolibri Design System components exclusively (no Vuetify). Backend: adds a send_organization_invitation_email endpoint, a revoke action and organization support on InvitationViewSet, and a role annotation on top of learningequality#6080's OrganizationViewSet so the frontend can show the current user's role in an organization without an extra request per organization. Also includes fixes from review: the invitation email actually sends (with organization-specific templates), the organization list only shows orgs you're a member of, organization invitations no longer leak into the channel invitation UI, Save-after-Create reuses the newly created id instead of a stale one, network errors surface via snackbar instead of failing silently, KTabsList's aria-controls targets a real KTabsPanel, and the sole active admin of an org can't be offered a way to remove or demote themselves that would only fail server-side. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fb78f38 to
262edf4
Compare
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #6126 — 11 of 16 prior findings resolved; 5 open: views/users.py:155, viewsets/invitation.py:268, OrganizationUsersTable.vue:29, OrganizationEditPage.vue:77, OrganizationUsersTable.vue:158.
CI pending; manual QA did not run.
Prior-finding status
RESOLVED — views/users.py:122 — send_mail
RESOLVED — useOrganizationList.js — member
RESOLVED — test_invitation.py:802 — editors
RESOLVED — useOrganization.js — organizationId
RESOLVED — InviteOrganizationUserForm.vue:92 — .catch
RESOLVED — OrganizationEditPage.vue:17 — KTabsPanel
RESOLVED — organization_permissions_email.txt — #/my-organizations
RESOLVED — organization_permissions_email.txt — new_user_redirect
RESOLVED — test_organization_invitation.py — {% else %}
RESOLVED — channelList/utils.js:1 — handleMembershipError
RESOLVED — module.spec.js:55 — getters
UNADDRESSED — views/users.py:155 — share_mode
UNADDRESSED — viewsets/invitation.py:268 — user_id
UNADDRESSED — OrganizationUsersTable.vue:29 — rowIndex
UNADDRESSED — OrganizationEditPage.vue:77 — useOrganization
UNADDRESSED — OrganizationUsersTable.vue:158 — members
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
| export default async function startApp({ store, router, index }) { | ||
| trackInputModality(); | ||
| await initiateServiceWorker(); | ||
| try { |
There was a problem hiding this comment.
suggestion: Out of scope — changes startup for every bundle; own PR.
| @@ -0,0 +1,136 @@ | |||
| <template> | |||
There was a problem hiding this comment.
suggestion: Near-verbatim fork of Channel/ChannelInvitation.vue — template, decline modal, .invitation SCSS, $trs. Vuex blocks reuse; make it presentational.
| invitation.refresh_from_db() | ||
| self.assertTrue(invitation.revoked) | ||
|
|
||
| def test_sender_can_revoke_even_if_no_longer_admin(self): |
There was a problem hiding this comment.
suggestion: Neither guard branch is isolated: organization_role defaults to status=ACTIVE, and this admin is the sender. Drop is_org_admin or the sender_id clause — all four pass.
| } | ||
|
|
||
| # Need to break into two steps to avoid MultipleObjectsReturned error | ||
| invitation = Invitation.objects.filter( |
There was a problem hiding this comment.
suggestion: Comment dropped from untouched code — why filter().first(), not get_or_create. Also router.js, ChannelListIndex.vue.
| adminRole: 'Admin', | ||
| editorRole: 'Editor', | ||
| viewerRole: 'Viewer', | ||
| pendingRole: 'Pending {role}', |
There was a problem hiding this comment.
suggestion: Pending {role} nests $tr('adminRole') — untranslatable gender and order. Three flat strings instead; line 146 also calls $tr('').
4 participants
Summary
Implements the frontend for #6088: a My Organizations landing page, an organization
edit page with Details and Sharing tabs (invite/manage members), and organization
creation — following the existing channel edit/details page pattern.
Closes #6088
New pages and features
Framework compliance (per the issue's Frameworks checklist)
(
KCard,KButton,KTextbox,KCheckbox,KSelect,KTable,KTabsList,KModal,KDropdownMenu,StudioImmersiveModal).(
useOrganization,useOrganizationList,useOrganizationMembers,useOrganizationInvitations).Accessibility
ariaLabel/tooltip, each carrying a per-itemdistinguishing label rather than a generic repeated one — e.g. "More options for
{org name}" on organization cards, "Options for {email}" on each row of the
members/invitations table, and "Accept/Decline invitation to {organization}" on
each pending-invitation banner entry.
StudioImmersiveModal'sKFocusTrap), with Escape closing it — matching the existing channel details page.cue that the SPA navigated even though there's no real page reload.
a visual dimming.
Test plan
pytest— organization, organization-role-annotation, and invitation tests pass(102 backend tests)
pnpm test— full channelList frontend suite passes (226 tests)pre-commit run --all-fileson all touched fileschanges, resend/revoke invites, member removal (including the last-active-admin
guard), organization creation, and the non-admin read-only states
Notes for reviewers
composables) per the issue's Frameworks requirements.
🤖 Generated with Claude Code