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

Propose a second EWG bidding project: Refactoring of OSM website notes #9

Merged
merged 7 commits into from
Nov 9, 2023

Conversation

tordans
Copy link
Contributor

@tordans tordans commented Oct 4, 2023

In this issue openstreetmap/openstreetmap-website#3831 Andy described the need for this refactoring and outlined a solution. This plan was confirmed by Tom.

The current system has bugs and – as Andy outlines – open PRs that try to work around its limitations are not enough to solve the underlying issue.

The goal of this bidding project is to refactor – as in "it works more or less the same but the underlying architecture and data model is better" – the notes system.

This will give the website a better foundation to add new features to, for example a concept of tagging notes, in future contributions or maybe bidding requests.

Please find the propose project outline in the README of this PR at https://github.com/osmfoundation/ewg_bidding/pull/9/files

This refactoring will unblock a lot of ideas; this change make those effects more visible as part of the project. Solving those issues is not part of the project, but its important to know about the positive effects.
@tordans
Copy link
Contributor Author

tordans commented Oct 27, 2023

@tordanik as we discussed during the last EWG meeting, I updated the section of this PR to better highlight the positive side effects of this project.

Please let me know if this is now ready to be voted on during the next meeting.

Co-authored-by: danieldegroot2 <[email protected]>
@tordanik tordanik merged commit 745e440 into osmfoundation:main Nov 9, 2023
@tordanik
Copy link
Collaborator

tordanik commented Nov 9, 2023

At the 2023-11-01 EWG meeting, we decided to merge this project description. We're now accepting proposals for this project. We'll announce this call more widely following our next meeting.

@AntonKhorev
Copy link

This entire thing makes little sense.

The bugs will be gone if you choose to do one of these:

  • entire notes disappearing when a user is deleted
  • comments NOT disappearing (or only partly disappearing, for example only the text is getting blanked but the rest is still visible) when a user is deleted

How are you going to solve it by bidding? This is a decision you'll have to get the osm-website maintainers to agree to.

The underlying data model doesn't matter much. I guess it's a bit easier to make disappearing notes when the note itself has an owner (and you check if the owner is not deleted). But that doesn't require the opening comment to be stored inside the note (which is what Andy wants).

@matkoniecz
Copy link
Contributor

By

Andy described the need for this refactoring and outlined a solution. This plan was confirmed by Tom.

I understood that solution was already accepted by maintainers. Is it inaccurate?

@AntonKhorev
Copy link

@matkoniecz Did you notice that this post starts with a solution, not with a problem? What is the problem here that needs this solution?

@AntonKhorev
Copy link

If this project's goal is to go whatever Andy says in openstreetmap/openstreetmap-website#3831 , consider this for example:

Personally, I think the long term goal should be to remove the concept of this "special first comment" entirely, and end up with a more straightforward model where we have:

  • A Note, which has a body and an author. A note can exist without any comments.
  • NoteComments, none of which are "special" and each can be individually added, removed, hidden etc without affecting the validity of the Note.

Let's suppose you create a note. Then I close it. This adds an NoteComment and changes the note state to "closed". Then my comment gets hidden. Now you have a closed note, but no visible comment that corresponds to its closing.

Now you might think that maybe hiding the comments entirely is not the best idea. Maybe only the comment text should disappear. Then maybe you'll think that the second option from #9 (comment) is a good idea, and it will also solve the actual problems such as error 500. But you don't need to move opening comment from the comments table to the notes table to achieve that. So maybe you shouldn't start with that move (even if it's the solution accepted by maintainers)?

@matkoniecz
Copy link
Contributor

@AntonKhorev I was responding to

How are you going to solve it by bidding? This is a decision you'll have to get the osm-website maintainers to agree to.

part. Then in your letter reply you seem to argue that strategy selected by maintainers is suboptimal, what is entirely different issue.

@tordans tordans deleted the patch-1 branch November 9, 2023 13:35
@tordans
Copy link
Contributor Author

tordans commented Nov 9, 2023

@AntonKhorev it is now possible to bid implementing a solution that follows the outline given by Andy and Tom in openstreetmap/openstreetmap-website#3831. Part of the bid will be to describe the plan on how to solve the outline problem (also following what was described in openstreetmap/openstreetmap-website#3831). Before the bid is accepted, feedback on this will be collected from the maintainers.

If you disagree with the proposed solution, this is not the right place. Please have this discussion upstream and – maybe – this request-for-bids has to be updated afterwards.

@AntonKhorev
Copy link

I attempted fixing the note model, which corresponds to the first step of the outline "Corral all the complexity and special casing into the models" back in July 2022: openstreetmap/openstreetmap-website#3607.

@AntonKhorev
Copy link

@matkoniecz Whatever implementation you decide to do, you still need to make the choice between disappearing notes and disappearing comments (or maybe not disappearing anything). This choice is not mentioned in the outline, therefore it's not clear what anyone agrees on. The only comment mentioning deleted users seems to favor not hiding anything and no objections are raised about it.

If we look elsewhere, it seems that @gravitystorm favors hiding the entire note. Maybe this choice is so obvious to him that it doesn't even worth mentioning in his refactoring plan. He makes a comparison with diary entries that disappear entirely, but you can as well compare notes to changesets that don't disappear, and that's what @woodpeck's comment does.

To make progress we need first to agree what to do with notes opened by deleted users or at least get a clear answer from Andy if he wants notes to disappear.

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 this pull request may close these issues.

6 participants