Repository navigation
Stop saving a condition from undoing exit page edits - #3280
Open
theseanything wants to merge 1 commit into
Open
theseanything wants to merge 1 commit into
theseanything wants to merge 1 commit into
Conversation
theseanything
force-pushed
the
fix-exit-page-reverting-on-routes-save
branch
2 times, most recently
from
November 4, 2026 15:17
cf0b842 to
6db7157
Compare
theseanything
marked this pull request as ready for review
November 5, 2026 16:55
theseanything
requested review from
a team and
lfdebrux
and
a balanced review from Copilot
and removed request for
Copilot
November 5, 2026 16:55
Copilot stopped reviewing on behalf of
theseanything due to an error
November 5, 2026 16:56
Copilot stopped reviewing on behalf of
theseanything due to an error
November 5, 2026 16:57
Exit pages created before the ExitPage model still have their content in the legacy exit_page_heading and exit_page_markdown fields on the condition. Condition#sync_exit_page copied those values onto the ExitPage every time the condition was saved. The exit page editor and the Welsh translation page update the ExitPage directly and leave the legacy fields as they were. Saving the routes page saves every condition, so it put the old content back on the exit page and undid the user's changes. Only copy each legacy value when it's being changed, or copy all of them when the condition doesn't have an ExitPage yet. Copying all four values whenever any one changed would still let an English edit through the legacy condition editor wipe Welsh added on the translation page, or copy stale Welsh from an exit page the condition used to go to.
theseanything
force-pushed
the
fix-exit-page-reverting-on-routes-save
branch
from
November 6, 2026 16:29
6db7157 to
4a8e8c0
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The guards can silently skip restoring legacy ExitPage heading or markdown values and leave directly edited Welsh content unchanged when a draft is discarded.
2 open issues
What changed in this PR
This PR prevents routine condition saves from overwriting newer ExitPage content with stale legacy values, including Welsh translations.
Changes:
- Copy legacy fields individually only when changed, or copy all fields when creating an ExitPage.
- Add model regression coverage for preserving direct edits.
- Add request coverage for route saves and legacy English edits preserving Welsh translations.
| File | Description |
|---|---|
spec/requests/routes_controller_spec.rb |
Tests that route saves preserve direct edits and English edits preserve Welsh content. |
spec/models/condition_spec.rb |
Tests selective synchronization after direct ExitPage edits. |
app/models/condition.rb |
Guards legacy synchronization with per-field English and Welsh change checks. |
Review effort: Balanced
Comment on lines
+34
to
+35
| exit_page.heading_en = legacy_exit_page_heading(locale: :en) if copy_all || exit_page_heading_changed? | ||
| exit_page.markdown_en = legacy_exit_page_markdown(locale: :en) if copy_all || exit_page_markdown_changed? |
Comment on lines
+36
to
+37
| exit_page.heading_cy = legacy_exit_page_heading_cy if copy_all || legacy_exit_page_welsh_changed?(:exit_page_heading) | ||
| exit_page.markdown_cy = legacy_exit_page_markdown_cy if copy_all || legacy_exit_page_welsh_changed?(:exit_page_markdown) |
|
🎉 A review copy of this PR has been deployed! You can reach it at: https://pr-3280.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. |
This branch has not been deployed
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
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?
Trello card:
A user reported that their exit page changes weren't saving. After they saved, the editor showed the old content again.
Exit pages created before the
ExitPagemodel still have their content in the legacyexit_page_headingandexit_page_markdownfields on the condition.Condition#sync_exit_pagecopied those fields onto theExitPageevery time the condition was saved.The exit page editor and the Welsh translation page update the
ExitPagedirectly, so the legacy fields go out of date. Saving the routes page saves every condition, which copied the old content back. This affects forms in groups with multiple branches enabled, and Welsh translations too.This PR changes
sync_exit_pageto copy each legacy value only when it changes, or all of them when there's noExitPageyet. Copying all four whenever one changed wasn't enough. An English edit in the legacy condition editor would still wipe the Welsh, or overwrite it with stale content.