Pre-calculate comment counts and activity scores #137
Conversation
Was a debugging artifact
* main: Truncate headings, too Polish filter menu
app/models/comment.rb
Outdated
| searchable_by :body, using: :comments_search_index | ||
|
|
||
| def captured_as(message) | ||
| message.bubble.increment! :comments_count |
kevinmcconnell
left a comment
There was a problem hiding this comment.
I think the general pattern here looks good -- we'll track the creation & deletion of comments, and increment/decrement a counter accordingly. Then we can cheaply sort bubbles by activity without having to count the associated records 👍
It would be nice if Comment could just track this directly, but I suppose you get into lifecycle problems where they're created before their corresponding Message. Could maybe work around that by using a builder method, but if that ends up feeling awkward then what you're doing here seems best.
app/models/bubble.rb
Outdated
| def activity_count | ||
| boost_count + messages.comments.size | ||
| def rescore | ||
| update! activity_score: boost_count + messages.comments.size |
There was a problem hiding this comment.
Since we're tracking the count of comments, can this be boost_count + comments_count?
Also at this point we could probably just sort by that expression rather than keep an activity_score column, given that all the information is in the other two columns. (That would be assuming we don't need to index this for performance).
There was a problem hiding this comment.
Ah yes I think at some point I overwrote my changes when deciding to remove #activity_count. Updated!
Re: sorting by expression — we also use this number for determining the size of bubbles. Which, it's just a simple addition, we could do it in-place as well. But since caching the literal score opens up doors to indexing, easy access, and ridiculously simple queries — I'm thinking let's keep the score in the db for now.
* main: (22 commits) Fix layout issue that caused comment form to be unselectable Stick with stock buttons for now Still a little odd that it's a separate form but this conceptually belongs here We've got a utility for this Fix navbar alignment Kind of hate that icon, let's try something else Use new bubble shape Prevent bubble getting squished Fix image fit, try animation Don't assume `@filter` is set Clicking assignees filters by that assignee Restore behavior: clicking tags filters by that tag Make these consist from view-to-view Improve hover motion for meta bubbles No longer used Improve assignee bubble display, especially when there are multiple assignees name -> key ** not required Swap one? and many? branches Change title based on selected buckets ...
|
|
||
| private | ||
| def created | ||
| bubble.comment_created if comment? |
There was a problem hiding this comment.
We considered this sort of design as well:
class Message < AR
after_create :captured
after_destroy :uncaptured
private
def captured
messageable.captured_as(self)
end
def uncaptured
messageable.uncaptured_as(self)
end
end
class Comment < AR
def captured_as(message)
message.bubble.comment_captured
end
def uncaptured_as(message)
message.bubble.comment_uncaptured
end
endBut it felt premature in that comments are the only kind of messageable (of which there's only two now) that needs to notify the bubble it's been captured.
We can migrate to that design if needed. Doing the simplest thing for now.
* main: Missed one Tighten up workflow panel Fix test Forget about the dividing line for now Add some structure to buckets index Adjust form Restore bucket settings link, adjust styles for filters No need for inline svg here anymore
This aligns better with counter_cache conventions, in case we turn boosts into a model later
These will make it easier to sort bubbles based on these attributes