Skip to content

Fix redirects broken by Terms and Conditions redirect#8036

Merged
leio10 merged 2 commits intodecidim:developfrom
i-need-another-coffee:ale-fix-store-location
May 25, 2021
Merged

Fix redirects broken by Terms and Conditions redirect#8036
leio10 merged 2 commits intodecidim:developfrom
i-need-another-coffee:ale-fix-store-location

Conversation

@alecslupu
Copy link
Copy Markdown
Contributor

@alecslupu alecslupu commented May 21, 2021

🎩 What? Why?

Move store_current_location before the needs_tos_accepted, in order ensure the requested path is being persisted before tos intercepting the request. In that way, after the user accepts the tos, it will be redirected to previously requested page.

📌 Related Issues

Link your PR to an issue

  • Related to #?
  • Fixes #?

Testing

  • Create a new User see the T&C page
  • Request a new page from the menu
  • Observe the T&C is intercepting
  • Accept the T&C
  • See landing page ( The same T&C page )
  • Apply the patch
  • create a new user
  • request new page from menu, accept T&C
  • observe redirect to new Page happening.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

📷 Screenshots

Please add screenshots of the changes you're proposing
Description

♥️ Thank you!

@alecslupu alecslupu force-pushed the ale-fix-store-location branch from 2915bf3 to 08d93f7 Compare May 25, 2021 04:58
@alecslupu alecslupu marked this pull request as ready for review May 25, 2021 05:06
leio10
leio10 previously approved these changes May 25, 2021
Copy link
Copy Markdown
Contributor

@leio10 leio10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great @alecslupu, I was very frustrated with this bug, thanks for taking care of it! ❤️

I added a small improvement suggestion, apply it if you think it will work properly and I will merge this in any case.

@leio10 leio10 added in-review type: fix PRs that implement a fix for a bug module: core labels May 25, 2021
@alecslupu alecslupu force-pushed the ale-fix-store-location branch from e305214 to 9d650a3 Compare May 25, 2021 10:25
@leio10 leio10 merged commit f25ce75 into decidim:develop May 25, 2021
entantoencuanto added a commit that referenced this pull request May 31, 2021
* develop: (59 commits)
  Update supported versions in docs (#8079)
  Meetings merge minutes and close actions (#7968)
  Meeting calendars providers (#7944)
  Fix broken test on meetings after merging PR without rebase (#8076)
  Show participants list in meetings (#7933)
  Security feature external link warning (#7397)
  Add missing tests for scope types admin page (#8053)
  Use symbols for polymorphic route arguments (#8052)
  Mockup design for Participation statistics tables in Votings (#7879)
  Fix boolean fields for .reported? and .hidden? which is nil if no report exists (#7990)
  Fix redirects broken by Terms and Conditions redirect (#8036)
  Amend CSS overwritting (#8007)
  New Crowdin updates (#8048)
  Fix undetected broken tests because of missing dependencies (#8050)
  Validate results by Monitoring Committee Members (#7899)
  Electoral certificate validation by Monitoring Committee Members (#7871)
  Publish and unpublish a meeting (#7893)
  New Crowdin updates (#8005)
  Polling station closure attach the physical electoral closure certificate (#7929)
  Fix attachment title migration generating possibly invalid values (#8020)
  ...
leio10 pushed a commit that referenced this pull request May 31, 2021
* Fix redirects broken by Terms and Conditions redirect

* Fix redirects broken by Terms and Conditions redirect - improvements
@leio10 leio10 mentioned this pull request Jun 22, 2021
12 tasks
@alecslupu alecslupu deleted the ale-fix-store-location branch July 19, 2021 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in-review module: core type: fix PRs that implement a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants