Remove the dedicated MOUs list page#2919
Draft
theseanything wants to merge 5 commits into
Draft
Conversation
The organisations pages now show each organisation's signed MOUs and agreements, making the standalone list at /mous redundant. Point the header navigation and sitemap at the organisations pages instead, and drop the now-unused MouSignaturePolicy and locale strings. The MOU signing flow is unchanged.
theseanything
force-pushed
the
remove-mous-list-page
branch
from
3650c14 to
5d1bcd9
Compare
|
🎉 A review copy of this PR has been deployed! You can reach it at: https://pr-2919.admin.review.forms.service.gov.uk/ It may take 5 minutes or so for the application to be fully deployed and working. If it still isn't ready For the sign in details and more information, see the review apps wiki page. |
The MOU upgrade spec visits the organisation page straight after clicking Save, racing the form submission: the save's redirect could land after the visit and clobber the organisation page, failing the final assertion. Wait for the redirect to the users page before navigating, replacing the sleep the spec relied on before it was rewritten.
The mou_signatures.index locale namespace referred to the MOUs list view, which no longer exists. Its surviving keys are only used by the organisations pages, so home them where they are consumed: the agreement type labels move to mou_signatures.agreement_type as they describe the model's enum and are shared between the organisations index, show and filter, while the table headings, preamble and share links move under organisations.show.mou_signatures alongside the existing heading. This stops the keys looking like dead code tied to a deleted view.
The MOU list page is gone and signed MOUs are now shown on each organisation's page, so the specs covering them describe organisations page behaviour. Move them to spec/features/organisations and reword the scenarios to match. The MOU signing flow spec stays where it is, as signing is unchanged.
Factories and specs inserted organisations with explicit primary keys (test_org reserved id 1). Explicit-id inserts do not advance the Postgres id sequence, so on a freshly prepared database any example mixing a hardcoded-id organisation with a sequence-assigned one could fail with a unique constraint violation, depending on test order. Nothing depends on the numeric ids: the organisation factory already deduplicates by slug, so every record asking for "test-org" shares one row regardless of id. The membership factory was only valid because the user's and group's separately-built organisations both carried id 1 and so compared equal. Build the group with the user's organisation instead, which is what the validation actually requires.
theseanything
force-pushed
the
remove-mous-list-page
branch
from
5d1bcd9 to
d43abd6
Compare
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.
What problem does this pull request solve?
Removes the standalone MOUs list page at
/mousin favour of the MOUs and agreements shown on the organisation pages.The organisations pages list each organisation's signed MOUs and agreements (and the organisations index shows MOU-signed status with filtering), so the separate list page is redundant.
Also fixes a latent flake in the rewritten feature specs: they create an organisation before
login_as_super_admin_usercreatestest_orgwith a hardcoded id of 1, so on a freshly prepared database the organisation could take id 1 from the sequence and collide. The organisation now uses an explicit id.