Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Server side redirects to the frontend break when the SDK uses hash based routing #3362

Closed
2 of 4 tasks
sergei-maertens opened this issue Aug 14, 2023 · 2 comments · Fixed by #3574 or open-formulieren/open-forms-sdk#586
Closed
2 of 4 tasks

Server side redirects to the frontend break when the SDK uses hash based routing #3362

sergei-maertens opened this issue Aug 14, 2023 · 2 comments · Fixed by #3574 or open-formulieren/open-forms-sdk#586
Assignees
@Viicos
Labels
bug Something isn't working
Milestone
Release 2.5.0

Comments

@sergei-maertens
Copy link
Member

sergei-maertens commented Aug 14, 2023 •
edited

Product versie / Product version

latest, 2.2.x, 2.1.x

Omschrijf het probleem / Describe the bug

The backend uses deep-links at times (submission resume, redirect back after authentication, change appointment and possible other cases) to the form on the frontend. It does this by taking submission.form_url and modifiying the URL with additional path segments and/or query string parameters. submission.form_url is recorded when a submission is started and is the only way to know where the public URL of the form is.

However, when you enable hash-based routing, the backend does not know this. Redirects are broken in those situations, as hash-based routing is typically used when embedding forms in a CMS where defining catch-all routes for a particular form is not (reasonably) achievable.

E.g. a form can be available on:

  • example.com/formulieren/foo
  • example.com/formulieren/foo#/

Possible solutions:

  • use some other form of input (query param, possibly HTTP header within the redirect response) to indicate which "route" or action is desired (/stap/bar, /afspraak-annuleren, submission-start etc.). This would decouple the backend from the specific routing solution AND any client-side URLs
  • record the routing type during submission creation so that the backend can generate the appropriate URLs

Stappen om te reproduceren / Steps to reproduce

No response

Verwacht gedrag / Expected behavior

Server side redirects work in both cases.

Screen resolution

None

Device

None

OS

None

Browser

No response

Tasks

  • Implement building redirect URL in backend
  • Update SDK to interpret backend actions & action args
  • Add data migration to clean submission.form_url
  • Deprecate "cleaned_form_url" wrapper.
@sergei-maertens sergei-maertens added bug Something isn't working triage Issue needs to be validated. Remove this label if the issue considered valid. and removed triage Issue needs to be validated. Remove this label if the issue considered valid. labels Aug 14, 2023
@sergei-maertens
Copy link
Member Author

sergei-maertens commented Aug 21, 2023 •
edited

Refinement: confusing topic.
Needs stakeholder?

@sergei-maertens sergei-maertens added this to the Release 2.4.0 milestone Aug 21, 2023
@sergei-maertens sergei-maertens mentioned this issue Sep 5, 2023
Closed
@joeribekker
Copy link
Contributor

Refinement: Maykin will be stakeholder on this.

@joeribekker joeribekker mentioned this issue Sep 18, 2023
Open
This was referenced Nov 2, 2023
Merged
Merged
Viicos added a commit that referenced this issue Nov 13, 2023
@Viicos
✨ [#3362] Handle hash based frontend routing
0aca6c7
Viicos added a commit that referenced this issue Nov 13, 2023
@Viicos
✅ [#3362] Add some tests for the utility function
3bcefec
Viicos added a commit that referenced this issue Nov 13, 2023
@Viicos
♻️ [#3362] Refactor action args
64a6914
Viicos added a commit to open-formulieren/open-forms-sdk that referenced this issue Nov 15, 2023
@Viicos
🐛 [open-formulieren/open-forms#3362] Handle server side redirects
c87f004
Viicos added a commit that referenced this issue Nov 15, 2023
@Viicos
[#3362] Fix outdated tests
feb37d3
Viicos added a commit that referenced this issue Nov 15, 2023
@Viicos
[#3362] Fix other tests
91214c3
Viicos added a commit to open-formulieren/open-forms-sdk that referenced this issue Nov 15, 2023
@Viicos
🐛 [open-formulieren/open-forms#3362] Sync with latest backend changes
374d2dc
Viicos added a commit to open-formulieren/open-forms-sdk that referenced this issue Nov 15, 2023
@Viicos
🐛 [open-formulieren/open-forms#3362] One fix and revert test changes
f9526bf
Viicos added a commit that referenced this issue Nov 16, 2023
@Viicos
[#3362] Use a JSON dict instead, fix all tests
9c545d2
Viicos added a commit that referenced this issue Nov 16, 2023
@Viicos
[#3362] Format
1d9ac78
Viicos added a commit that referenced this issue Nov 16, 2023
@Viicos
[#3362] Add extra tests with already existing query
853ed05
Viicos added a commit that referenced this issue Nov 16, 2023
@Viicos
[#3362] Fix mixin
0cd75c4
Viicos added a commit to open-formulieren/open-forms-sdk that referenced this issue Nov 16, 2023
@Viicos
🐛 [open-formulieren/open-forms#3362] Align with latest backend
b254d94
Viicos added a commit that referenced this issue Nov 16, 2023
@Viicos
[#3362] Fix test class
a6d6282
Viicos added a commit to open-formulieren/open-forms-sdk that referenced this issue Nov 16, 2023
@Viicos
🐛 [open-formulieren/open-forms#3362] Differenciate between react rout…
ab1020e 
…er basepath and real pathname
Viicos added a commit to open-formulieren/open-forms-sdk that referenced this issue Nov 16, 2023
@Viicos
🐛 [open-formulieren/open-forms#3362] Simplify logic
a939e9d
Viicos added a commit that referenced this issue Nov 16, 2023
@Viicos
[#3362] Add a migration to clean up urls
0d236a6
Viicos added a commit that referenced this issue Nov 16, 2023
@Viicos
[#3362] Update and clarify docs
f2ecfc6
Viicos added a commit that referenced this issue Nov 17, 2023
@Viicos
✨ [#3362] Handle hash based frontend routing
752ac7d
Viicos added a commit that referenced this issue Nov 17, 2023
@Viicos
✅ [#3362] Add some tests for the utility function
feaf9e9
Viicos added a commit that referenced this issue Nov 17, 2023
@Viicos
♻️ [#3362] Refactor action args
3116ddd
Viicos added a commit that referenced this issue Nov 17, 2023
@Viicos
[#3362] Add a migration to clean up urls
f2369d3
Viicos added a commit that referenced this issue Nov 17, 2023
@Viicos
[#3362] Update and clarify docs
031e3f1
Viicos added a commit that referenced this issue Nov 17, 2023
@Viicos
[#3362] PR feedback
2dbe608
sergei-maertens pushed a commit to open-formulieren/open-forms-sdk that referenced this issue Nov 17, 2023
@Viicos @sergei-maertens
🐛 [open-formulieren/open-forms#3362] Handle server side redirects
54d1198 
The backend is now agnostic to URL routing/hash based routing - it will always
redirect back to the URL that was used to start the form and provide action &
action params query arguments that specify the target for the frontend.

The frontend parses this action & the params and maps it to the intended client
side routes, effectively decoupling implementation details between backend and
frontend.

Note that this requires the SDK to operate correctly in two of the tree main
steps in this flow:

1. SDK must correctly derive the 'base URL' for the form, irrespective of
   whether hash based routing is used or not. Fragments should *not* be sent
   to the backend, since they are ignored anyway.
2. The backend uses the URL supplied from 1. and append the action/action
   params from the context of the backend action/validation that was
   performed.
3. The SDK must correctly interpret the action and its params and route to
   the appropriate part of the application.

TODO: using this pattern, we can probably refactor the _start=1 flow from the
backend too, this can likely be converted to _of_action=startSubmission.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Viicos Viicos

Labels
bug Something isn't working
Projects
Development
Archived in project
Milestone
Release 2.5.0
3 participants
@joeribekker @sergei-maertens @Viicos