Adapt prefill to new auth context #4472

sergei-maertens · 2024-07-01T22:32:37Z

Closes #4246

Changes

Updated existing tests to mention legacy behaviour
Updated attribute name for authorizee prefill (which points to the legal subject)
Added tests for prefill of authorizee data with new format

Checklist

Check off the items that are completed or not relevant.

Impact on features
- Checked copying a form
- Checked import/export of a form
- Config checks in the configuration overview admin page
- Problem detection in the admin email digest is handled
Release management
- I have labelled the PR as "needs-backport" accordingly
I have updated the translations assets (you do NOT need to provide translations)
- Ran ./bin/makemessages_js.sh
- Ran ./bin/compilemessages_js.sh
Commit hygiene
- Commit messages refer to the relevant Github issue
- Commit messages explain the "why" of change, not the how

codecov · 2024-07-02T06:50:56Z

Codecov Report

Attention: Patch coverage is 91.30435% with 2 lines in your changes missing coverage. Please review.

Project coverage is 96.56%. Comparing base (ef079c3) to head (1d6a2b1).
Report is 575 commits behind head on master.

Files with missing lines	Patch %	Lines
...enforms/prefill/contrib/haalcentraal_brp/plugin.py	85.71%	0 Missing and 1 partial ⚠️
src/openforms/prefill/contrib/stufbg/plugin.py	85.71%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4472      +/-   ##
==========================================
- Coverage   96.57%   96.56%   -0.02%     
==========================================
  Files         720      720              
  Lines       24000    24017      +17     
  Branches     2843     2847       +4     
==========================================
+ Hits        23179    23192      +13     
- Misses        559      562       +3     
- Partials      262      263       +1

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

The value will need to be updated too, but that will involve some migrations and import shims to update the prefill configuration.

…ext data The legacy machtigen data-structure is deprecated, instead, we check more strictly for a BSN in a mandate context set by authentication.

Updating the values of the choices means we need to update the values of existing form definitions and variables, otherwise the code will get non-matching equality comparisons.

authorised_person is a confusing value when the authorizee is actually a company identified by KVK or RSIN, so we rename the value to authorizee instead.

The TODO was handled in previous commits and can be removed.

vaszig · 2024-07-02T11:29:14Z

src/openforms/prefill/contrib/stufbg/plugin.py

@@ -143,7 +143,18 @@ def get_identifier_value(
            return submission.auth_info.value

        if identifier_role == IdentifierRoles.authorizee:
-            return submission.auth_info.machtigen.get("identifier_value")
+            legacy_fallback = submission.auth_info.machtigen.get("identifier_value")


I see the code is duplicated but I guess we want that in this explicit way. I would have this in one place to be honest, in contrib for example.

IMO that breaks the isolation of plugins and ties them too much to generic open-forms code. in 3.0 we will probably remove the fallback behaviour and then it's just a similar-but-not-the-same pattern. maybe it could become a method on AuthInfo at some point, but the extra checks if it's a BSN/KVK kind of make it plugin-specific what needs to be checked.

In essence this is one-time only code, but because StUF-BG and HaalCentraal BRP plugins are alternatives for the same functionality, this code is duplicated.

sergei-maertens force-pushed the feature/4246-adapt-prefill-to-auth-context branch from 27244ce to 0d96730 Compare July 2, 2024 06:42

sergei-maertens force-pushed the feature/4246-adapt-prefill-to-auth-context branch from 0d96730 to 9544ff4 Compare July 2, 2024 08:51

sergei-maertens changed the title ~~Feature/4246 adapt prefill to auth context~~ Adapt prefill to new auth context Jul 2, 2024

sergei-maertens marked this pull request as ready for review July 2, 2024 10:10

sergei-maertens requested a review from vaszig July 2, 2024 10:10

sergei-maertens force-pushed the feature/4246-adapt-prefill-to-auth-context branch from c1e5444 to 46e2784 Compare July 2, 2024 10:13

sergei-maertens added 10 commits July 2, 2024 12:13

♻️ [#4246] Rename python attribute to better name

4990261

The value will need to be updated too, but that will involve some migrations and import shims to update the prefill configuration.

✨ [#4246] Update BRP and StUF-BG prefill plugins to use new auth cont…

35cd111

…ext data The legacy machtigen data-structure is deprecated, instead, we check more strictly for a BSN in a mandate context set by authentication.

✅ [#4246] Add tests for new machtiging structure

5045d7f

✅ [#4246] Add test for data migration result

cbf22bb

Updating the values of the choices means we need to update the values of existing form definitions and variables, otherwise the code will get non-matching equality comparisons.

🗃️ [#4246] Rename choice value to authorizee

ef6d968

authorised_person is a confusing value when the authorizee is actually a company identified by KVK or RSIN, so we rename the value to authorizee instead.

✨ [#4246] Rename authorizee identifier role in frontend

5d4c95a

🍱 [#4246] Update generated API schema

c927d10

🌐 [#4246] Extract new frontend translation messages

13dced6

✅ [#4246] Add unit tests for get_identifier_value plugin method

6e9e2f6

💡 [#4246] Update code comments

1d6a2b1

The TODO was handled in previous commits and can be removed.

sergei-maertens force-pushed the feature/4246-adapt-prefill-to-auth-context branch from 46e2784 to 1d6a2b1 Compare July 2, 2024 10:13

vaszig approved these changes Jul 2, 2024

View reviewed changes

sergei-maertens merged commit 0fb74d4 into master Jul 2, 2024
28 of 31 checks passed

sergei-maertens deleted the feature/4246-adapt-prefill-to-auth-context branch July 2, 2024 12:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adapt prefill to new auth context #4472

Adapt prefill to new auth context #4472

sergei-maertens commented Jul 1, 2024 •

edited

Loading

codecov bot commented Jul 2, 2024 •

edited

Loading

vaszig Jul 2, 2024

sergei-maertens Jul 2, 2024

Adapt prefill to new auth context #4472

Adapt prefill to new auth context #4472

Conversation

sergei-maertens commented Jul 1, 2024 • edited Loading

codecov bot commented Jul 2, 2024 • edited Loading

Codecov Report

vaszig Jul 2, 2024

Choose a reason for hiding this comment

sergei-maertens Jul 2, 2024

Choose a reason for hiding this comment

sergei-maertens commented Jul 1, 2024 •

edited

Loading

codecov bot commented Jul 2, 2024 •

edited

Loading