This repository has been archived by the owner on Oct 27, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1
Conversation
This file contains 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
We had to implement a change to how urls, and specifically any url that points to a page which handles transient registrations are formed. DEFRA/waste-carriers-engine#579 Essentially we needed to switch to using a token rather than the registration identifier. That specific change does not affect the frontend, but as part of the same PR we also fixed an issue with validation errors, where generating one would break back links. This turned out to just be because we were putting the ID for a 'resource' at the end of the url, rather than before the action we wanted to carry out against it. This seems to break a rails convention hence generated back links stopped working in certain cases. That fix does impact this project, because it now needs to ensure that the path it redirects users to when renewing matches what the new front and back office apps expect. This change covers everything needed to update the frontend to redirect users correctly.
Cruikshanks
added
the
housekeeping
Refactoring, tidying up or other work which supports the code
label
Dec 11, 2019
PR #278 removed the renewals holding page. We should have probably removed the check whether to show the holding page as well, but forgot we had one!
This covers all the remaining changes needed to generate the right renewal path in both production and non-production environments.
cintamani
approved these changes
Dec 12, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cruikshanks
added a commit
that referenced
this pull request
Dec 13, 2019
When we made the changes in #279 we mistakenly thought the `/bo` and `/fo` paths were only needed in production, and were not to be used when running locally. This is incorrect and they should be used in all environments. So this removes the check for production in `base_office_url()`.
Cruikshanks
added a commit
that referenced
this pull request
Dec 13, 2019
When we made the changes in #279 we mistakenly thought the `/bo` and `/fo` paths were only needed in production, and were not to be used when running locally. This is incorrect and they should be used in all environments. So this removes the check for production in `base_office_url()`.
Cruikshanks
added a commit
that referenced
this pull request
Dec 13, 2019
When we made the changes in #279 we mistakenly thought the `/bo` and `/fo` paths were only needed in production and were not to be used when running locally. This is incorrect and they should be used in all environments. So this removes the check for production in `base_office_url()`. It also updates the way we format the URL's to the back office to actually match was is needed. Finally, it includes some small refactoring to make naming, behaviour, and the location of things consistent.
Cruikshanks
added a commit
that referenced
this pull request
Mar 5, 2020
The Accessibility link seems to be adding a `/fo` where one isn't needed, because it is already included in the `Rails.configuration.front_office_url` value. We think the culprit for this regression was changes made as part of #279
Cruikshanks
added a commit
that referenced
this pull request
Mar 5, 2020
https://eaflood.atlassian.net/browse/RUBY-940 Spotted as part of regression tested that the accessibility statement returns a page not found when accessed from the frontend. ## Note The Accessibility link was adding a `/fo` where one wasn't needed because it is already included in the `Rails.configuration.front_office_url` value. We think the culprit for this regression was changes made as part of #279
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
We had to implement a change to how urls, and specifically any url that points to a page which handles transient registrations are formed.
DEFRA/waste-carriers-engine#579
Essentially we needed to switch to using a token rather than the registration identifier. That specific change does not affect the frontend, but as part of the same PR we also fixed an issue with validation errors, where generating one would break back links.
This turned out to just be because we were putting the ID for a 'resource' at the end of the url, rather than before the action we wanted to carry out against it. This seems to break a rails convention hence generated back links stopped working in certain cases.
That fix does impact this project, because it now needs to ensure that the path it redirects users to when renewing matches what the new front and back office apps expect.
This change covers everything needed to update the frontend to redirect users correctly.