FEATURE: Publish read topic tracking events for private messages. (PR #14274)

Follow-up to fc1fd1b41689694b3916dc4e10605eb9b8bb89b7

GitHub

No longer required because the SQL query doesn’t include highest_post_number anymore.

No longer required because the SQL query doesn’t include highest_post_number anymore.

No idea what this comment is referring to.

How come we can’t just call publish_read_message directly instead of having publish_read call it? (ditto for the other state class)

Overall looks good just a few clarifications.

I think it is because there is an msecs argument for this function and there was a bigger comment closer to the top:

But yeah not really necessary

I don’t get why this is necessary when both classes have the same method and parameters?

This is to avoid leaking the message_type and channel concerns into TopicUser

This method here is to decide which class of topic tracking state it should use. I didn’t want to duplicate this logic in multiple places in the class.

Oh yes I see now, thanks

Oh right of course not sure what I was thinking with this comment…carry on!

Thank you for the review @martin-brennan and @lis2 :hugs: