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

[#911] Added 'indicatieInternOfExtern' access check, also refactored cases-views tests #394

Merged
merged 3 commits into from
Dec 19, 2022

Conversation

Bartvaderkin
Copy link
Contributor

@Bartvaderkin Bartvaderkin commented Dec 15, 2022

Note: the important change was to is_zaak_visible()

Then I found this wasn't applied to the list-views at all, and that required a refactor because we needed the filtering before the pagination which required to move the zaaktype retrieval code around.

But then I found the issue with the property casing in the mocks, and some other test noise like the cache-reset mixin and test data setup.

As a bonus: less noisy UserFactory, and improved ZGW api_model dataclass annotations that support swapping link URI's to objects because that is how it was used.

@codecov-commenter
Copy link

codecov-commenter commented Dec 15, 2022

Codecov Report

Merging #394 (c9888fe) into develop (047a0cb) will increase coverage by 0.00%.
The diff coverage is 99.35%.

@@           Coverage Diff            @@
##           develop     #394   +/-   ##
========================================
  Coverage    96.10%   96.10%           
========================================
  Files          462      462           
  Lines        15056    15082   +26     
========================================
+ Hits         14469    14494   +25     
- Misses         587      588    +1     
Impacted Files Coverage Δ
...rc/open_inwoner/openzaak/tests/test_cases_cache.py 100.00% <ø> (ø)
src/open_inwoner/accounts/views/cases.py 96.93% <92.30%> (-0.45%) ⬇️
src/open_inwoner/accounts/tests/factories.py 100.00% <100.00%> (ø)
src/open_inwoner/openzaak/api_models.py 98.94% <100.00%> (+0.01%) ⬆️
...rc/open_inwoner/openzaak/tests/test_case_detail.py 100.00% <100.00%> (ø)
...c/open_inwoner/openzaak/tests/test_case_request.py 100.00% <100.00%> (ø)
src/open_inwoner/openzaak/tests/test_cases.py 100.00% <100.00%> (ø)
src/open_inwoner/openzaak/tests/test_documents.py 100.00% <100.00%> (ø)
src/open_inwoner/openzaak/tests/test_utils.py 100.00% <100.00%> (ø)
src/open_inwoner/openzaak/utils.py 93.97% <100.00%> (+0.30%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Bartvaderkin Bartvaderkin force-pushed the fix/991-internal-case-leaks branch from 74d0935 to 7cac73a Compare December 15, 2022 15:38
…cases-views tests

Refactoring tests was about casing issue in generate_oas_component() mocks, using setUpTestData(), ClearCachesMixin and use of URLs instead of links
@Bartvaderkin Bartvaderkin force-pushed the fix/991-internal-case-leaks branch from 7cac73a to c9888fe Compare December 15, 2022 16:38
@Bartvaderkin Bartvaderkin requested a review from vaszig December 15, 2022 16:42
@Bartvaderkin Bartvaderkin marked this pull request as ready for review December 15, 2022 16:42
Copy link
Contributor

@vaszig vaszig left a comment

Choose a reason for hiding this comment

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

As far as I am concerned, I don't see something weird. Only a small comment-question.

# resolve case-type
self.case.zaaktype = fetch_single_case_type(self.case.zaaktype)
if not self.case.zaaktype:
return self.handle_no_permission()
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if this is done for readability purposes but we have five times the same return statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, to keep the decision procedure more readable (without adding extra noisy workarounds) it just returns after every failed condition.

@alextreme alextreme merged commit 4692511 into develop Dec 19, 2022
@alextreme alextreme deleted the fix/991-internal-case-leaks branch December 19, 2022 13:08
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.

4 participants