Extract AccountFollowable concern for Account.followable_by scope

[mjankowski]

Pulled out from #26093

This scope was sort of complicated and hard to follow and parse visually.

This change pulls the whole thing out into a concern which does nothing but build the scope back up from better-named private methods. From my local testing, a) the generated sql hasn't changed, b) the specs which exercise this method seem fine with the change.

[github-actions]

This pull request has merge conflicts that must be resolved before it can be merged.

[github-actions]

This pull request has resolved merge conflicts and is ready for review.

[github-actions]

This pull request has merge conflicts that must be resolved before it can be merged.

[github-actions]

This pull request has resolved merge conflicts and is ready for review.

[github-actions]

This pull request has merge conflicts that must be resolved before it can be merged.

[github-actions]

This pull request has resolved merge conflicts and is ready for review.

[mjankowski]

Updated this to move the whole query generation to a new class, and remove the scope from Account since its only ever used in the suggestion area. I think the goal of making it less dense and more readable is better with this approach than just the extraction to the concern.

The suggestions classes, and maybe the new query class could probably be further simplified. I suspect that at least part of the query generation does not need the full low level arel treatment it's currently using, but haven't verified in detail.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3f3df9a) 84.83% compared to head (9673d43) 84.84%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #26179      +/-   ##
==========================================
+ Coverage   84.83%   84.84%   +0.01%     
==========================================
  Files        1038     1039       +1     
  Lines       28142    28161      +19     
  Branches     4536     4536              
==========================================
+ Hits        23874    23893      +19     
  Misses       3103     3103              
  Partials     1165     1165              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mjankowski]

Rebased and moved to Account:: concerns module.

[mjankowski]

Rebased this again after the introduction of spec coverage in #28689

Current version of this is pull out the scope creation to a concern module, building two smaller scopes with the conditions, and then combining them. I think this is a net-improvement as-is, but would like feedback. If this is not good to go, the other idea I have is to move (and expand) the coverage to be directly on AccountSuggestions::Source (the only place this is used), and then shift the scope generation into that class and out of Account.

[ClearlyClaire]

Taking a step back from this query, it heavily uses arel to build queries of the form LEFT OUTER JOIN … ON … WHERE id = NULL , which can be expressed in a somewhat simpler (and sometimes more efficient) way with WHERE NOT EXISTS (SELECT 1 FROM … WHERE …). Unfortunately, neither can be easily written with ActiveRecord itself and would require either raw SQL or extensive use of arel.

Given that this query is only ever used in AccountSuggestions::Source, I would move it there and use raw SQL, as this file already does for another part of the query, and in a similar way AccountSuggestions::FriendsOfFriendsSource already does.

Something like:

  def followable_by(account)
    Account.where.not('EXISTS (SELECT 1 FROM follows f WHERE f.target_account_id = accounts.id AND f.account_id = :id)', id: account.id)
           .where.not('EXISTS (SELECT 1 FROM follow_requests f WHERE f.target_account_id = accounts.id AND f.account_id = :id)', id: account.id)
  end

[mjankowski]

Great. I'll move the coversage over to account suggesstion spec, expand it a bit, and then explore something along those lines.

[mjankowski]

Closing this, goal accomplished by linked PR.