Notes throw errors if there are no visible comments

[gravitystorm]

It's not possible to view a note if there are no visible comments. This can come about either:

• There's only one comment (the original) and the user is deleted (see Allow users to delete their own accounts #3398)
• There's only one comment (the original) and that comment is hidden (see Added (un)hide note comments API endpoint #3438)

In both cases, you get errors, albeit different ones in each case. But the point remains that there's a lot of (untested) logic that assumes there's at least one note comment visible.

Related:

• When creator of a note is deleted, the first commentator looks like the creator #3396
• rework notes to have a body and author gravitystorm/openstreetmap-website#6

[mmd-osm]

I thought I tested a scenario where all notes were hidden, and I don't remember seeing an error message for non-moderators. Hmm.

[gravitystorm]

I think with the UI that you made in #3441 you don't surface the option to hide the first comment, but there's no similar restriction in the API implementation in #3438. I haven't checked that conclusively though. So if you use the UI that you wrote then you won't hit the problem.

[mmd-osm]

Good point. What would be your suggestion here? Should the new API endpoint block any attempts to delete the first comment?
Besides, "Resolved by..." and "Reactivated by..." can be hidden / un-hidden in the new UI. I don't see this as a bug, as a moderator might want to hide excessive closing/reactivating of notes to get rid of some clutter.

      # Find the note comment
      comment = NoteComment.find(id)

      # Opening comment cannot be hidden
      raise OSM::APIBadUserInput, "Cannot hide id" if comment.event == "opened"

      # Hide the comment
      comment.update(:visible => false)

[gravitystorm]

Good point. What would be your suggestion here?

I don't know. I think relying on the fact that the first comment has the "opened" event, and that there's no other "opened" events, seems fragile - but I'm not entirely sure why I feel that. But maybe that's a workaround until we sort things out properly?

[mmd-osm]

Hmm. An "opened" event is used exclusively when creating a new note. https://github.com/openstreetmap/openstreetmap-website/blob/master/app/controllers/api/notes_controller.rb#L76

I think what needs to be clarified first is whether we want the opening event to be hidden at all. Depending on this decision other parts of the code need to be adjusted to handle this case properly.

[gravitystorm]

I've moved the discussion of the "opened" comment to the PR in #3438 , since this issue is to deal with the wider problems of the current implementation.