Test every screen for every role, fix the authorization gaps found, restore the privacy policy page - #460
Merged
Conversation
The suite was written from the admin's point of view: no test rendered a page as a spectator, so the spectator branches of the scopes never ran their SQL and the sign-in 500 slipped through. Add a role-by-screen matrix (admin, member, spectator, user without organization, signed out), direct tests of every policy scope and rule per role, a cross-organization suite, API tests for members and spectators, and controller tests for settings, copies, locale, team-member modals and organization switching that had none. Bugs the new tests exposed, fixed here: - Members could open the edit modal of any colleague's time entry; edit_modal? ignored the record. It now follows the edit/update rule. - CSV export handed a member every colleague's entries on the project. It is now limited to the exporter's report scope. - Signed-out visitors got 500s on workspace pages, the assigned-task modal, the locale switch and organization switch: those controllers never authenticated and crashed on a nil user. - LocaleController#set_locale shadowed the LocaleHandler before_action of the same name and so ran before authentication. Renamed to update; unknown locales now redirect back instead of 406. - Submitting an invalid currency in settings crashed because the currency list was only loaded for edit. Fixtures gain a project access for the member role and a second organization's project/time entry so leaks across organizations can be asserted. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The view was deleted together with the landing page, but the route, controller, locale text and footer link stayed behind, so /privacy-policy returned 406 and the only link to it was hidden for the users who could see the layout. Recreate the view on the standard layout, link it from the sign-in screens and show the footer links to signed-in users. Make it readable for every role and for users still in onboarding (skip the authorization check, allow it past the onboarding redirect, guard the navbar against users without an organization). Correct the text in both languages: list all processors actually in use (Hetzner, Twilio SendGrid, Microsoft Clarity, PostHog, Sentry), replace the reference to the repealed 2000 Personal Data Act with the current act and GDPR articles 13-15, use the registered spelling "Bedriftsvegen", and fix a Norwegian typo. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Keep the refreshed layout; reapply the navbar guard for users without an organization and show the footer links to signed-in users.
The refreshed header links the logo to the time page, which spectators are not allowed to see and only get bounced from. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The UI refresh moved the user's email into the account menu (the trigger now shows the name) and turned the row's Edit into an icon-only button with an aria-label. Both tests failed on main since #459. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…to t3code/test-all-roles-screens
…-screens # Conflicts: # test/system/authentication_test.rb
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.
Why
A spectator got a 500 on sign-in (#457) and the suite did not notice. It was written from the admin's point of view: no test rendered a page as a spectator, so the spectator branches of the scopes never ran their SQL. This PR closes that class of gap.
Tests added (265 → 363, all green; 8 browser tests unchanged and green)
screens_by_role_testvisits every screen as admin, member, spectator, user without organization, and signed-out visitor, checking render/redirect and the navigation links per role.scopes_by_role_testruns every relation scope and record rule directly for every role, including the workspace scopes, with a second-organization fixture to catch leaks.cross_organization_access_test: an admin active in one organization tries to read, edit, copy, delete, export and report on the other organization's data.roles_testcovers members and spectators and theX-Organization-Idheader (previously admin-only tests).Bugs the tests exposed, fixed here
edit_modal?ignored the record).authenticate_user!, nil user).LocaleController#set_localeshadowed theLocaleHandlerbefore_action of the same name and ran before authentication. Renamed toupdate; unknown locale redirects back instead of 406.edit).Privacy policy page restored
/privacy-policyreturned 406: the view went with the landing page, the route and text stayed. Recreated on the standard layout, linked from the sign-in screens and the app footer, readable by every role and by users still in onboarding.Text corrected in
en/nb(please review, this is legal copy): all processors in use are now listed (Hetzner, Twilio SendGrid, Microsoft Clarity, PostHog, Sentry), the repealed 2000 Personal Data Act reference replaced with the current act + GDPR art. 13–15, registered address spelling "Bedriftsvegen", one Norwegian typo. Note that stemplin.com/privacy-policy hosts the old wording and should get the same corrections.Left as-is, for a decision
GET /projectsto members and spectators (ProjectPolicy#index?is admin-only) while serving them clients, tasks and reports. The test asserts today's behavior.Organization#time_regsgoes through users, so it includes entries a shared user logged in another organization. Unused in the app today.🤖 Generated with Claude Code