Moves the note's description, author ID, and author IP from the first comment to the note itself

[nenad-vujicic]

Description

PR improves using of note's description / author_id / author_ip by:

1. Adding new fields to notes table for storing these 3 values
2. Copying values from first note's comment to notes table
3. Replacing using note's description / author / author_id from first note's comment with from note's table directly

A few comments:

• We left adding description to body when creating new note - it looked dirty to add blank string ("") or similar, also, left comments body intact during data-migration for improving performances,
• We kept backward compatibility of generated XML / JSON / .. by inserting note's description as a first comment's text,
• We added more robust version of data-migration script for retrying migration of failed notes.

How has this been tested?

By running automated tests and by semi-manual testing (semi-manual generating notes, doing migration and applying changes, manually checking rendered notes and new memory variables content).

[tomhughes]

How does this relate to the notes restructuring plan from #3831? Is this a step on the way to that or is it going in a different direction?

[kcne]

This does align with the goals of #3831 in a way, and can be considered alternative to #4481. In fact, the initial motivation for this change came from a comment in #5294, which suggested that note versioning could be a right approach for adding tags. To implement note versioning properly, this PR serves as a first step towards that. It might be worth opening a new issue specifically about note versioning, linking back to that comment for context.

[AntonKhorev]

How does this relate to the notes restructuring plan from #3831?

What I was going to say in comments for #5294 is that the plan from #3831 tries to solve (a) by ignoring (b). You can read the original post of #3831 and see that it doesn't mention note actions/events at all. And those actions, they are stored in the comments table. You'll still have another leaky abstraction as that post says. Note comments won't be really comments. You'll close a note without a comment, that will still generate a "comment" entry in NoteComments.

Is this a step on the way to that or is it going in a different direction?

The direction is hopefully the same for things stated explicitly in #3831. It's different for things omitted.

[nenad-vujicic]

This PR should be first step toward adding editable note-tags and solving #3831. In next PRs we planned to:

1. Add notes versioning (add version column to notes table and add new table old_notes variant),
2. Add note tags (similar to Adds non-mutable note tags support #5344),
3. Add displaying history of notes (API, website, pagination, ... - this will be further decomposed),
4. Add user-interface for creating / updating / deleting note tags (this will be further decomposed),
5. Remove event from NoteComment and special note-comments,
6. ...

We thought to do above as a sequence of as small as possible PRs (like this one), but if you would prefer one big, we can do it? Every comment / suggestion is very welcome :-)

[tomhughes]

I hadn't previously noticed that #5294 had proposed adding versioning - that's a whole other level of additional complication.

I can understand why it might be needed for tags, and I guess it also allows for editing the initial description if we want once that it part of the note itself rather than being a special comment. I do wonder how it interacts with comments though - do we duplicate those for every version like we do with members of other objects or do we always show all comments whatever the version or maybe track add the current version to the comment when it's created and then show comments up to the note's version when showing an old version of the note?

[tomhughes]

This is going to need to be broken down in to multiple PRs as we can't take all three migrations in one go, and we can't take code changes that rely on new fields until after those fields have been created.

The first two PRs should be one to create new methods on Note that correspond to the new fields but just look (for now) at the first comment, and one to add the new fields to Note.

After that a PR to validate the new key and one to adjust the new methods in Note to take the description from the note if the first comment has been migrated.

Only after that can we have a PR to actually do the migration.

[tomhughes]

The plan in #3831 was to add methods like description to the Note model first so that we can hide this kind of logic in one place and avoid duplicating it even temporarily - those methods can then be removed once the migration is complete.

[nenad-vujicic]

... I do wonder how it interacts with comments though - do we duplicate those for every version like we do with members of other objects or do we always show all comments whatever the version or maybe track add the current version to the comment when it's created and then show comments up to the note's version when showing an old version of the note?

Our current plan is to always show all comments whatever the version. However, if others prefer a different approach, we are open to making updates.

[AntonKhorev]

I do wonder how it interacts with comments though - do we duplicate those for every version like we do with members of other objects or do we always show all comments whatever the version or maybe track add the current version to the comment when it's created and then show comments up to the note's version when showing an old version of the note?

I wouldn't say "duplicate (comments) for every version like we do with members of other objects" - do we even have comments for versioned objects elsewhere?

The idea is to take the current comments, let's call them api 0.6 comments, and split them into actual comments and versions. Versions will contain states (open/closed/hidden) that were previously in api 0.6 comments. After that it should be possible to reassemble actual comments and versions into api 0.6 comments for use in api calls. This reassembling wouldn't require associating comments with versions if I'm correct.