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

[#927] Added cache decorator to some case-views to improve navigation #380

Merged
merged 4 commits into from
Dec 13, 2022

Conversation

Bartvaderkin
Copy link
Contributor

No description provided.

@Bartvaderkin Bartvaderkin marked this pull request as ready for review December 12, 2022 13:54
@codecov-commenter
Copy link

codecov-commenter commented Dec 12, 2022

Codecov Report

Merging #380 (1e313f4) into develop (7d17b02) will increase coverage by 0.12%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #380      +/-   ##
===========================================
+ Coverage    95.92%   96.05%   +0.12%     
===========================================
  Files          457      461       +4     
  Lines        14471    14842     +371     
===========================================
+ Hits         13881    14256     +375     
+ Misses         590      586       -4     
Impacted Files Coverage Δ
src/open_inwoner/accounts/views/cases.py 97.38% <100.00%> (ø)
src/open_inwoner/conf/base.py 94.96% <100.00%> (+0.06%) ⬆️
src/open_inwoner/openzaak/cases.py 70.66% <100.00%> (+5.38%) ⬆️
src/open_inwoner/openzaak/catalog.py 52.63% <100.00%> (+1.72%) ⬆️
src/open_inwoner/openzaak/documents.py 92.85% <100.00%> (+1.02%) ⬆️
...rc/open_inwoner/openzaak/tests/test_case_detail.py 100.00% <100.00%> (ø)
...c/open_inwoner/openzaak/tests/test_case_request.py 100.00% <100.00%> (ø)
src/open_inwoner/openzaak/tests/test_cases.py 100.00% <100.00%> (ø)
src/open_inwoner/openzaak/tests/test_documents.py 100.00% <100.00%> (ø)
src/open_inwoner/accounts/models.py 97.77% <0.00%> (-0.88%) ⬇️
... and 24 more

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

Copy link
Member

@alextreme alextreme left a comment

Choose a reason for hiding this comment

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

Maybe it would be good to make these durations into Django settings?

I'm doubting the zaak/informatieobject caching, it might be viable for ~30 seconds but 3 minutes feels a bit too long. And with 30 seconds it might just not be worthwhile anymore.

Feel free to discuss this with Anna if you want. Could also be something that we enable after the demo.

… calls only used in document-download view
@Bartvaderkin
Copy link
Contributor Author

@alextreme I appended the PR with settings for the timeouts and removed caching on two calls that currently don't need it because they are only used in the download view.

@Bartvaderkin Bartvaderkin requested a review from vaszig December 13, 2022 09:45
@Bartvaderkin Bartvaderkin changed the title Added cache decorator to some case-views to improve navigation [#927] Added cache decorator to some case-views to improve navigation Dec 13, 2022
Copy link
Contributor

@vaszig vaszig left a comment

Choose a reason for hiding this comment

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

For fetch_specific_status function (openzaak/cases) we don't use a variable because it's only used once right?

src/open_inwoner/openzaak/cases.py Outdated Show resolved Hide resolved
@alextreme alextreme merged commit 44f3973 into develop Dec 13, 2022
@alextreme alextreme deleted the issue/927-cases-caching branch December 13, 2022 11: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