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

[#1427] Implemented OpenZaak status translation #748

Merged
merged 3 commits into from
Sep 8, 2023

Conversation

Bartvaderkin
Copy link
Contributor

@Bartvaderkin Bartvaderkin commented Aug 25, 2023

This touches a lot of files, but basically just adds a simple lookup model and helper code, an admin with import/export and tests for the lookup/translate code.

Copy link
Contributor Author

@Bartvaderkin Bartvaderkin Aug 25, 2023

Choose a reason for hiding this comment

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

Another one gone! 🥳

Copy link
Contributor

Choose a reason for hiding this comment

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

while template_tags:
    template_tags.pop()

@Bartvaderkin Bartvaderkin force-pushed the feature/1427-status-translation branch 2 times, most recently from 8d6906e to 384b715 Compare August 25, 2023 12:22
@@ -29,7 +29,19 @@ <h1 class="h1" id="title">{{ case.description }}</h1>
{# Status history. #}
{% if case.statuses %}
<h2 class="h2" id="statuses">{% trans 'Status' %}</h2>
{% status_list case.statuses %}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Begone!

@Bartvaderkin Bartvaderkin marked this pull request as ready for review August 25, 2023 14:04
@@ -221,6 +224,19 @@ def get_initiator_display(self, case: Zaak) -> str:
return ""
return ", ".join([get_role_name_display(r) for r in roles])

def get_statuses_data(
self, statuses: List[Status], lookup: TranslationLookup
) -> List[dict]:
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use the built-in type list for the annotation since we don't need to support Python < 3.9

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should, but let's do it in one operation and sweep for all main PEP 585 containers: I created Taiga issue 1718 for it.

simple key value lookup seeded from queryset.values_list(key, value)
"""

def __init__(self, key_value_iterable: Iterable[Tuple[str, str]]):
Copy link
Contributor

Choose a reason for hiding this comment

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

Tuple > tuple and delete import

Copy link
Contributor

@pi-sigma pi-sigma left a comment

Choose a reason for hiding this comment

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

Type annotations could be updated (List > list, Tuple > tuple).

Also, I noticed that some classes/functions have these annotations while others don't. Should we be more consistent or continue as do now? Let's discuss this at some point and make a decision.

@codecov-commenter
Copy link

Codecov Report

Merging #748 (ede1259) into develop (08a3925) will decrease coverage by 0.02%.
Report is 18 commits behind head on develop.
The diff coverage is 93.83%.

@@             Coverage Diff             @@
##           develop     #748      +/-   ##
===========================================
- Coverage    96.34%   96.32%   -0.02%     
===========================================
  Files          688      693       +5     
  Lines        24384    24514     +130     
===========================================
+ Hits         23492    23613     +121     
- Misses         892      901       +9     
Files Changed Coverage Δ
...open_inwoner/openzaak/resources/import_resource.py 59.09% <59.09%> (ø)
src/open_inwoner/cms/cases/views/mixins.py 96.87% <100.00%> (+0.03%) ⬆️
src/open_inwoner/cms/cases/views/status.py 89.02% <100.00%> (+0.18%) ⬆️
src/open_inwoner/openzaak/admin.py 91.40% <100.00%> (+0.88%) ⬆️
src/open_inwoner/openzaak/managers.py 87.30% <100.00%> (+0.86%) ⬆️
...oner/openzaak/migrations/0021_statustranslation.py 100.00% <100.00%> (ø)
...rge_0021_statustranslation_0022_mark_as_is_sent.py 100.00% <100.00%> (ø)
src/open_inwoner/openzaak/models.py 98.19% <100.00%> (+0.12%) ⬆️
src/open_inwoner/openzaak/tests/factories.py 98.86% <100.00%> (+0.06%) ⬆️
...rc/open_inwoner/openzaak/tests/test_case_detail.py 100.00% <100.00%> (ø)
... and 4 more

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

📢 Have feedback on the report? Share it here.

@alextreme alextreme merged commit a700e23 into develop Sep 8, 2023
10 checks passed
@alextreme alextreme deleted the feature/1427-status-translation branch September 8, 2023 16:42
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