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

Fix in memoriam accounts appearing in follow recommendations #31474

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

wheatear-dev
Copy link
Contributor

@wheatear-dev wheatear-dev commented Aug 17, 2024

Further to #30466, this fix prevents in memoriam accounts from appearing in accounts' follow recommendations.

This is achieved by adding an extra restriction (memorial: false) in both of the following sources of these recommendations:

  • app/models/account_suggestions/source.rb
  • app/models/account_suggestions/friends_of_friends_source.rb

Note - this does not invalidate any caches upon an admin "memorialising" a user. Since this action could hypothetically affect the recommendation cache for any account, and due to memorialisation being a one-off and irreversible operation, I did not see this as necessary.

@wheatear-dev wheatear-dev force-pushed the fixes/recommend-in-memoriam branch from eb2ad37 to b2cfa63 Compare August 17, 2024 18:28
@wheatear-dev wheatear-dev marked this pull request as ready for review August 17, 2024 18:39
@wheatear-dev wheatear-dev marked this pull request as draft August 18, 2024 16:42
@wheatear-dev

This comment was marked as outdated.

@wheatear-dev wheatear-dev force-pushed the fixes/recommend-in-memoriam branch 3 times, most recently from ad41a3b to a0ff865 Compare August 18, 2024 17:11
@wheatear-dev wheatear-dev marked this pull request as ready for review August 18, 2024 17:13
@wheatear-dev
Copy link
Contributor Author

When (or if 😅) this is merged, please preserve the notice of co-authorship by @kernal053 !

Should be as simple as leaving the commit message untouched, but I am not especially familiar with how this project uses GitHub PR's. 😌

@renchap renchap requested a review from a team August 19, 2024 11:33
ClearlyClaire
ClearlyClaire previously approved these changes Aug 19, 2024
Copy link
Contributor

@ClearlyClaire ClearlyClaire left a comment

Choose a reason for hiding this comment

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

Hi! Thanks for your contribution! The change looks good to me, but could you add a couple tests in spec/models/account_suggestions for this?

(As an aside, the follow_recommendations materialized view will still store records for memorialized accounts, which is a (small) waste of resources, but this is ok because the number of trending memorialized accounts should be low. Furthermore, changing follow_recommendations is much more complex as it involves tricky database migrations)

Gargron
Gargron previously approved these changes Aug 19, 2024
@wheatear-dev wheatear-dev force-pushed the fixes/recommend-in-memoriam branch from a0ff865 to bb75dd9 Compare August 19, 2024 13:40
@renchap
Copy link
Member

renchap commented Aug 19, 2024

(As an aside, the follow_recommendations materialized view will still store records for memorialized accounts, which is a (small) waste of resources, but this is ok because the number of trending memorialized accounts should be low.

We were discussing something similar for not ignoring "bot" accounts from recommendations, and thought about having the query return a bit more results, then filtering in Ruby to ignore the ones we dont want and limit to 30.

Not sure if this is required here as I expect memorialized accounts to be less frequent than bot accounts in trends, but this may be a solution.

@ClearlyClaire
Copy link
Contributor

In this case, this is ok as far as the actual results are concerned, the filtering is done in PostgreSQL and will return the expected number of results. The potential concern is about performance, but I do not think this is a concern for memorialized accounts since there are few of it.

Filters out memorial accounts in the two account suggestions sources.

Co-authored-by: Utkarsh Wankar <[email protected]>
@wheatear-dev wheatear-dev dismissed stale reviews from ClearlyClaire and Gargron via 740aeeb August 19, 2024 15:31
@wheatear-dev wheatear-dev force-pushed the fixes/recommend-in-memoriam branch from bb75dd9 to 740aeeb Compare August 19, 2024 15:31
@renchap renchap added this pull request to the merge queue Aug 19, 2024
Merged via the queue into mastodon:main with commit d4f135b Aug 19, 2024
29 checks passed
@wheatear-dev wheatear-dev deleted the fixes/recommend-in-memoriam branch August 19, 2024 16:08
ponapalt pushed a commit to ponapalt/mastodon that referenced this pull request Aug 19, 2024
ponapalt pushed a commit to ponapalt/mastodon that referenced this pull request Aug 19, 2024
justinwritescode pushed a commit to justinwritescode/mastodon that referenced this pull request Sep 15, 2024
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