FEATURE: Show draft count in user menu and activity (PR #13812)

This commit adds the number of drafts a user has next to the “Draft” label in the user preferences menu and activity tab. The count is updated via MessageBus when a draft is created or destroyed.

GitHub

Why don’t you use a single string to handle all the counts?

I noticed that we do not show the count when it is 0. We prefer to show “Drafts” over “Drafts (0)”.

We can just have the translation do that.

Example

drafts:
  label:
    zero: "Drafts"
    other: "Drafts (%{count})"

Ooooh, that’s what you meant. I forgot we can do that.

Unfortunately, this is not possible because of the pluralization rules for English.

How is this query performing on db with a large number of users?

Also, instead of updating all the rows, you could add a condition to only update changed counts

   AND draft_count <> new_user_stats.draft_count

With your optimization, for a forum with 30k users and 3k drafts, it takes about 30ms.

1 Like
    messages = MessageBus.track_publish("/user") do
      Draft.set(user, "test", 0, "data")
    end

    expect(messages.first.data[:draft_count]).to eq(1)
  describe '.update_draft_count' do
    messages = MessageBus.track_publish("/user") do
      Draft.where(user: user).destroy_all
    end

    expect(messages.first.data[:draft_count]).to eq(0)

There is a fair bit of duplication here so I think moving UserStat.update_draft_count will help DRY the code up.

Also we can probably do something like

after_commit :update_draft_count, on: [:create, :destroy]

Are we able to remove this method? It doesn’t seem to be used.

I’m wondering if we need to reconcile periodically when we’re confident that we’re always updating the counter cache properly. Maybe just do away with this for now until a bug in the draft count shows up? Even if there is a spot we miss somewhere, creating or removing a new draft will fix the count.

We need a data migration here. Otherwise, a draft count of 0 shows up for everyone until a draft has been created/removed.

I added a post migration.

We do that for other fields too. I think you are right and I removed that part.

Sorry quick comment so we don’t need to wait another day of back and fourth.

I think this needs to be a migration and not a post migration. Otherwise, the counts will appear broken until the deploy is completed and post migrations have run. Also, we’ve been bitten multiple times by a single giant update query where the query ends up timing out on large site. I think it’ll be better to migrate in batches instead.

I made this a post-migration because I was concerned that it could be a bit more slower than I expected on gigantic sites. Making this run in batches is quite difficult because of the subquery. The LIMIT and OFFSET will make this query run very slow.