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

perf(likes): limit likes relationship results #3781

Merged
merged 18 commits into from
Apr 19, 2023
Merged

perf(likes): limit likes relationship results #3781

merged 18 commits into from
Apr 19, 2023

Conversation

SychO9
Copy link
Member

@SychO9 SychO9 commented Apr 1, 2023 •
edited
Loading

Supersedes #3585

Changes proposed in this pull request:
Similar to #3780, it limits the results of the likes relationship of a post, and paginates all results in the existing modal.
The PR makes sure that the actor ID is in the limited results of the relationship, so that we can show the proper translation value that has You liked and others....

Reviewers should focus on:
Changes here are based on the linked PR, so they can only be merged after that.

Screenshot
Screenshot from 2023-04-01 15-25-11
Screenshot from 2023-04-01 15-25-19

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

SychO9 and others added 6 commits March 31, 2023 21:43
@SychO9
perf(core,mentions): limit mentionedBy post relation results
e538a41 
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
@StyleCIBot
Apply fixes from StyleCI
3018210
@SychO9
chore: use a static property to allow customization
bb05b3f 
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
@SychO9
chore: use a static property to allow customization
c94d28a 
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
@SychO9
chore: include count in show post endpoint
f8c178b 
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
@SychO9
chore: consistent locale key format
7b7de9f 
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
@SychO9 SychO9 requested a review from a team as a code owner April 1, 2023 15:13
@SychO9 SychO9 mentioned this pull request Apr 1, 2023
Closed
10 tasks
@SychO9 SychO9 added this to the 1.8 milestone Apr 1, 2023
@SychO9 SychO9 changed the title perf(likes): limit likes relationship results perf(likes): limit likes relationship results Apr 1, 2023
SychO9 and others added 10 commits April 1, 2023 23:55
@SychO9
chore: forgot to delete FilterVisiblePosts
5644c31 
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
@SychO9
test: mentionedByCount must not include invisible posts to actor
9cc3613 
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
@SychO9
fix: visibility scoping on mentionedByCount
1045aa8 
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
@SychO9
fix: loadAggregates conflicts with visibility scopers
9613865 
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
@StyleCIBot
Apply fixes from StyleCI
7f6f8ce
@SychO9
chore: phpstan
f00f6a8 
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
@SychO9
perf(likes): limit likes relationship results
922fd84 
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
@StyleCIBot @SychO9
Apply fixes from StyleCI
4e2e7da
@SychO9
chore: simplify
92ce0c0 
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
@SychO9
test: likesCount is as expected
42fc79b 
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
luceos
luceos approved these changes Apr 4, 2023
View reviewed changes
Copy link
Member

@luceos luceos left a comment

Choose a reason for hiding this comment

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

Looking good, even seeing improvements locally with the discuss data.

Base automatically changed from sm/3705-mentions-perf to main April 19, 2023 07:23
@imorland imorland merged commit d0669b0 into main Apr 19, 2023
@imorland imorland deleted the sm/likes-perf branch April 19, 2023 08:22
@github-actions github-actions bot mentioned this pull request May 17, 2023
Closed
@imorland imorland mentioned this pull request Jun 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luceos luceos luceos approved these changes

@imorland imorland imorland approved these changes

Assignees
No one assigned
Labels
prio/high type/performance
Projects
None yet
Milestone
1.8
Development

Successfully merging this pull request may close these issues.
4 participants
@SychO9 @luceos @imorland @StyleCIBot