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

[#2903] Refactor import/export for ZaakType configs #1503

Merged
merged 2 commits into from
Dec 4, 2024

Conversation

pi-sigma
Copy link
Contributor

@pi-sigma pi-sigma commented Nov 26, 2024

  • Support export of only select ZaakType configs
  • Support import of ZaakType configs without library

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

@pi-sigma pi-sigma force-pushed the task/2903-import-export-zaaktype-config branch from 46e6e64 to c4d1d34 Compare December 3, 2024 10:41
@codecov-commenter
Copy link

codecov-commenter commented Dec 3, 2024

Codecov Report

Attention: Patch coverage is 67.04545% with 29 lines in your changes missing coverage. Please review.

Project coverage is 94.25%. Comparing base (064d3ce) to head (f4f89ce).
Report is 7 commits behind head on develop.

Files with missing lines Patch % Lines
src/open_inwoner/openzaak/admin.py 28.94% 27 Missing ⚠️
src/open_inwoner/openzaak/import_export.py 92.59% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1503      +/-   ##
===========================================
- Coverage    94.31%   94.25%   -0.06%     
===========================================
  Files         1068     1068              
  Lines        40386    40457      +71     
===========================================
+ Hits         38089    38133      +44     
- Misses        2297     2324      +27     

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

@pi-sigma pi-sigma marked this pull request as ready for review December 3, 2024 11:03
@pi-sigma pi-sigma requested a review from swrichards December 3, 2024 11:03
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.

An architectural question before I proceed, but TLDR: I think we can perhaps make this simpler by just adding another factory method to CatalogusConfigExport (and renaming it).

src/open_inwoner/openzaak/import_export.py Show resolved Hide resolved
@pi-sigma pi-sigma force-pushed the task/2903-import-export-zaaktype-config branch 4 times, most recently from 476ab5a to 814eb49 Compare December 4, 2024 07:54
@pi-sigma pi-sigma marked this pull request as draft December 4, 2024 08:17
@pi-sigma pi-sigma force-pushed the task/2903-import-export-zaaktype-config branch 2 times, most recently from b3c97d0 to 1b099a9 Compare December 4, 2024 09:17
@pi-sigma pi-sigma force-pushed the task/2903-import-export-zaaktype-config branch from 1b099a9 to 0a32b32 Compare December 4, 2024 09:19
@pi-sigma pi-sigma force-pushed the task/2903-import-export-zaaktype-config branch from 0a32b32 to f4f89ce Compare December 4, 2024 09:26
@pi-sigma pi-sigma changed the title [#2903] Support export of only select ZaakType configs [#2903] Refactor import/export for ZaakType configs Dec 4, 2024
@pi-sigma pi-sigma marked this pull request as ready for review December 4, 2024 09:48
@pi-sigma pi-sigma requested a review from swrichards December 4, 2024 09:48
CatalogusConfigExport,
CatalogusConfigImport,
)
from open_inwoner.openzaak.import_export import CatalogusConfigImport, ZGWConfigExport
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we could propagate the rename to the import container as well:

Suggested change
from open_inwoner.openzaak.import_export import CatalogusConfigImport, ZGWConfigExport
from open_inwoner.openzaak.import_export import ZGWConfigImport, ZGWConfigExport

Comment on lines +449 to +461
@admin.action(description=_("Export to file"))
def export_zaaktype_configs(modeladmin, request, queryset):
export = ZGWConfigExport.from_zaaktype_configs(queryset)
response = StreamingHttpResponse(
export.as_jsonl_iter(),
content_type="application/json",
)
response[
"Content-Disposition"
] = 'attachment; filename="zgw-zaaktype-export.json"'
return response

def process_file_view(self, request):
Copy link
Contributor

Choose a reason for hiding this comment

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

The only thing that springs to mind is whether to do this as a mixin, given that there's non trivial logic here. But I would say we can add that to the technical debt list (because I think we might also want to add a Celery option given how long these imports take, but it shouldn't delay the current PR and patch release).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted, I'll create a Taiga issue for this.

@swrichards swrichards requested a review from alextreme December 4, 2024 09:58
@alextreme alextreme merged commit 3192304 into develop Dec 4, 2024
20 checks passed
@alextreme alextreme deleted the task/2903-import-export-zaaktype-config branch December 4, 2024 10:03
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