mumuki-laboratory icon indicating copy to clipboard operation
mumuki-laboratory copied to clipboard

Order discussions by last message date on profile

Open felipecalvo opened this issue 3 years ago • 0 comments

Currently, on the profile view, discussions are exclusively ordered by their read/unread subscription status. Right now I'm adding on https://github.com/mumuki/mumuki-domain/pull/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.

felipecalvo avatar Aug 04 '21 16:08 felipecalvo