[openstreetmap/openstreetmap-website] Note: Add body & author (PR #4481)

[Gregory Igelmund]

@grekko commented on this pull request.



> +  desc "Backfills notes-columns `body`, `author_ip` and `author_id`"
+  task :migrate_notes => :environment do
+    scope = Note.where(:body => nil, :author => nil, :author_ip => nil)
+    total_count = scope.count
+    remaining_count = total_count
+    puts "A total of #{total_count} Note-records have to be migrated."
+
+    # NB: default batch size is 1000
+    scope.find_in_batches do |batch|
+      puts "Processing batch of #{batch.size} records."
+      batch.each do |record|
+        opened_comment = record.comments.unscope(:where => :visible).find_by(:event => "opened")
+        (putc "x" && next) unless opened_comment
+
+        attributes = opened_comment.attributes.slice(*%w[body author_id author_ip]).compact_blank
+        record.update_columns(attributes) # rubocop:disable Rails/SkipsModelValidations

> I think this needs to remove the comment too, right? Otherwise there will be a body, and then the first comment will duplicate the body.

I am hesitant to perform any destructive operations until the very last step, but I open to change my approach here since it would simplify the code.

`Note`-model methods (`#body`, `#author`, `#author_ip`) have been adjusted so that calls to these return the migrated data once its present and otherwise look it up through the `opened`-NoteComment-record.

```rb
class Note < ApplicationRecord
  # FIXME: notes_refactoring remove this once the backfilling is completed
  def body_migrated?
    attributes["body"].present?
  end

  # FIXME: notes_refactoring remove this once the backfilling is completed
  def author
    return super if body_migrated?

    opened_note_comment&.author
  end

  # FIXME: notes_refactoring remove this once the backfilling is completed
  def author_ip
    return super if body_migrated?

    opened_note_comment&.author_ip
  end

  # Return the note body
  def body
    body = body_migrated? ? super : opened_note_comment&.body&.to_s
    RichText.new("text", body)
  end
end
```

> I also think this needs to have tests, and so I would move all the logic into the app (e.g. Note.migrate_bodies, or a standalone class in lib, or somesuch) to make it easier to test.

👍🏽 will add this.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4481#discussion_r1481486368
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/4481/review/1867831350 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240207/4a6c8894/attachment-0001.htm>