display warning in new note form after too many anonymous notes #5468
Conversation
create a counter of anonymous notes in localStorage, then, if an anonymous visitor has already created too many notes, display a warning in the new note form to encourage him to contribute by himself
|
If they're valid notes I don't really see the problem? Are these genuine notes or is this some sort of spam attack? |
| if (typeof OSM.user === "undefined" && anonymousNotesCount >= 20) { | ||
| var counterWarning = content.find("#new-note-counter-warning"); | ||
| if (typeof counterWarning.html() !== "undefined") { | ||
| counterWarning.html(counterWarning.html().replace("(N)", anonymousNotesCount)); |
There was a problem hiding this comment.
That's not how translatable numerals work. The number could also affect other parts of the sentence in various languages ("24 заметки", "25 заметок").
There was a problem hiding this comment.
OK I understood how pluralization works with i18n, but it is server-side ... If the value of the counter is in the browser (whether in a cookie or in localStorage), it has to be inserted by jquery, so after the translation, and it's too late to manage the different plurals, right ? Is there another option to manage plurals with jquery ? Or is it why you propose the counter to be server-side ?
There was a problem hiding this comment.
There is i18n library in javascript too but it's better to avoid it if possible. You'd have to declare strings in a different section of en.yml, then all of those strings are sent over to the client.
Example of javascript i18n with count:
|
I'd store the counter in a cookie, set an expiration date for that cookie to something like a week and replace the existing "You are not logged in" alert with this more motivating one with a note counter server-side. |
| @@ -162,6 +172,9 @@ OSM.NewNote = function (map) { | |||
| newNoteMarker.dragging.disable(); | |||
|
|
|||
| createNote(location, text, (feature) => { | |||
| if (typeof OSM.user === "undefined") { | |||
| localStorage.setItem("anonymousNotesCount", anonymousNotesCount + 1); | |||
There was a problem hiding this comment.
If we keep localstorage (see @AntonKhorev comment):
localStorage.setItem can throw an exception.
There was a problem hiding this comment.
Is the nullish coalescence operator ?? you propose below sufficient to handle the case of an exception? Or does it only work for null and undefined?
There was a problem hiding this comment.
?? replaces null and undefined values only.
There was a problem hiding this comment.
OK I will add a try ... catch
| @@ -152,6 +152,16 @@ OSM.NewNote = function (map) { | |||
| .on("input", updateControls) | |||
| .focus(); | |||
|
|
|||
| var anonymousNotesCount = Number(localStorage.getItem("anonymousNotesCount")); | |||
There was a problem hiding this comment.
| var anonymousNotesCount = Number(localStorage.getItem("anonymousNotesCount")); | |
| var anonymousNotesCount = Number(localStorage.getItem("anonymousNotesCount") ?? 0); |
after #5421 is merged
This contributor's notes are potentially problematic and pollute the other notes. |
A large part of them are not valid or useless. This PR should not be seen as a way to solve a one guy problem, but a global issue with anonymous notes. The lack of communication channel makes a lot of them difficult to deal with, so at least when someone is creating a lot of notes, we should push to create that communication channel, and it also a way to avoid spam. |
I am not a specialist of cookies VS localStorage, the only advantage of cookies is the possibility to set an expiration date ?
good idea!
I'm not sure I understood this part. I imagined this would require storing an ‘anonymous visitor ID’ in the cookie and creating a table on the server to associate this ID with the notes created by the visitor? but maybe I've misunderstood? |
The browser sends cookies to the server. It doesn't send localStorage. If the server knows the counter value, it can display another message based on that value, that's what I mean by:
|
This thing already exists as session id in That request will increase the counter stored in session if necessary. After that app/views/notes/new.html.erb or its controller can check the counter value and display the message based on that.
All of this depends on how soon our sessions expire. The example in #5468 (comment) shows a 10-minute burst of activity, so the session doesn't have to live for very long. |
Motivation
In the French community, we've had a problem for over a year with someone creating a huge number of anonymous notes, which hampers the processing of other relevant notes in the same area. See this discussion on the French community forum.
I thought it would be a good idea to count the number of anonymous notes created in the same browser and to display a warning when this number becomes too high, to encourage visitors to become users and contributors.
Description
This PR creates a new warning in the UI of the new note form if an anonymous visitor has already created too many anonymous notes (threshold set at 20 for now)
Changes:
Possible additional change (not included yet)
After an even larger number of anonymous notes (50?), I thought we could hide the textarea, and trigger its display by clicking on a button sure you want to post an anonymous note again?, which would force the visitor to perform an additional action. This is not included in this PR yet, but tell me if you want me to add it.
How has this been tested?
The code has been tested on my personal computer using rails built-in webserver. I verified that the new note form is still working fine both when logged in and in anonymous mode.
However, as described in issue #5467 I was not able to make all the tests requested in CONTRIBUTING.md.
As this is my very first PR, I'm not sure I've done everything right, so I welcome any relevant comments!