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

Improve speed of front page ‘recent problems’ query #4424

Merged
merged 1 commit into from May 10, 2023

Conversation

davea
Copy link
Member

@davea davea commented May 9, 2023

No description provided.

@davea davea force-pushed the front-page-recent-speed-up branch from 2b05bd5 to 28f2b94 Compare May 9, 2023 19:59
@davea davea requested a review from dracos May 9, 2023 20:00
@codecov
Copy link

codecov bot commented May 9, 2023

Codecov Report

Merging #4424 (10995ee) into master (051c9dc) will increase coverage by 0.00%.
The diff coverage is 62.50%.

❗ Current head 10995ee differs from pull request most recent head 30a1804. Consider uploading reports for the commit 30a1804 to get more accurate results

@@           Coverage Diff           @@
##           master    #4424   +/-   ##
=======================================
  Coverage   82.62%   82.62%           
=======================================
  Files         364      364           
  Lines       27471    27472    +1     
  Branches     4273     4276    +3     
=======================================
+ Hits        22697    22699    +2     
+ Misses       3489     3486    -3     
- Partials     1285     1287    +2     
Impacted Files Coverage Δ
perllib/FixMyStreet/DB/ResultSet/Problem.pm 95.90% <62.50%> (-2.39%) ⬇️

... and 1 file with indirect coverage changes

@davea davea force-pushed the front-page-recent-speed-up branch from 28f2b94 to 85ab0a4 Compare May 9, 2023 20:14
Copy link
Member

@dracos dracos left a comment

Choose a reason for hiding this comment

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

Odd test failure to investigate, and possible defaults to set - though as I say there, perhaps db sort is enough, and don't bother then sorting by confirmed. If you had:

id | created | confirmed
1 | 2023-05-10T08:00 | 2023-05-10T14:00
2 | 2023-05-10T09:00 | 2023-05-10T09:00
3 | 2023-05-10T10:00 | 2023-05-10T11:30
4 | 2023-05-10T11:00 | 2023-05-10T11:00
5 | 2023-05-10T12:00 | 2023-05-10T12:00
6 | 2023-05-10T13:00 | 2023-05-10T13:00

Before PR: 1, 6, 5, 3, 2
With PR: 6, 5, 3, 4, 2
With PR but no code sorting: 6, 5, 4, 3, 2
Not sure it matters if they're displaying as the last rather than the middle :shrug: :)

perllib/FixMyStreet/DB/ResultSet/Problem.pm Outdated Show resolved Hide resolved
@davea
Copy link
Member Author

davea commented May 10, 2023

Thanks for spotting the Zürich thing, fixed that in 10995ee. Might have fixed the test failure too, let's see.

I think it's worth ensuring they're ordered nicely. I know it's just a tiny thing but having something like this on the front page feels wrong:

image

Co-authored-by: Matthew Somerville <matthew@mysociety.org>
@davea davea force-pushed the front-page-recent-speed-up branch from 10995ee to 30a1804 Compare May 10, 2023 13:25
@davea davea merged commit 30a1804 into master May 10, 2023
10 of 17 checks passed
@davea davea temporarily deployed to github-pages May 10, 2023 13:35 — with GitHub Pages Inactive
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.

None yet

2 participants