Fix user activity pagination when there are hidden items#8202
Fix user activity pagination when there are hidden items#8202leio10 merged 6 commits intodecidim:developfrom
Conversation
leio10
left a comment
There was a problem hiding this comment.
Awesome work! 👏
I've found situations similar to this one and I wasn't able to build a query to include the relationships with all possible spaces at once.
| ) | ||
| end | ||
|
|
||
| query |
There was a problem hiding this comment.
💬 I'm a bit worried about the performance and the mantainability of these queries, but I can't find a better approach, so let's merge it and see how it works. 🤷♂️ 😕
There was a problem hiding this comment.
Regarding performance
Generally performance is not an issue to worry about unless the query takes a really long time to execute and it is run very often. This query falls into neither of those categories.
In fact, I think the general advice regarding performance is that you should do as much as possible at the DB layer.
This change will obviously make the query less performant than the original but here we are only talking about some milliseconds which is nothing.
I ran EXPLAIN ANALYZE on both of these queries on an instance with a short of 90k users and a user that has over 3k action log items in total or 1.5k public action log items that would be visible there.
The results are:
| Query | Planning time | Execution time | Total time |
|---|---|---|---|
| Original | 0.355ms | 0.064ms | 0.419ms |
| Modified (PR) | 5.560ms | 0.230ms | 5.790ms |
| Difference | 5.205ms (or 15.66x slower) | 0.166ms (or 3.59x slower) | 5.371ms (or 13.82x slower) |
So can you see it's significantly slower (~14-15 times slower) but we are only talking about a difference of ~5ms here. It's not going to be noticeable.
Regarding maintainability
You are correct that it makes it more difficult to maintain but I don't know another way which wouldn't be significantly less performant than the proposed solution. Such as pre-counting the visibility for all records which would take potentially a very long time with a lot of records. Maybe the private space activities should be completely hidden from the activity log (?), which would make it easier, but then it would change existing behavior.
For moderated action log items you could create some logic that would change a flag in the action logs table when the item is hidden by moderation. But I think that's conceptually wrong because then the action log items would have to be touched afterwards (I think the idea is to keep them intact).
For the private space items, I cannot think of a better way unless we create some kind of action log - user mapping indicating the visibility of the item per user. But that seems cumbersome and quite a lot more work as well.
* develop: (32 commits) Remove obsolete rake webpack task (#8237) Active storage migrations service (#7902) Fix content type delegation to blank attachments (#8230) Evote bug fixing (#8220) Fix the proposal data migration for proposals without authors or organization (#8015) Bump addressable version because security issues (#8229) Online meetings iframe visibility with time (#8097) Meetings iframe and iframe URL (#8096) Remove flaky test on meetings (#8226) Fix broken tests after problematic PRs (#8224) Apply permissions system to comments (#8035) Set current_component as commentable when commentable is a participatory space (#8189) Fix don't require inactive authorization handlers (#8122) Improve metrics calculations performance (#8215) Fix performance issue in notification settings page (#8155) Active storage migration (#7598) Update manual installation guide in documentation (#8217) Load JS configuration in elections focus mode layout (#8213) Fix user activity pagination when there are hidden items (#8202) Make it possible to define SCSS settings overrides from modules (#8198) ...
🎩 What? Why?
Right now if there are hidden items (e.g. private space items or moderated items) included in the user's activity page, they will mess up the pagination on that page.
This happens because rather than filtering out these items directly from the results, the activities cell will call
visible_for?for each of the activity items individually:decidim/decidim-core/app/cells/decidim/activities_cell.rb
Line 31 in 370ae76
This is detached from the activity results which is used for the pagination:
decidim/decidim-core/app/cells/decidim/user_activity/show.erb
Line 28 in 370ae76
This fixes the issue by directly filtering out these items from the activity query.
Testing
📋 Checklist
docs/.