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

[#2098] Show case-related questions in case detail view #1059

Merged
merged 4 commits into from
Mar 15, 2024

Conversation

pi-sigma
Copy link
Contributor

@pi-sigma pi-sigma commented Feb 27, 2024

Taiga: #2098

Related: PR #1045

@pi-sigma pi-sigma force-pushed the feature/2098-link-vragen-aanvragen branch 4 times, most recently from bceaad1 to 931d85b Compare February 28, 2024 12:00
@pi-sigma pi-sigma force-pushed the feature/2098-link-vragen-aanvragen branch 4 times, most recently from 75f91e7 to e780963 Compare March 1, 2024 11:52
@pi-sigma pi-sigma marked this pull request as ready for review March 1, 2024 12:31
@pi-sigma pi-sigma requested review from stevenbal and jiromaykin March 1, 2024 12:34
@pi-sigma
Copy link
Contributor Author

pi-sigma commented Mar 1, 2024

@stevenbal @jiromaykin I thought it makes sense to paginate the questions for particular cases, though it is not entirely straightforward (we need to paginate inside a detail view + the query param for the page is not easily accessible because of the inner/outer view split for cases). Let me know if this functionality is not needed, then I'll get rid of it.

I just saw that this was requested as part of a follow-up (Taiga #2148)

@jiromaykin
Copy link
Contributor

[...] we need to paginate inside a detail view + the query param for the page is not easily accessible because of the inner/outer view split for cases). Let me know if this functionality is not needed, then I'll get rid of it.

So, I think we agree the pagination is not needed for the detailview - the detailview needs a collapse/readmore button.
The other task (2148) about the pagination was just a pure styling task (make pagination centered on all views, so make them look the same through the entire project, but that task is for me).

Copy link
Contributor

@jiromaykin jiromaykin left a comment

Choose a reason for hiding this comment

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

I need more 'Vragen' data in order to properly review this.

src/open_inwoner/templates/pages/cases/status_inner.html Outdated Show resolved Hide resolved
src/open_inwoner/templates/pages/cases/status_inner.html Outdated Show resolved Hide resolved
@pi-sigma pi-sigma force-pushed the feature/2098-link-vragen-aanvragen branch 2 times, most recently from 96c8b93 to 1fb0ac6 Compare March 4, 2024 15:11
@jiromaykin jiromaykin self-requested a review March 4, 2024 15:47
@pi-sigma pi-sigma force-pushed the feature/2098-link-vragen-aanvragen branch 2 times, most recently from 4b14943 to e59fb16 Compare March 4, 2024 17:43
@codecov-commenter
Copy link

codecov-commenter commented Mar 4, 2024

Codecov Report

Attention: Patch coverage is 94.78261% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 95.01%. Comparing base (3bbd1a3) to head (c8f3f74).
Report is 28 commits behind head on develop.

Files Patch % Lines
src/open_inwoner/openklant/clients.py 80.00% 4 Missing ⚠️
src/open_inwoner/accounts/views/contactmoments.py 86.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           develop    #1059    +/-   ##
=========================================
  Coverage    95.00%   95.01%            
=========================================
  Files          896      909    +13     
  Lines        31449    31917   +468     
=========================================
+ Hits         29879    30325   +446     
- Misses        1570     1592    +22     

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

@pi-sigma pi-sigma force-pushed the feature/2098-link-vragen-aanvragen branch 6 times, most recently from 5835a93 to 769b099 Compare March 6, 2024 17:02
@pi-sigma pi-sigma force-pushed the feature/2098-link-vragen-aanvragen branch from 701449a to c1c6ab9 Compare March 12, 2024 08:19
@pi-sigma pi-sigma force-pushed the feature/2098-link-vragen-aanvragen branch 2 times, most recently from c967536 to bfb2dfc Compare March 14, 2024 09:10
@pi-sigma pi-sigma marked this pull request as draft March 14, 2024 09:10
src/open_inwoner/cms/cases/views/status.py Outdated Show resolved Hide resolved
src/open_inwoner/openzaak/tests/test_case_detail.py Outdated Show resolved Hide resolved
src/open_inwoner/cms/cases/tests/test_contactform.py Outdated Show resolved Hide resolved
@pi-sigma pi-sigma force-pushed the feature/2098-link-vragen-aanvragen branch from bfb2dfc to 51e14f2 Compare March 14, 2024 09:46
Copy link
Contributor

@jiromaykin jiromaykin left a comment

Choose a reason for hiding this comment

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

I am not sure how to fix the tangled javascript, but if we change the 'dropdown' class into an 'expand' class than this may work as an intermediate solution.
Also some small requests to change the Card SCSS and some html classes for the primary buttons.

src/open_inwoner/js/components/card/ToggleHideSelection.js Outdated Show resolved Hide resolved
src/open_inwoner/templates/pages/questions/questions.html Outdated Show resolved Hide resolved
{# redirect to fetch klantcontactmoment from contactmoment #}
<a href="{% url 'cases:kcm_redirect' contactmoment.uuid %}" class="card card--compact card--stretch card--toggle-hide contactmomenten__link">
<div class="card__body">
{% render_list %}
Copy link
Contributor

Choose a reason for hiding this comment

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

There might not be a need to use a UL or list-items inside this card.
But I am not sure if this is the definite design:
https://www.figma.com/file/iKGhWhstaLIlFSaND2q7cE/OIP---Designs-(new)?type=design&node-id=1839%3A12002&mode=design&t=BLYIn4BeQOFtBCyU-1

src/open_inwoner/js/components/card/ToggleHideSelection.js Outdated Show resolved Hide resolved
src/open_inwoner/js/components/card/ToggleHideSelection.js Outdated Show resolved Hide resolved
src/open_inwoner/templates/pages/cases/status_inner.html Outdated Show resolved Hide resolved
src/open_inwoner/scss/components/Card/Card.scss Outdated Show resolved Hide resolved
@pi-sigma pi-sigma force-pushed the feature/2098-link-vragen-aanvragen branch from 51e14f2 to c4159e8 Compare March 14, 2024 15:51
@pi-sigma pi-sigma force-pushed the feature/2098-link-vragen-aanvragen branch from c4159e8 to 1dbbf08 Compare March 14, 2024 21:21
@pi-sigma pi-sigma force-pushed the feature/2098-link-vragen-aanvragen branch from 1dbbf08 to 1025af6 Compare March 15, 2024 09:23
@pi-sigma pi-sigma force-pushed the feature/2098-link-vragen-aanvragen branch from 1025af6 to fcddaf8 Compare March 15, 2024 10:17
@pi-sigma pi-sigma marked this pull request as ready for review March 15, 2024 10:18
Comment on lines +536 to +537
created=dateutil.parser.parse(
self.zaak_informatie_object_old["registratiedatum"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Note for other reviewers: we discussed this and dateutil.parser.parse is acceptable here because the zgw_consumers factory() also uses that and it produces different timezones then datetime.fromisoformat so the equality check failed.

There might be a way to convert it but let's declare that out of scope for this PR because it is a hassle.

Copy link
Contributor

@Bartvaderkin Bartvaderkin left a comment

Choose a reason for hiding this comment

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

👍 Let's go! 🚀

@stevenbal stevenbal requested a review from jiromaykin March 15, 2024 11:40
@pi-sigma pi-sigma force-pushed the feature/2098-link-vragen-aanvragen branch from fcddaf8 to c8f3f74 Compare March 15, 2024 11:53
@alextreme alextreme merged commit 9397863 into develop Mar 15, 2024
15 checks passed
@alextreme alextreme deleted the feature/2098-link-vragen-aanvragen branch March 15, 2024 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants