Skip to content

A/B results: check the right per form - #302

Merged
Musiker15 merged 2 commits into
mainfrom
fix/experiment-page-access
Oct 29, 2026
Merged

Musiker15 merged 2 commits into
mainfrom
fix/experiment-page-access

Conversation

@Musiker15

Copy link
Copy Markdown
Member

Problem

  • forms/[formId]/experiment/page.tsx checked canManageForms(guildId, userId), so it required manage rights on the whole guild. Edit and preview of the same form use canManageForm(guildId, userId, formId). A manager of only some forms could set up an A/B test in the builder (the form PATCH checks per form), but the results page told them they had no permission.
  • The forms list showed "A/B results" on every row with an experiment, also to reviewers, who then hit the same wall.

Fix

  • The page checks canManageForm for the form in the URL.
  • The list shows the link only when form.manages is set, like edit and preview.

Tests

  • src/test/experiment-page-access.test.ts calls the page with mocked auth, rights, form lookup and dictionary. A per-form manager gets past the check (the form is loaded), someone without the right for this form doesn't.
  • Counter-check: with the old guild-level check, the per-form manager test fails.
  • pnpm --filter @msk-forms/web typecheck, eslint and prettier on the changed files.

The results page of an A/B test asked for guild-wide manage rights, while
editing and preview of the same form check the right for that form. Someone
who manages only some forms could set up an experiment in the builder but
not see its results. The forms list, on the other hand, offered the link to
everyone who sees the form, reviewers included.

The page now uses canManageForm for the form in the URL, and the list
shows the link only on rows the viewer manages, like edit and preview.
@Musiker15
Musiker15 merged commit 9299418 into main Oct 29, 2026
4 checks passed
@Musiker15
Musiker15 deleted the fix/experiment-page-access branch October 29, 2026 06:23
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.

1 participant