Map moderation system

[grvsachdeva]

Fixes #1029

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

• [x] PR is descriptively titled 📑 and links the original issue above 🔗
• [ ] tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
• [x] code is in uniquely-named feature branch and has no merge conflicts 📁
• [x] screenshots/GIFs are attached 📎 in case of UI updation
• [x] ask @publiclab/mapknitter-reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

[qlty-cloud-legacy[bot]]

Code Climate has analyzed commit fe0c7053 and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Style	3

View more on Code Climate.

[codecov[bot]]

Codecov Report

Merging #1030 into main will increase coverage by 0.32%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1030      +/-   ##
==========================================
+ Coverage   72.87%   73.20%   +0.32%     
==========================================
  Files          40       41       +1     
  Lines        1401     1418      +17     
==========================================
+ Hits         1021     1038      +17     
  Misses        380      380              

Impacted Files	Coverage Δ
app/controllers/maps_controller.rb	85.59% <100.00%> (+0.12%)	:arrow_up:
app/models/map.rb	93.63% <100.00%> (+0.08%)	:arrow_up:
app/models/user.rb	100.00% <100.00%> (ø)
app/services/moderation_service/moderate_map.rb	100.00% <100.00%> (ø)

[jywarren]

Hi, @gauravano !!! 🙌

Just noting that we've had a number of small changes to how map moderation UI works, and focused on map deletion in #238 #1251 #1265 (i believe) while putting aside the spam functionality (which was a really primitive one that couldn't be undone) in https://github.com/publiclab/mapknitter/issues/1251.

Second, one big gap was that i think we don't prevent users banned on PublicLab.org from editing or posting new maps. We had wanted to try accessing user status (as well as admin/moderator role) from the plots2 model using... some kind of wrapper around a remote call, i think? I'm trying to find this again.

[jywarren]

Yes, this idea of calling a remote user model from plots2 is here: https://github.com/publiclab/mapknitter/issues/626 and implemented for one case here:

https://github.com/publiclab/mapknitter/blob/eb120589a5e3519bb3757630d47d9cea23d61949/app/services/exporter_client.rb#L1-L15

Maybe we can expand that a bit to cover user location, user status, and user role, and bake those into the user model? There may need to be some careful error handling if PL.org goes down or is slow, though!

[grvsachdeva]

@jywarren hey 👋

so in current flow, I authorized my local website to login using Public lab and it did it using Production PublicLab website. My role in publiclab.org prod is admin so after authentication user is created in mapknitter DB with correct role as admin.

[11] pry(main)> User.last 
  User Load (0.5ms)  SELECT  `users`.* FROM `users` ORDER BY `users`.`id` DESC LIMIT 1
=> #<User:0x00007fbae6d42cb0
 id: 7,
 login: "gauravano",
 name: "",
 email: "################<email present in public lab account>",
 crypted_password: nil,
 salt: nil,
 identity_url: "https://publiclab.org/openid/gauravano",
 role: "admin",
 created_at: Sat, 08 Aug 2020 15:52:55 UTC +00:00,
 updated_at: Sat, 08 Aug 2020 15:52:55 UTC +00:00,
 remember_token: nil,
 remember_token_expires_at: nil>

So, looks like role is getting saved at time of user creation correctly so that problem is solved.

[grvsachdeva]

Maybe we can expand that a bit to cover user location, user status, and user role, and bake those into the user model? There may need to be some careful error handling if PL.org goes down or is slow, though!

Agree with your point @jywarren.

1. User role issue is already solved.
2. User status issue

We have 3 user status in plots2 0: banned 1: normal 5: moderated

How about we stop banned users from logging into mapknitter at the time of auth using Public lab step as if we have identified a user as banned at publiclab.org then we don't need to keep them in mapknitter DB too?

can you explain what "moderated" user signify for clarity? I referred it last time from wiki I think but not able to find it right now.

Thanks!

[grvsachdeva]

Just saw inbox and received a sentry for my action (logging in mapknitter local using production website) - https://sentry.io/organizations/publiclab/issues/1830824416/?environment=production&project=1410626 :P Looks like we need to handle such cases in plots2 production.

[grvsachdeva]

Have raised the fix for #1251 in https://github.com/publiclab/mapknitter/pull/1387 for now. But, once we merge this functionality we can let go of deleting the map and can mark it as spam.

[jywarren]

Hi @gauravano - sorry it took me so long. Here are some thoughts:

So, looks like role is getting saved at time of user creation correctly so that problem is solved.

I wonder about updating though. Can we, each time we log in, refetch the user role? In case it changes.

moderated

This is like banned, but has slightly different policies. Mainly it stops people from logging in, rather than stops them from posting.

Thank you, @gauravano !!!! Much appreciated!!!

[grvsachdeva]

Hi @jywarren,

There are 2 design choices here:

1. Polling publiclab.org for user status for each login (Suggested by you)

wonder about updating though. Can we, each time we log in, refetch the user role? In case it changes.

This one sounds fine but it has some disadvantages:

• If a user is logged in at mapknitter.org and banned in publiclab.org, they will be able to enjoy the normal privilege till their session expire (whose duration is quite high I believe).
• We would be making un-necessary network calls putting pressure on publiclab.org system, just for sync.

2. Pushing updates to Mapknitter.org Here's what I think would be better if we want to make network calls. Every time a user status gets changed at publiclab.org, make a call to mapknitter telling the change. If the user exists in Mapknitter, then update its status otherwise respond with success to API call acknowledging receiving the update.

This approach assume that we don't change user status very often.

What do you think about these 2 approaches @jywarren?

I was also pondering about these points:

1. If we are updating user status at mapknitter from plots2, then should public lab also get updated when user status update takes place at mapknitter? Are we making it complex 😅 ?

2. Mapknitter has 5 modes of entry

Only Public Lab and Google Oauth seem to be working. If we're planning to poll updates or push updates from publiclab.org, what would be the identifier for account identification - username or email or ...?

3. Should we really couple User information flow between Mapknitter and Plots2, as their DB is different so are we just making it complex by doing so? Or, do we have numbers that how many Public Lab users are also mapknitter users?

Please let me know what you think about the above point @jywarren. I am proceeding with finalizing marking spam maps as spam (and users as after effect) functionality for now.

Thanks!!

Thanks!!

[jywarren]

Hi @gauravano -- sorry for slow reply again! i noted some of my notification settings via gitter for future ref.

I wonder if we could use option 1 above, but allow dumping the session with a signal from plots2? Like, worst case, you can still keep going until your next login. But, we try to get plots2 to force a login if any status changes, using a special URL and token? And maybe checking that the flush request comes from publiclab.org?

The load on plots2 should be minimal if it's just on login. But alternatively, could we check every X days, or something, from the MK end? Or, like, is there a timestamp we can check on last session use, and recheck the status if it's been 24 hours or something?