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.

Sign up for GitHub

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

Jump to bottom

[#2007] Disable upload message if there are recently uploaded documents #961

Merged
merged 1 commit into from
Jan 23, 2024
Merged

[#2007] Disable upload message if there are recently uploaded documents #961

merged 1 commit into from
Jan 23, 2024

Conversation

pi-sigma
Copy link
Contributor

Taiga #2007

@pi-sigma pi-sigma force-pushed the fix/2007-document-upload-message branch from 55d99b7 to 490956c Compare January 19, 2024 08:40
@pi-sigma pi-sigma requested a review from stevenbal January 19, 2024 12:06
stevenbal
stevenbal requested changes Jan 19, 2024
View reviewed changes
Copy link
Contributor

@stevenbal stevenbal left a comment

Choose a reason for hiding this comment

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

Code looks good and I tested this locally and it works 👍

Only something seems to go wrong with the submodule reverting to an older commit

open-inwoner-design-tokens Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

I think your open-inwoner-design-tokens submodule is outdated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just realized this while testing locally. I'll update the tokens and push again.

Copy link
Member

Choose a reason for hiding this comment

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

Jiro has now linked the submodule to the main branch of open-inwoner-design-tokens, having an older version of this repo than main seems to be shown in a Github PR but doesn't affect our develop branch? It's a bit odd how Github deals with submodules, Bitbucket does this differently...

@pi-sigma pi-sigma force-pushed the fix/2007-document-upload-message branch 2 times, most recently from 70e4d7b to c4e919c Compare January 19, 2024 15:28
@pi-sigma pi-sigma marked this pull request as ready for review January 19, 2024 15:29
@pi-sigma
Copy link
Contributor Author

Submodules are fixed

alextreme
alextreme reviewed Jan 19, 2024
View reviewed changes
src/open_inwoner/cms/cases/views/status.py Outdated
@@ -383,6 +386,10 @@ def get_upload_info_context(self, case: Zaak):
zt_statustype_config.document_upload_description
)

# disable document upload message
if self.has_recently_uploaded_documents:
Copy link
Member

Choose a reason for hiding this comment

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

This isn't how this should be solved. The upload-message should only be hidden directly after you've uploaded your documents, and not be related to if you have recent documents or not.

Reason is that the municipality can also upload documents, so ensure we only hide the upload description after returning from CaseDocumentUploadFormView. I'd personally do this based on a GET-argument after a succesful upload

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I didn't consider this case.

@codecov-commenter
Copy link

codecov-commenter commented Jan 19, 2024 •
edited
Loading

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (c63452b) 94.77% compared to head (b4bfbe7) 94.76%.

Files Patch % Lines
src/open_inwoner/cms/cases/views/status.py 50.00% 2 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop     #961      +/-   ##
===========================================
- Coverage    94.77%   94.76%   -0.01%     
===========================================
  Files          861      861              
  Lines        30192    30196       +4     
===========================================
+ Hits         28614    28616       +2     
- Misses        1578     1580       +2

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

@pi-sigma pi-sigma marked this pull request as draft January 19, 2024 16:54
@pi-sigma pi-sigma force-pushed the fix/2007-document-upload-message branch 2 times, most recently from 64a5e32 to 9116355 Compare January 22, 2024 13:41
@pi-sigma pi-sigma requested a review from alextreme January 22, 2024 15:36
@pi-sigma pi-sigma force-pushed the fix/2007-document-upload-message branch from 9116355 to b4bfbe7 Compare January 23, 2024 08:40
@pi-sigma pi-sigma marked this pull request as ready for review January 23, 2024 09:12
@pi-sigma pi-sigma requested a review from alextreme January 23, 2024 13:25
@alextreme alextreme merged commit e3d539f into develop Jan 23, 2024
14 checks passed
@alextreme alextreme deleted the fix/2007-document-upload-message branch January 23, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevenbal stevenbal stevenbal approved these changes

@alextreme alextreme Awaiting requested review from alextreme

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pi-sigma @codecov-commenter @alextreme @stevenbal