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

[#2645] Filter cases by status #1331

Merged
merged 1 commit into from
Aug 12, 2024
Merged

Conversation

pi-sigma
Copy link
Contributor

@pi-sigma pi-sigma commented Aug 1, 2024

Styling for the case filter form is rudimentary and should be improved in accordance with design (see https://taiga.maykinmedia.nl/project/open-inwoner/task/2650)

Taiga: https://github.com/maykinmedia/open-inwoner/pull/new/task/2645-case-filter-status

@pi-sigma pi-sigma changed the title Task/2645 case filter status [#2645] Filter cases by status Aug 1, 2024
@pi-sigma pi-sigma force-pushed the task/2645-case-filter-status branch from 4a30aed to 7fc015f Compare August 2, 2024 07:10
@codecov-commenter
Copy link

codecov-commenter commented Aug 2, 2024

Codecov Report

Attention: Patch coverage is 94.11765% with 8 lines in your changes missing coverage. Please review.

Project coverage is 95.09%. Comparing base (81fa5d6) to head (0c8a157).

Files Patch % Lines
src/open_inwoner/cms/cases/views/cases.py 84.61% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           develop    #1331    +/-   ##
=========================================
  Coverage    95.09%   95.09%            
=========================================
  Files          993      994     +1     
  Lines        36219    36325   +106     
=========================================
+ Hits         34443    34544   +101     
- Misses        1776     1781     +5     

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

@pi-sigma pi-sigma force-pushed the task/2645-case-filter-status branch 6 times, most recently from f15addc to dfc9876 Compare August 5, 2024 11:42
@pi-sigma pi-sigma marked this pull request as ready for review August 5, 2024 12:05
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.

Looks good.

src/open_inwoner/cms/cases/views/cases.py Outdated Show resolved Hide resolved
src/open_inwoner/templates/pages/cases/list_inner.html Outdated Show resolved Hide resolved
case_statuses = [case.zaak.status_text for case in cases]

# add static text for open submissions
case_statuses += ["Openstaande aanvraag" for submission in submissions]
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that we use the Openstaande aanvraag literal below as well, we might want to put this in a constant, something like SUBMISSION_STATUS_OPEN.

I was also wondering: should we not also have the closed status, or do they disappear from the list?

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'll create the constant, makes sense
  • My understanding is that open submissions turn into regular cases once they've been taken up by s.o. at the municipality, so they disappear from the list. We also have the hard-coded string in src/open_inwoner/templates/pages/cases/list_inner.html

src/open_inwoner/cms/cases/views/cases.py Outdated Show resolved Hide resolved
src/open_inwoner/openzaak/tests/test_cases.py Outdated Show resolved Hide resolved
src/open_inwoner/cms/cases/views/cases.py Show resolved Hide resolved
src/open_inwoner/openzaak/api_models.py Show resolved Hide resolved
src/open_inwoner/cms/cases/views/cases.py Outdated Show resolved Hide resolved
src/open_inwoner/cms/cases/views/cases.py Show resolved Hide resolved
from .mixins import CaseAccessMixin, CaseLogMixin, OuterCaseAccessMixin

logger = logging.getLogger(__name__)


SUBMISSION_STATUS_OPEN = _("Openstaande aanvraag")
Copy link
Contributor

Choose a reason for hiding this comment

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

Double checking: will this value also be translated in the submissions themselves?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Translation in the template was indeed missing; I've added it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I didn't even think of that, but 👍 .

I actually meant this snippet:

 open_submissions = (
                open_submissions if SUBMISSION_STATUS_OPEN in statuses else []
            )

I was concerned whether there is a situation where SUBMISSION_STATUS_OPEN is translated but statsuses is not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The statuses are retrieved in openzaak/api_models.py and indeed not translated, since we don't have control over what statuses are returned. We can translate the "status" for open submissions since it is our own. Do you think it would be better to leave it untranslated as well for consistency?

    - retrieval of cases + open submissions is encapsulated
      in a dedicated service class
    - extract status_text processing of Zaak api model to
      separate property
@pi-sigma pi-sigma force-pushed the task/2645-case-filter-status branch from 75e375c to 0c8a157 Compare August 7, 2024 06:35
@alextreme alextreme merged commit e18dafe into develop Aug 12, 2024
18 checks passed
@alextreme alextreme deleted the task/2645-case-filter-status branch August 12, 2024 08:20
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