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

[Gregory Igelmund]

## Why are the changes necessary?

Addresses #3831 which describes how a set of bugs related to the current `Note`-model can be solved by merging the "special first comment" into the `Note`-record itself.

_This PR is meant to serve as a discussion starter. I gave my best to make sure the code changes are not introducing any undesired behavioural changes, but I am sure there are things I've missed, so please regard this PR a work in progress._

## What has changed?

- Adjusted the `Note`-Model to hold the new information about `body` and the `author`
  - Added db fields `notes.{body,author_id,author_ip}` for that
  - Added `Note#comments_with_extra_open_comment` for the API to keep existing behaviour as it is
- Adjusts Web Controller and View code to work with the new `Note`-Model


## Open Questions

a) Should the data migration (to backfill the `notes.{body,author_id,author_ip}` run in the scope of the migration or can this be a rake task which is running in the background and allows to perform some sanity checks on the data?

b) I am making the assumption that the backfilling process can be performed for all `Note`-records. Does that hold true, so that eventually all `Note`-records will have a non-empty `body`-field or do we need to account for `Note`-records without a `body`?

## Open TODOs

I am not sure if I successfully migrated the logic of `Api::NotesController#feed`. I understand that at least the behaviour of the `.limit(result_limit)`-call will involuntarily return more results than it currently does. This part of the PR needs more focus and probably I'll have to add some tests first.
You can view, comment on, or merge this pull request online at:

  https://github.com/openstreetmap/openstreetmap-website/pull/4481

-- Commit Summary --

  * Extend Note with body & author, update controllers & views
  * Update Tests to reflect Note refactoring
  * Add migration task for Notes

-- File Changes --

    M app/controllers/api/notes_controller.rb (54)
    M app/controllers/notes_controller.rb (3)
    M app/models/note.rb (54)
    M app/models/note_comment.rb (1)
    M app/views/api/notes/_description.html.erb (2)
    M app/views/api/notes/_note.json.jbuilder (2)
    M app/views/api/notes/_note.rss.builder (2)
    M app/views/api/notes/_note.xml.builder (2)
    M app/views/notes/index.html.erb (2)
    M app/views/notes/show.html.erb (6)
    A db/migrate/20240107144209_add_author_and_body_to_notes.rb (33)
    M db/structure.sql (14)
    A lib/tasks/migrate_notes.rake (24)
    M test/controllers/api/notes_controller_test.rb (258)
    M test/controllers/notes_controller_test.rb (28)
    M test/factories/note_comments.rb (2)
    M test/factories/notes.rb (11)
    M test/helpers/issues_helper_test.rb (4)
    M test/models/issue_test.rb (4)
    M test/models/note_test.rb (48)
    M test/system/index_test.rb (8)
    M test/system/issues_test.rb (2)
    M test/system/note_comments_test.rb (6)
    M test/system/report_note_test.rb (8)
    M test/system/report_user_test.rb (4)

-- Patch Links --

https://github.com/openstreetmap/openstreetmap-website/pull/4481.patch
https://github.com/openstreetmap/openstreetmap-website/pull/4481.diff

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

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