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.

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

♻️ [open-formulieren/open-forms#4398] Cache initialDataReference to pass it with submissionCreate #716

Merged
merged 2 commits into from
Oct 14, 2024

Conversation

stevenbal
Copy link
Contributor

@stevenbal stevenbal commented Oct 1, 2024

required for open-formulieren/open-forms#4696

previously this was not passed properly, resulting in the initialDataReference not being sent on submissionCreate

@stevenbal stevenbal marked this pull request as draft October 1, 2024 15:07
src/components/Form.js Outdated Show resolved Hide resolved
Copy link

codecov bot commented Oct 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.42%. Comparing base (06d6ac1) to head (3a202c0).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #716      +/-   ##
==========================================
- Coverage   77.63%   77.42%   -0.22%     
==========================================
  Files         234      234              
  Lines        4780     4784       +4     
  Branches     1301     1305       +4     
==========================================
- Hits         3711     3704       -7     
- Misses       1036     1047      +11     
  Partials       33       33              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@sergei-maertens sergei-maertens left a comment

Choose a reason for hiding this comment

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

Discussed in the office:

  • caching in session storage seems risky because it may re-use this reference in a completely different form
  • ideal approach: baking in this parameter in the login link, so that it's sent to the backend and when the backend redirects again to the SDK, it should pass along the parameter again
  • if we can not reliably obtain this parameter from the URL, we may need to grab it in App.js and store it in state (in-memory), and expose it via React.createContext
  • make sure this works when the introduction page is enabled, which is a page before FormStart - it should not swallow the initial data reference query param

Good luck 🙂

@stevenbal stevenbal force-pushed the feature/4398-initial-data-reference branch 4 times, most recently from 9386ee0 to 70b1a66 Compare October 14, 2024 08:38
…d via login URL

previously it was attempted to grab this from the query params, but the parameter was not set anymore at the time it was actually needed (when sending the submission create request, after authentication)
@stevenbal stevenbal force-pushed the feature/4398-initial-data-reference branch from 70b1a66 to 3a202c0 Compare October 14, 2024 10:34
@stevenbal stevenbal marked this pull request as ready for review October 14, 2024 10:40
@sergei-maertens sergei-maertens merged commit 3d1cf3b into main Oct 14, 2024
17 of 19 checks passed
@sergei-maertens sergei-maertens deleted the feature/4398-initial-data-reference branch October 14, 2024 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants