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

✨ [#1838] Add zaaktypen select widget for Category admin #845

Merged
merged 7 commits into from
Nov 20, 2023

Conversation

stevenbal
Copy link
Contributor

@stevenbal stevenbal commented Nov 13, 2023

@stevenbal stevenbal marked this pull request as draft November 13, 2023 11:54
@stevenbal stevenbal force-pushed the feature/1838-category-visibility-zaaktypen branch 3 times, most recently from e8c97f7 to 9f58b6b Compare November 13, 2023 14:57
@stevenbal stevenbal force-pushed the feature/1837-category-visibility-digid-eherkenning branch from d41d4b2 to 887d74f Compare November 13, 2023 15:02
Base automatically changed from feature/1837-category-visibility-digid-eherkenning to develop November 13, 2023 15:28
@stevenbal stevenbal force-pushed the feature/1838-category-visibility-zaaktypen branch 3 times, most recently from a60bd8c to 7c1a642 Compare November 13, 2023 16:17
@stevenbal stevenbal requested a review from alextreme November 13, 2023 16:24
@stevenbal stevenbal changed the title [Draft] ✨ [#1838] Add zaaktypen select widget for Category admin ✨ [#1838] Add zaaktypen select widget for Category admin Nov 13, 2023
@stevenbal stevenbal marked this pull request as ready for review November 13, 2023 16:24
@alextreme alextreme requested a review from pi-sigma November 13, 2023 19:54

months_since_last_zaak_per_zaaktype = {}
for case in cases:
resolve_zaak_type(case)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alextreme this approach does mean that we have to do a fetch to get the zaaktype for each zaak, which isn't very efficient. I'm not sure how we could avoid this without something like an include parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed: simplify this by storing the zaaktype URL on zaaktypeconfig and use that instead of identificatie on category, to avoid having to resolve here

@stevenbal stevenbal marked this pull request as draft November 14, 2023 10:48
@stevenbal
Copy link
Contributor Author

Marked as draft again until performance issues have been resolved

@stevenbal stevenbal force-pushed the feature/1838-category-visibility-zaaktypen branch from dfddb8f to ff919cb Compare November 14, 2023 12:39
@codecov-commenter
Copy link

codecov-commenter commented Nov 14, 2023

Codecov Report

Attention: 17 lines in your changes are missing coverage. Please review.

Comparison is base (37d6af9) 92.84% compared to head (49b32d4) 92.83%.
Report is 7 commits behind head on develop.

Files Patch % Lines
...ak/migrations/0035_populate_zaaktypeconfig_urls.py 53.84% 12 Missing ⚠️
src/open_inwoner/pdc/managers.py 87.50% 4 Missing ⚠️
src/open_inwoner/pdc/admin/category.py 93.75% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #845      +/-   ##
===========================================
- Coverage    92.84%   92.83%   -0.01%     
===========================================
  Files          759      765       +6     
  Lines        26233    26486     +253     
===========================================
+ Hits         24355    24589     +234     
- Misses        1878     1897      +19     

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

@stevenbal stevenbal force-pushed the feature/1838-category-visibility-zaaktypen branch 2 times, most recently from f01def1 to ae04de6 Compare November 20, 2023 08:43
@@ -258,6 +258,10 @@ class SiteConfigurarionAdmin(OrderedInlineModelAdminMixin, SingletonModelAdmin):
)
},
),
(
_("Display options for authenticated users"),
{"fields": ("enable_categories_filtering_with_zaken",)},
Copy link
Member

Choose a reason for hiding this comment

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

Graag verplaatsen naar de OpenZaak solo config model

@stevenbal stevenbal removed the request for review from pi-sigma November 20, 2023 09:38
task: https://taiga.maykinmedia.nl/project/open-inwoner/task/1838

this is required, because otherwise the categories plugin will always apply the filter based on zaken for DigiD users, essentially overriding the other visibility flags for those users
@stevenbal stevenbal force-pushed the feature/1838-category-visibility-zaaktypen branch 2 times, most recently from 48a7749 to 49b32d4 Compare November 20, 2023 10:13
task: https://taiga.maykinmedia.nl/project/open-inwoner/task/1838

this is necessary to avoid performance issues when implementing personalization of categories based on zaaktypen
task: https://taiga.maykinmedia.nl/project/open-inwoner/task/1838

* move relevante_zaak_periode to ZaakTypeConfig, to have this fixed per zaaktype
* use the ZaakTypeConfig.url that was added to figure out the identificaties, to avoid having to resolve a lot of zaaktypes
@stevenbal stevenbal force-pushed the feature/1838-category-visibility-zaaktypen branch from 49b32d4 to db38712 Compare November 20, 2023 11:47
@stevenbal stevenbal marked this pull request as ready for review November 20, 2023 13:21
@stevenbal stevenbal merged commit 592ad79 into develop Nov 20, 2023
14 checks passed
@stevenbal stevenbal deleted the feature/1838-category-visibility-zaaktypen branch November 20, 2023 13:22
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