openstreetmap / openstreetmap-website

The Rails application that powers OpenStreetMap
https://www.openstreetmap.org/
GNU General Public License v2.0
2.2k stars 913 forks source link

Error 500 on showing note with no visible comments #2146

Open westnordost opened 5 years ago

westnordost commented 5 years ago

Observed behavior

Displaying a note with no comments yields an error 500.

How to reproduce

Display this note: https://www.openstreetmap.org/note/940980 the note via the API: https://api.openstreetmap.org/api/0.6/notes/940980

Expected behavior

  1. The OSM Api should reject creating a note without an initial comment
  2. Either all current notes with no comments are deleted from DB or displaying a note with no comment should not yield an error 500
tomhughes commented 5 years ago

I don't think that's true is it. I thought @Firefishy established that the cause was that the creator of the note had been deleted because they were a spammer?

tomhughes commented 5 years ago

So the immediate cause might be that there are no (visible) comments, but the conclusion that the API allows creation of a note without a comment is not - the only bug here is the exception when a note has no visible comments.

westnordost commented 5 years ago

That might as well be, I can't see whether it is invisible or never existed, I only reported what I saw. Anyway, if the (only) comment was deleted, then error 500 would still not be appropriate.

tomhughes commented 5 years ago

Sorry I assumed it was you that had been discussing it with @Firefishy on IRC last night but I now see that it wasn't and in fact that was a different note.

tomhughes commented 5 years ago

The cause is essentially the same - the user that opened this note deleted their account causing their note comments to be hidden.

This wasn't a spam note though, which raises the question of whether we should in fact hide comments from deleted users or not, or whether we should just treat them as anonymous,

westnordost commented 5 years ago

I guess a good approach would be to treat it the same as map data and GPS traces a deleted user contributed.

Dimitar5555 commented 1 year ago

This issue is still present. https://api.openstreetmap.org/api/0.6/notes/2523764 https://www.openstreetmap.org/note/2523764

tomhughes commented 1 year ago

That would be why this issue is still open yes. Did have something useful to add?

Dimitar5555 commented 1 year ago

The note that westnodorst linked seems to no longer results in a HTTP 500 error and currently it has content (i.e. it's not empty).

Discostu36 commented 1 year ago

Sorry for not finding this issue before opening mine. I think you should hide/delete these notes. There is no use for empty notes leading to error pages, it only makes maintenance harder.

tomhughes commented 1 year ago

We already have a plan (#3831) to fix this which we have started work on.

kmpoppe commented 1 year ago

@tomhughes by skimming the planet-notes.osn I found 1555586 and 1555588 which seem to have no public comments. They're from 2018, are those a symptom of the original user having deleted their account?

pa5cal commented 11 months ago

@tomhughes by skimming the planet-notes.osn I found 1555586 and 1555588 which seem to have no public comments. They're from 2018, are those a symptom of the original user having deleted their account?

Yes, the note was hidden, perhaps because of its text content or the original user was deleted. Then someone else closed it.