FEATURE: Display pending posts on user’s page (PR #14501)


I’m with @ZogStriP on this one, we should have the username param in this route, i.e. posts/:username/pending and then pull the user from the params. This allows staff members to debug issues if/when necessary and it would be consistent with similar routes (deleted_posts, flagged_posts).

That’s what I thought at first but then Gerhard explained me why: https://github.com/discourse/discourse/pull/14501#discussion_r722258300 (and we have a linter in place to avoid using zero)

I’m used to avoid putting logic directly in the views but if having a CP used only once is a smell then I’ll change it :wink:

Yes we already discussed about that here: FEATURE: Display pending posts on user’s page by Flink · Pull Request #14501 · discourse/discourse · GitHub As there are only two events triggered here (as of today), using their keys allows to just send pending_posts_count and drafts_count (if I’m not mistaken for the latter). It’s simpler to just listen to the right event instead of having to listen to a more global one and then check if the key we want is in there. I still could prefix the events with counter: as I suggested previously if you prefer?

This will add unnecessary complexity both on client side and server side IMO:

  • server side we’ll have to get the user from the params, check if they can access pending posts for that user, etc.
  • client side it’s more or less the same, we need to check current user can see pending posts through the user route (so yes for admins and owner but no for other users).

If we didn’t have access to pending posts through an other existing UI I probably wouldn’t argue here, just to make my position clearer :sweat_smile: But if that’s the way we want to do it then I guess I’ll make the change.

Oops sorry! Thanks for the clarification.

I’d totally forgotten! But yes, count-updated: or similar is necessary here. We can’t have events without namespaces.

Ok done, just need to adapt some tests and write some new ones.

Moved this method as public since I find it quite useful when it’s needed to know if guardian is the current user.

All comments have been addressed :slight_smile: