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

Order discussions by last message date on profile #1650

Open
felipecalvo opened this issue Aug 4, 2021 · 0 comments
Open

Order discussions by last message date on profile #1650

felipecalvo opened this issue Aug 4, 2021 · 0 comments

Comments

@felipecalvo
Copy link
Contributor

Currently, on the profile view, discussions are exclusively ordered by their read/unread subscription status. Right now I'm adding on mumuki/mumuki-domain#233 a sub-order criteria, the discussion creation date, as to provide a better approximation to what we'd really like to do - ordering by last message date, which is the date shown on the view.

The problem is the last message date is a calculated value. A message can only be considered if it's visible, which is currently done like

  scope :visible, -> () do
    where.not(deletion_motive: :self_deleted)
      .or(where(deletion_motive: nil))
      .where(assignment_id: nil)
  end

at https://github.com/mumuki/mumuki-domain/blob/97a0b79d492d873e296b6f34e5e90407f1da3818/app/models/message.rb#L17-L23.

While that's quite a complex scope to insert in a reorder method, even if we could do it we still need the created_at field of the very last message - we don't really care about all the others. So it's not really a join, I think, since that would show the discussion as many times as messages it has. I could SQL my way through it, I guess, but that'd not be ideal.

Another less-than-optimal way would be getting every discussion and sorting the array. However, that defeats the whole purpose of the pagination, as we need to query every discussion to reorder. Here's an incomplete approach:

  def discussions
    @watched_discussions = Kaminari
                             .paginate_array(current_user
                                               .watched_discussions_in_organization
                                               .sort { |d, e| d.last_message_date <=> e.last_message_date }
                                               .reverse!)
                           .page(params[:page]).per(10)
  end

Even with terrible performance, it still doesn't work as it should because now we're sorting by last message date only - we should get all unread first and sort those; then get all read and sort those. I don't think this approach is feasible.

The way to go should be denormalizing the last message date, I think. That would make it as easy as

scope :unread_first, -> { includes(:subscriptions).reorder('subscriptions.read', :last_message_date) }

which is very quick since the ordering is done on the DB side, and also allows for proper pagination. What's slightly more complex is that you have to consider visible messages on the denormalization. That is, if a message is deleted, the last message date should be reverted to the now-last message (if there even is one). Messages don't get deleted that often but it should still be considered.

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

No branches or pull requests

1 participant