FEATURE: Add post edits count to user activity (PR #13495)

As requested in this topic Can we have an Edit Log please? - #10 by markvanlan - feature - Discourse Meta this PR adds a post edits count in user activity section (admin dashboard).

See it in action

GitHub

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

If a param is null I suspect you should be able to omit it.

Overall this is looking VERY good! I’ve made some comments.

Another nitpick from the screenshots: can we add more spacing to the table displayed in the modal? It looks very scrunched in there.

Does this field have HTML? I suspect it’s just a number and we can do {{model.post_edits_count}}

My preference would be for this to use a gray button that is over to the right like the other fields on the admin page like “Show Flags Received”. I know this doesn’t match the spec we sent over but seeing it in the screenshots I would prefer it to be consistent with the other admin controls.

It would be more future proof to make this two tests, one for each field. You might need to put one in a <span> or something to target it directly with a querySelector but it’s worth it!

Nice catch! It should be consistent with the file name :+1:

This is a trivial thing but I think it would be more readable as a multi line if/else:

if editor_id
  builder.where("editor.id = ?", editor_id)
else
  builder.where("editor.id > 0 AND editor.id != author.id")
end

I like putting conditions on the end of lines but in this case you’re doing it on two lines in succession with opposite conditions.

I’m curious about this - if it’s updated in post revisor why would we also need to update the denormalized counts here?

    UserStat.create!(new_since: Time.zon.now, user_id: id)
    stat.update!(post_edits_count: stat.post_edits_count + 1)

Should we only increment the count when the post has been successfully saved?

        expect { subject.revise!(post.user, raw: "This is a new revision") }.to change { coding_horror.user_stat.reload.post_edits_count }.by(1)

Instead of repeating the same assertion twice, there is a nice RSpec matcher we can use here.

      fab!(:post_revision) { Fabricate(:post_revision, user: user) }
      fab!(:post) { post_revision.post }

At Discourse, we prefer to use fab! to prefabricate objects for performance reasons.

Agree with @eviltrout here, I don’t think we need to correct the counts here for users on an hourly basis.

      before do
        user.update!(:last_seen_at, 1.second.ago)
        expect(report_with_two_edits.data.count).to be(2)
      fab!(:posts) { Fabricate.times(3, :post) }

      fab!(:editor_with_two_edit) do 
        Fabricate(:user).tap do |user|
          2.times do |i|
            posts[i].revise(user, raw: "edit #{i + 1}")
          end
        end
      end

      fab!(:editor_with_one_edit) do 
        Fabricate(:user).tap do |user|
          posts.last.revise(user, raw: "edit 3")
        end
      end

      let(:report_with_one_edit) do
        Report.find('post_edits', { filters: { 'editor_id' => editor_with_one_edit.id } })
      end

      let(:report_with_two_edits) do
        Report.find('post_edits', { filters: { 'editor_id' => editor_with_two_edits.id } })
      end

Small suggestion for the fabrication to make ti clearer.