Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal for adding support for map notes tags #5294

Open
nenad-vujicic opened this issue Oct 29, 2024 · 10 comments · May be fixed by #5344
Open

Proposal for adding support for map notes tags #5294

nenad-vujicic opened this issue Oct 29, 2024 · 10 comments · May be fixed by #5344

Comments

@nenad-vujicic
Copy link
Contributor

nenad-vujicic commented Oct 29, 2024

Problem

Do you have some key-value meta-data you would like to store with map notes? Would you like to do it like with tags in nodes / ways / relations / changesets / .. instead of writing in description / comments? Note tags have already been mentioned / requested several times in #385 , #801 , #3932 and similar. Here we expose our plan about how we could add them.

Description

Here is a list of PRs (steps) we would like to work on with rough description of what they will do. We would highly appreciate your feedback and suggestions on how we could improve the plan:

  1. Create migration script for creating note-tags table, appropriate model file and setting up associations:
    • Create new table note_tags with note_id (bigint(8)), k (string, default “”, not-null) and v (string, default(“”), not-null) attributes
    • (note_id, k) will be set as private key and note_id will be foreign key referring id attribute from note table
    • Create new model NoteTag which will model row of note_tags table and add associations between NoteTag and Note
    • Add / improve new / existing unit tests for testing new functionalities
  2. Add loading and displaying note tags using browse/tag_details partial
    • Add displaying of note tags using browse/tag_details partial between Description (currently "special first comment") and Comments (remaining comments / status messages / ..)
    • Add / improve new / existing unit tests for testing new functionalities
  3. Update Notes API to include retrieving note tags
    • Update (j)builder files to include note tags in generated files, by inserting <tag k="" v=""/> pairs between status and comments like in this example
    • Add / improve new / existing unit tests for testing new functionalities
  4. Update Notes API to include creating Note instances with tags
    • Update API::NoteController to create Note instances with tags
    • Update new_note.js to pass created_by:OpenStreetMaps-Website tag to newly created note when created from OSM website
    • Add / improve new / existing unit tests for testing new functionalities
  5. Update Wiki documentation about OSM API v0.6 section Map Notes (this will be more of a side task than a PR)

Screenshots

image

@simonpoole
Copy link
Contributor

simonpoole commented Nov 15, 2024

It isn't clear from the description if this is intended or not, but to be really useful (for example to document with which client an operation is being performed) they need to be "per interaction" so not just for the original note creation but at least for every comment, and maybe for opening and closing too.

@nenad-vujicic
Copy link
Contributor Author

It isn't clear from the description if this is intended or not, but to be really useful (for example to document with which client an operation is being performed) they need to be "per interaction" so not just for the original note creation but at least for every comment, and maybe for opening and closing too.

At the moment, tags are added only at creation time and are per-note. For example, if created from OpenStreetMap, only pair (created_by, OpenStreetMaps-Website) will be created. If note is created from editor, editor defines number and tags which will be created.

But, it will not be hard to add tags for comments / actions (open, close, ..) .. We can add it without disrupting current implementation as next steps, if others agree.

Thanks for great idea!

@AntonKhorev
Copy link
Collaborator

but to be really useful they need to be "per interaction"

Do you want versioned notes with sets of tags for every version, like elements?

@simonpoole
Copy link
Contributor

No, I don't think that would be an appropriate model.

Just as comments extend the conversation around the original note and do not replace it, tags associated with comments should not replace the original ones.

@nenad-vujicic nenad-vujicic linked a pull request Nov 19, 2024 that will close this issue
@AntonKhorev
Copy link
Collaborator

I thought about this again and versioned notes may be what we actually want. We already kind of have note versions, they only contain state changes (open -> closed -> open again). But with tags we might want to record tag changes too. Being able to change tags allows to use them for more than just recording what tool the note was created with. We may want to use them for some kind of categorization (like "needs survey"). We might want to add some categories to already existing notes. Also we might want to add them to our own newly created notes because the tools we were using to create these notes don't allow adding tags or it was inconvenient to add the tags at the moment.

tags associated with comments should not replace the original ones

We don't have tags associated with comments anywhere else on the site.

they need to be "per interaction" so not just for the original note creation but at least for every comment

And what's the goal of that? Is it that important to record a client if it was just a comment? We're not adding tags to changeset comments and not trying to record clients in some other way, should we start doing that?

and maybe for opening and closing too

This is different to commenting. If anyone actually modifies the note, we might want to record that by adding a tag such as edited_by or closed_by or maybe even by overwriting created_by. Although in the latter case maybe we shouldn't be using created_by for notes at all (#5344 currently does), and just use edited_by.

@AntonKhorev
Copy link
Collaborator

Comment asking for editable tags: #3932 (comment)

@nenad-vujicic
Copy link
Contributor Author

If we want editable tags, then we definitely need (if we want to follow remaining of website) versioning for notes.

But what do you mean by "modifying note"? Only adding / removing / changing tags (the easiest) or also changing note's status or perhaps also updating note's description or also some other "interaction" (hardness rises when going from left to right :-))?

@AntonKhorev
Copy link
Collaborator

At first editable tags and status (which is already editable). The versions table will also contain the description and coordinates, but we won't make them editable right away. Currently there's no API for that, maybe when API 0.7 happens we can have create/update endpoints receiving xml/json with the entire note data.

@AntonKhorev
Copy link
Collaborator

We have a so called note comments table:

CREATE TABLE public.note_comments (
    id bigint NOT NULL,
    note_id bigint NOT NULL,
    visible boolean NOT NULL,
    created_at timestamp without time zone NOT NULL,
    author_ip inet,
    author_id bigint,
    body text, -- (a) not always a comment
    event public.note_event_enum -- (b) not a comment
);

As you can see, it contains things that are not comments:

  • (a): text is a description in case of opening events
  • (b): event is not a comment of course, other comment tables don't have events

Here's the changeset comments table for comparison:

CREATE TABLE public.changeset_comments (
    id integer NOT NULL,
    changeset_id bigint NOT NULL,
    author_id bigint NOT NULL,
    body text NOT NULL,
    created_at timestamp without time zone NOT NULL,
    visible boolean NOT NULL
);

You can turn note_comments table into an actual comments table if you remove things that are not comments (opening event rows, rows without comment text, event column). But you can also turn it into a note version table if you remove other things (comment event rows), modify some (text to always contain the description) and add what's missing (version numbers).

If we replace note_comments with two tables as described above, we'll have a pure comments table and a versions table. Then we can add tags referencing the versions table.

@AntonKhorev
Copy link
Collaborator

The note versions table could look like this:

... (
    note_id bigint NOT NULL,
    version bigint NOT NULL,
    status public.note_status_enum NOT NULL,
    "timestamp" timestamp without time zone NOT NULL, -- more standard name would be created_at but that could be mixed up with the note creation timestamp
    latitude integer NOT NULL,
    longitude integer NOT NULL,
    tile bigint NOT NULL,
    user_id bigint NOT NULL, -- I guess we're not going to call this "author", that won't sound right for non-initial versions
    user_ip inet, -- if we keep anonymous notes
    ...
)

We also need to choose between

    visible boolean NOT NULL,

and

    redaction_id integer

depending on whether we want to use redactions for hiding note version.

If we want the versions table to be similar to element versions, its primary key should be (note_id, version). But that surrogate id from note_comments is useful for sorting by last update, see #4532 (comment). I'd like to keep it on the versions table in addition to the comments table. Element version tables don't have it, but I'd like them have it too, see #4660, but I'm afraid of that being a migration that is too large.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants