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

[#1045] Added ZaakTypeInformatieObjectTypeConfig model and import #443

Merged
merged 1 commit into from
Feb 1, 2023

Conversation

Bartvaderkin
Copy link
Contributor

@Bartvaderkin Bartvaderkin commented Jan 31, 2023

No description provided.

@Bartvaderkin Bartvaderkin force-pushed the feature/1045-zaak-config-4-docs branch 2 times, most recently from db5782a to 3943f9b Compare February 1, 2023 13:35
@codecov-commenter
Copy link

Codecov Report

Merging #443 (3943f9b) into develop (2d1b020) will increase coverage by 0.02%.
The diff coverage is 92.99%.

@@             Coverage Diff             @@
##           develop     #443      +/-   ##
===========================================
+ Coverage    96.15%   96.18%   +0.02%     
===========================================
  Files          489      492       +3     
  Lines        17114    17387     +273     
===========================================
+ Hits         16456    16723     +267     
- Misses         658      664       +6     
Impacted Files Coverage Δ
src/open_inwoner/openzaak/catalog.py 55.02% <38.09%> (+4.39%) ⬆️
src/open_inwoner/openzaak/admin.py 72.36% <60.00%> (-4.42%) ⬇️
src/open_inwoner/openzaak/zgw_imports.py 97.77% <97.95%> (+2.22%) ⬆️
...er/openzaak/management/commands/zgw_import_data.py 100.00% <100.00%> (ø)
...ner/openzaak/migrations/0009_auto_20230131_1253.py 100.00% <100.00%> (ø)
src/open_inwoner/openzaak/models.py 100.00% <100.00%> (ø)
src/open_inwoner/openzaak/tests/factories.py 100.00% <100.00%> (ø)
...rc/open_inwoner/openzaak/tests/test_zgw_imports.py 100.00% <100.00%> (ø)
...inwoner/openzaak/tests/test_zgw_imports_command.py 100.00% <100.00%> (ø)
...inwoner/openzaak/tests/test_zgw_imports_iotypes.py 100.00% <100.00%> (ø)
... and 3 more

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

@Bartvaderkin Bartvaderkin marked this pull request as ready for review February 1, 2023 14:15
def has_add_permission(self, request, obj):
return False

def has_delete_permission(self, request, obj=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure maybe I didn't understand something but do we want to allow delete? I mean since we just show data that we retrieve and the next day it will be again there. This has to do generally with the delete operation (ZaakTypeConfig as well).

Copy link
Contributor Author

@Bartvaderkin Bartvaderkin Feb 1, 2023

Choose a reason for hiding this comment

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

Allowing delete like this is mostly for development and testing purposes, which is why I put it as superuser only. It is not essential but sometimes convenient.

@@ -202,6 +193,34 @@ def fetch_single_case_type(case_type_url: str) -> Optional[ZaakType]:
return case_type


@cache_result(
Copy link
Contributor

Choose a reason for hiding this comment

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

This function exists twice in the file, maybe after rebasing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well spotted!

We really need to clean these at some point because what started as a few handy helper is growing out of control. I made a ticket 1096 on Taiga for this.

verbose_name=_("Document upload URL"),
blank=True,
)
document_upload_enabled = models.BooleanField(
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this have to do with external uploading?We have the same below for ZaakTypeInformatieObjectTypeConfig and its name is a bit confusing, at least for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the Taiga ticket 1045 explains best how it is supposed to work (it was my ticket so you might not have seen this.).

Maybe the name is not ideal; we can rename to fit the actual implementation (easy to migrate).

@Bartvaderkin Bartvaderkin changed the title [#1023] Added ZaakTypeInformatieObjectTypeConfig model and import [#1025] Added ZaakTypeInformatieObjectTypeConfig model and import Feb 1, 2023
@Bartvaderkin Bartvaderkin changed the title [#1025] Added ZaakTypeInformatieObjectTypeConfig model and import [#1045] Added ZaakTypeInformatieObjectTypeConfig model and import Feb 1, 2023
@Bartvaderkin Bartvaderkin force-pushed the feature/1045-zaak-config-4-docs branch from 3943f9b to 6eb8cc8 Compare February 1, 2023 15:20
@Bartvaderkin
Copy link
Contributor Author

@vaszig I processed your feedback.

Admittedly I also force-pushed again, because I used the wrong issue number in the commit (should be 1045 instead of 1023) 😕

@alextreme alextreme merged commit e5d60b5 into develop Feb 1, 2023
@alextreme alextreme deleted the feature/1045-zaak-config-4-docs branch February 1, 2023 17:16
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