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 · 5 comments · May be fixed by #5344
Open

Proposal for adding support for map notes tags #5294

nenad-vujicic opened this issue Oct 29, 2024 · 5 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.

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