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

[#2946] Fix login for eHerkenning user with single vestiging #1535

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

pi-sigma
Copy link
Contributor

The KVK branch selection view should be displayed, and the vestigingsnummer of the company should be stored in the session, for companies that have only a single vestiging (i.e. a hoofdvestiging but no nevenvestiging).

We currently skip the KVK branch selection view for companies with a single vestiging. The reason for doing this was a problem with endless redirects caused by our middleware (see https://taiga.maykinmedia.nl/project/open-inwoner/task/2000).

The issue now is that if a client logs in with a KVK nummer that has only one vestiging, the vestigingsnummer is not stored in the session. Consequently, any calls to APIS that make use of the KVK + vestigingsnummer will give the wrong results for this KVK number.

Reproducible with KVK number 90002490, which has only a hoofdvestiging.

Taiga: https://taiga.maykinmedia.nl/project/open-inwoner/task/2946

@pi-sigma pi-sigma changed the title [#2931] Fix login for eHerkenning user with only one vestiging [#2931] Fix login for eHerkenning user with single vestiging Dec 20, 2024
@pi-sigma pi-sigma changed the title [#2931] Fix login for eHerkenning user with single vestiging [#2946] Fix login for eHerkenning user with single vestiging Dec 20, 2024
@codecov-commenter
Copy link

codecov-commenter commented Dec 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.13%. Comparing base (0564935) to head (7cfe067).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1535   +/-   ##
========================================
  Coverage    94.13%   94.13%           
========================================
  Files         1064     1064           
  Lines        39271    39276    +5     
========================================
+ Hits         36967    36972    +5     
  Misses        2304     2304           

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

@pi-sigma pi-sigma force-pushed the task/companies-open-submissions branch from 09ee10a to f5ab67a Compare December 20, 2024 10:58
@pi-sigma pi-sigma marked this pull request as ready for review December 20, 2024 11:00
@pi-sigma pi-sigma requested a review from swrichards December 20, 2024 11:00
@swrichards
Copy link
Contributor

One note about the copy:

Screenshot from 2024-12-20 13-00-58

Log in namens alle vestigingen

Is a bit misleading: what you're actually doing is "Log in namens rechtspersoon" which specifically excludes all vestigingen, including the hoofdvestiging.

Copy link
Contributor

@swrichards swrichards left a comment

Choose a reason for hiding this comment

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

LGTM, just some contextual things, plus my comment about a change to the form copy, to avoid confusion about rechtspersoon versus "all branches".

I double checked this locally, and it looks like it's sending the correct values to both Mijn Aanvragen and Mijn Vragen, depending on the selection (either kvk or vestigingsnummer for Mijn Vragen, depending on selection, and either kvk or kvk + vestiging for Mijn Aanvragen).

src/open_inwoner/kvk/views.py Outdated Show resolved Hide resolved
src/open_inwoner/kvk/views.py Show resolved Hide resolved
src/open_inwoner/kvk/views.py Show resolved Hide resolved
    - display the kvk branch selection and store vestigingsnummer in
      in session for companies with single vestigingsnummer
@pi-sigma pi-sigma force-pushed the task/companies-open-submissions branch from f5ab67a to 7cfe067 Compare December 20, 2024 13:27
@pi-sigma pi-sigma requested a review from swrichards December 20, 2024 14:23
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.

3 participants