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

This looks like a new option we added. I’m curious as to why we need it.

The fetching of post_edits_count and updating of the count needs to happen in a transaction. Otherwise, a race condition can happen where another process may have updated the count after we’ve fetch it from the database.

It looks like this method is currently only called from inside a transaction from PostRevisor.

    stat.increment!(:post_edits_count)

Still I think this query is better as it will not be subjected to concurrency issues if someone accidentally calls it outside a transaction in the future.

A recent performance problem we encountered has made me wary of queries that backfill records in one go. Can we update this query such that it backfills in batches instead? You can have a look at PERF: optimise backfilling of topic_id (#13545) · discourse/discourse@14a0247 · GitHub for the common pattern which we use.

Let’s avoid cosmetic changes here to avoid polluting the commit history.

@jmperez127 A couple of concerns but I think the PR is looking really good.

I think we should add the UI for filtering a report by the user but I’ll let @eviltrout decide if we should do it in this PR or a follow-up PR.

Screenshot from 2021-07-05 10-27-44

        {{#link-to "adminReports.show" "post_edits" (query-params filters=postEditsByEditorFilter) class="btn btn-icon"}}

Looks good to me too, but I agree with TGX the increment_edits_count option is unnecessary.

Also, Robin is off this week, I vote we address the UI for filtering edit reports by user in another PR.

Hey Alan, it was because of this. Do you suggest a better way to tackle this issue? https://meta.discourse.org/t/edit-log-by-user-audition-project/194791/16

sounds good, this was my editor though, I’ll be sure not to include these changes in the future.

The count revision needs to go through a central pipeline, either in a model callback or in this case the PostRevisor. Otherwise, the code may be calling PostRevisor from somewhere else and the count wouldn’t be kept updated.

I looked at the meta topic but it isn’t clear to me what the topics controller was doing such that it was revising the post?

Hey, it is because of this condition

if changes.length > 0
  first_post = topic.ordered_posts.first
  success = PostRevisor.new(first_post, topic).revise!(current_user, changes, validate_post: false)

In my case, the value of changes is {:tags=>[]} I’m not sure if this is the intended behavior or if it is a bug. What do you think?

I think it would be good to follow up for the filter as Jean suggested. If the PR otherwise looks good to you @tgxworld and @pmusaraj I’d like to merge!

@tgxworld can you review again?

I think this is a bug, if there are no changes in the number of tags, there shouldn’t be an edit made.

  disable_ddl_transaction!
  BATCH_SIZE = 30_000

The value of this variable is around line 377

This happens when you edit just the content of the initial post - even if there are no changes to the tags. I’m debugging to see if I can fix this issue, I’ll keep you posted but if you have any ideas let me know.

In Ruby, if the key doesn’t exist, it will automatically return nil :wink:

      editor_username = report.filters['editor']

Looks good to me! Please merge when you feel you are around to support the feature in case any bugs show up after we deploy.