HyphaApp / hypha

Submission management software for open calls
https://www.hypha.app
BSD 3-Clause "New" or "Revised" License
67 stars 39 forks source link

Speed & appearance improvements to the results page #4043

Closed wes-otf closed 2 weeks ago

wes-otf commented 1 month ago

Fixes #3941. This makes some speed improvements to the results page along with fixes to the appearance of some currency values. Some of the biggest improvements come with the initial loading of the /submissions/result/ with no filters/query params applied, the .values() for all submissions were being calculated twice. After some crude benchmarking with very large (OTF scale) datasets, it seems to be ~26% faster. Other improvements were just not loading fields that were irrelevant to the data presented.

Testing of initial loading of /submissions/result/ was as follows:

Unoptimized

Trial SQL Time (ms) SQL Queries
1 3715.86 32
2 4429.34 33
3 4449.12 33
4 4615.63 33
5 4852.89 33

Average SQL Time: 4412.57

Optimizied

Trial SQL Time (ms) SQL Queries
1 3006.05 32
2 3448.47 32
3 3315.12 32
4 3317.39 32
5 3137.71 32

Average SQL Time: 3244.95

Hopefully this leads to less crashing on production. If you see anything else that can be optimized let me know!

Test Steps

wes-otf commented 1 month ago

I also removed the search functionality for now. It seems like it's hard for the user to know what data is being pulled in that (ie. if you search wes@test.org are you getting stats from applications where wes@test.org is a applicant, mentioned within the application, etc). This also allowed for deferring of the search_document & search_data fields which can be huge text fields.

frjo commented 1 month ago

@wes-otf I discovered a bug in my original code with the use of intcomma so I attempted to fix it.

I guess I started using intcomma and later added babel currency conversion. They do not work together. That why we needed to remove commas but that only works for some locales.

Hopefully this fix makes it work for all locales.

wes-otf commented 1 month ago

I think this is ready for test if you also agree @frjo

frjo commented 2 weeks ago

Filtering on lead and picking Abhishek results in this error https://otf.sentry.io/issues/5681941380/?project=5823154&query=is%3Aunresolved+issue.priority%3A%5Bhigh%2C+medium%5D&referrer=issue-stream&statsPeriod=14d&stream_index=0

Seems to be a amount that is badly formed and babel choke on it.

frjo commented 2 weeks ago

I think I found the issue. If none of the listed submissions have a amount you get the above error.

I added an amount to https://test-apply.hypha.app/apply/submissions/681/ and now it work

wes-otf commented 2 weeks ago

thanks for testing this! When the there's no fields the total value gets passed to format_number_as_currency as None, so catching this and swapping it to 0 seems to fix this issue both when there are no applications with the Requested Amount field & when the field exists on applications but it just not populated.