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

display an encouragement to contribute after many anonymous notes #5468

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

etienneJr
Copy link
Contributor

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)
image

Changes:

  • in template views/note/new.html.erb
    • add a new div element displaying the warning (hidden at first)
  • in locales/en.yml
    • add text and links for the warning, with ‘(N)’ being replaced by the number of anonymous notes
  • in new_note.js
    • when an anonymous note is created: add +1 to the counter in localStorage
    • when the form is displayed: check the counter in localStorage. If more that 20 notes have already been created: display the warning, and replace (N) in the text warning with the number of anonymous notes already created.

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!

@tomhughes
Copy link
Member

If they're valid notes I don't really see the problem? Are these genuine notes or is this some sort of spam attack?

@AntonKhorev
Copy link
Collaborator

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.

@gendy54
Copy link

gendy54 commented Jan 5, 2025

If they're valid notes I don't really see the problem? Are these genuine notes or is this some sort of spam attack?

This contributor's notes are potentially problematic and pollute the other notes.
image

@cquest
Copy link

cquest commented Jan 5, 2025

If they're valid notes I don't really see the problem? Are these genuine notes or is this some sort of spam attack?

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.

@etienneJr
Copy link
Contributor Author

etienneJr commented Jan 5, 2025

I'd store the counter in a cookie, set an expiration date

I am not a specialist of cookies VS localStorage, the only advantage of cookies is the possibility to set an expiration date ?

replace the existing "You are not logged in" alert with this more motivating one

good idea!

with a note counter server-side.

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?

@AntonKhorev
Copy link
Collaborator

I am not a specialist of cookies VS localStorage, the only advantage of cookies is the possibility to set an expiration date ?

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:

replace the existing "You are not logged in" alert with this more motivating one with a note counter server-side.

@AntonKhorev
Copy link
Collaborator

AntonKhorev commented Jan 6, 2025

I imagined this would require storing an ‘anonymous visitor ID’ in the cookie

This thing already exists as session id in _osm_session cookie. And that's another option. Instead of managing the counter and its expiration date in javascript, you can make a request at some special endpoint after a note is successfully created here


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.

@etienneJr etienneJr force-pushed the anonymous-notes-counter branch from 5250276 to 6e6000c Compare January 7, 2025 22:07
@etienneJr
Copy link
Contributor Author

etienneJr commented Jan 7, 2025

Thanks for your time, comments, and advice. I just pushed a new version with :

  • the counter stored in a cookie, with a duration of 30 days, as anonymous visitors don't necessarily post notes every day (I'm not sure about other cookie options, please review)
  • the cookie read in the controller
  • a message in the view template, included in the already existing warning
  • with pluralization managment in en.yml

I have slightly modified the standard warning in this way, to insist that discussion will help to resolve the note:
image

And I added the encouragement to contribute yourself in the same box, after at least 10 notes, with a link to the beginners' guide, and to the forum:
image

I know that finding the perfect sentence is always complicated, and English is not my mother tongue, so tell me if you prefer another sentence.

@etienneJr etienneJr changed the title display warning in new note form after too many anonymous notes display an encouragement to contribute after many anonymous notes Jan 7, 2025
Copy link
Collaborator

@AntonKhorev AntonKhorev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're adding things like var anonymousNotesCount = Number(localStorage.getItem("anonymousNotesCount"));
in your first commit, then you completely remove them in the second. In this case you can just squash these commits together.

@etienneJr
Copy link
Contributor Author

you can just squash these commits together.

Good idea, I'll do that, it'll be cleaner. Sorry, I'm not really used to PR so I hadn't thought of it myself.

@AntonKhorev
Copy link
Collaborator

What shall we do with the counter

  • if the user logs in?
  • if the user creates an account?

@etienneJr
Copy link
Contributor Author

What shall we do with the counter

  • if the user logs in?
  • if the user creates an account?

I thought about it, we could delete the cookie after logging in, but it seemed pointless as the cookie will expire 30 days after the last anonymous note anyway. Do you agree?

@AntonKhorev
Copy link
Collaborator

but it seemed pointless as the cookie will expire 30 days after the last anonymous note anyway

The point could be that if I log in my session may expire faster than that and I'll se the message again. But maybe it's not worth doing anything about it.

@etienneJr etienneJr force-pushed the anonymous-notes-counter branch from 6e6000c to 13d1113 Compare January 10, 2025 22:20
@etienneJr
Copy link
Contributor Author

Hi,
Thanks again for the positive and pertinent comments, I pushed a new version with:

  • only 1 squashed commit
  • updated threshold
  • modified warning with a gap
  • deletion of the cookie after successful login

@etienneJr etienneJr requested a review from AntonKhorev January 10, 2025 22:29
@AntonKhorev
Copy link
Collaborator

deletion of the cookie after successful login

If you do this, you probably also want to delete the cookie on signup. I'm not sure if it's better to do before or after confirming the email. If before, you'd delete the cookie somewhere here:

SIGNUP_IP_LIMITER&.update(request.remote_ip)

@etienneJr etienneJr force-pushed the anonymous-notes-counter branch from 13d1113 to b49cdc1 Compare January 12, 2025 21:45
@etienneJr etienneJr requested a review from AntonKhorev January 12, 2025 21:58
create a counter of anonymous notes in a cookie, read by new note controller to display an encouragement to contribute in the already existing anonymous warning if the anonymous visitor has already created at least 10 anonymous notes. Cookie deleted on log-in and sign-up after email validation
@AntonKhorev AntonKhorev force-pushed the anonymous-notes-counter branch from b49cdc1 to 0f2df0b Compare January 13, 2025 11:46
@AntonKhorev AntonKhorev merged commit 1732325 into openstreetmap:master Jan 13, 2025
15 checks passed
@AntonKhorev
Copy link
Collaborator

Merged, thanks! I pushed some whitespace changes to satisfy erblint test. That test didn't run because it needed approval, so you couldn't see it here.

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