Outreachy: "MapKnitter.org Spam Management System" Project Q&A

[PeculiarE]

This is the discussion thread for questions on my Outreachy project (#1744).

[PeculiarE]

Hi @jywarren! I plan on starting work on the first couple of tasks on the list at #1744 but to do that, I need clarifications on the following, please.

(1) So @grvsachdeva has already done some work on these tasks in the PR #1030. And the project goals did mention building on existing work. So I'd like to clarify if this existing PR will be merged or am I supposed to just simply incorporate some elements from it into my own?

(2) When adding the status column to the maps (as seen in #1030), all maps will get a default status of 1 (i.e. published) including maps which have been archived using the old spam/archive method. This means we would then have two conditions for fetching map(s): their status(published/spammed) and whether or not they've been archived.

Should we then:

a. Make every archived map automatically a spammed map when doing migrations, i.e. for rows whose archived value is 1, assign the status of 0 instead of the default 1 to the status column. This way, our fetching endpoints will only fetch based on a singular criteria: the status column

b. Leave as is. Give all maps a default of 1 and fetch based on status as well as check if the map is archived.

I believe these are the 2 questions I need answered first before I get started. Other qs will definitely crop up as I move from one task to the next on the list.

Lastly, even though the design implementation won't be happening now, I would like to clarify if the proposed design is okay or we need to make a few tweaks to it before implementation later down the line.

Thank you!

[jywarren]

(1) So @grvsachdeva has already done some work on these tasks in the PR #1030. And the project goals did mention building on existing work. So I'd like to clarify if this existing PR will be merged or am I supposed to just simply incorporate some elements from it into my own?

I would be OK with either route. Do you have a feeling for which you think makes more sense, based on the work Gaurav did, or would you like me or another mentor to look through the PR and see how far along it is, and whether we think there's a best way forward?

(2) When adding the status column to the maps (as seen in #1030), all maps will get a default status of 1 (i.e. published) including maps which have been archived using the old spam/archive method. This means we would then have two conditions for fetching map(s): their status(published/spammed) and whether or not they've been archived.

Should we then:

a. Make every archived map automatically a spammed map when doing migrations, i.e. for rows whose archived value is 1, assign the status of 0 instead of the default 1 to the status column. This way, our fetching endpoints will only fetch based on a singular criteria: the status column

b. Leave as is. Give all maps a default of 1 and fetch based on status as well as check if the map is archived.

I believe these are the 2 questions I need answered first before I get started. Other qs will definitely crop up as I move from one task to the next on the list.

I think we should make spammed/archived maps status 0, as essentially all of them are spam. And in any case, we aren't deleting them -- since it's non-destructive, it's OK to just reclassify them under the new system.

Lastly, even though the design implementation won't be happening now, I would like to clarify if the proposed design is okay or we need to make a few tweaks to it before implementation later down the line.

I think the design is good as it is! Since spam moderators will be using both PublicLab.org and MapKnitter's spam dashboards, we want both the style/layout and functionality to be very similar, or they could get confused and make mistakes. So unless you see an opportunity for a major improvement, or elimination of a serious ambiguity or bug, we should stick as close as we can to the PublicLab.org design. That said, if you have extra time towards the end of your project, we could think about making improvements across both systems in synchrony!

Thank you for the excellent questions!!

[PeculiarE]

I would be OK with either route. Do you have a feeling for which you think makes more sense, based on the work Gaurav did, or would you like me or another mentor to look through the PR and see how far along it is, and whether we think there's a best way forward?

I believe I'd prefer incorporate stuffs from the PR as I work down the planning list. While there are some functions in there that are pretty straightforward and I can understand what they do and where they are to be used, I'm not so sure of others. But they might begin to make sense as I progress through the project. And I believe in that case, it's better to come to those conclusions myself rather than merging them all at once without being sure what each function does and how it ties to the project goals and tasks.

I think we should make spammed/archived maps status 0, as essentially all of them are spam. And in any case, we aren't deleting them -- since it's non-destructive, it's OK to just reclassify them under the new system.

Alrighty! Thank you. This further buttresses my point about not merging the PR straight up as the migrations there do not take this new addition into consideration.

However, this leads to another very important question, I think. My thought process is still garbled on this one, been turning my head on it for a couple of hours now. So, I'm just gonna lay it all out and hopefully, we can decide on a clear-cut process:

The policy on PublicLab (which we fully intend to incorporate in this project) is that spamming content automatically bans the owner of the content. If we spam maps with the archived status, we should also ban the author of those maps as well during the migrations.

But does this make sense from a business/community perspective? Banning authors now whose maps were archived maybe 2 years ago.... I'm not sure if this is okay. What if the author is no longer a spammer and now uses MapKnitter as it should be used?

And in a sorta roundabout way, we have to consider that one of the project goals is to spam all maps of a banned user. So do we also spam all maps of that banned user?

In summary, if we give an archived map a status of 0 (i.e. spammed), should we ban the user, and then spam all maps belonging to that user? Is this overkill, or is there a better way of handling this?

I think the design is good as it is! Since spam moderators will be using both PublicLab.org and MapKnitter's spam dashboards, we want both the style/layout and functionality to be very similar, or they could get confused and make mistakes. So unless you see an opportunity for a major improvement, or elimination of a serious ambiguity or bug, we should stick as close as we can to the PublicLab.org design. That said, if you have extra time towards the end of your project, we could think about making improvements across both systems in synchrony!

Cool, thank you @jywarren. This makes a lot of sense.

[jywarren]

I think you're right that although, as a matter of policy, we usually ban authors, we don't have to in this retrospective way. Although I want to note that banning is reversible and we usually tell people how to appeal in the message they see upon logging in, so it's not a permanent state. But I still agree with you, let's just spam the maps, not the users for now.

One additional advantage of taking pieces from the PR instead of merging the whole thing is that we could look for more opportunities to break it into pieces. Some parts could probably be done before, even if those parts don't actually get activated in the live site without their companion pieces. But if we write tests to protect them, they are still ok to merge in their own earlier PR, simplifying the review process and lowering the stakes of publishing to the live site. For example, we could decide to deal with old maps (and the migration) in it's own PR, without yet worrying about code that actually interacts with the migration. That's just one example.

It might be good to start thinking about how your tasks break into PRs and which if any might be dependent on others. But don't worry too much about knowing every step ahead of time. You can also just keep it in mind, while picking out smaller tasks that can be done in a PR, and it'll naturally filter out the big ones that'll take more coordination in a big PR later.

Great questions!

[PeculiarE]

Thank you @jywarren! I do agree with you on the importance of writing tests to help protect against any breaking changes.

I'm used to solving a task in one huge PR, and breaking one task into several related PRs is going to be a major (but very doable) shift in my coding style. I'm excited (and a wee bit anxious) to delve into this and learn a new way of doing things 😅

[PeculiarE]

Hi @jywarren, @cesswairimu, @TildaDares....I'm not so conversant with tests in the Rails framework so I'm not sure what the error in the image below means.

I believe that all the tests pass (because of the 0 failures) but then codecov throws an error about not finding a repo or a owner and I'm not quite sure what this means and how to debug it. Is this error related to the codebase or just specific to codecov?

Thank you!

[cesswairimu]

Hi @PeculiarE you are right, for codecov, we normally use it on pull requests to check for test coverage and its weird that the reports are appearing locally I will look into that but you can ignore those for now, thanks

[PeculiarE]

Alright! Thank you 😄 @cesswairimu

[PeculiarE]

Hi @jywarren....so I've been working on the single spam and batch spam endpoints for the past few days. Usually, the policy at PublicLab.org is that spamming content bans the author, but since we have anonymous authors on MapKnitter, what I implemented checks if the owner of the map is anonymous, and if yes, the only action to be carried out will be the spamming of the map; banning of the author is skipped.

Does this pass muster? Or is there a better way of going about maps with anonymous authors?

[jywarren]

what I implemented checks if the owner of the map is anonymous, and if yes, the only action to be carried out will be the spamming of the map; banning of the author is skipped.

I think that sounds great. Thanks!

[PeculiarE]

Hi @jywarren, so I have a PR up at #1762 on spamming maps (either singly or in batches).

Code Climate is requesting for a bit of refactoring to reduce complexity. While I work on that, I'd love to get a bit of clarity on some questions:

1. In the project goals, we have:

• Display a list of any maps a banned PublicLab.org user has (if any) when they are banned
• Offer a button to spam all maps by that user in one click, like mapknitter.org/spam/user/

Now, when we spam a map or several maps, we ban the author(s) (as long as they aren't anonymous). Based on the project goals above, is the spamming cycle supposed to run like this:

spam a map -> automatically ban the author -> spam all maps belonging to that author

Or does this feature of spamming all maps belonging to a banned author only apply to when the moderator manually clicks on ban user(s)?

2. In the general project discussion thread at #10754, you mentioned that advertisement on a map or any of its images was a major spam flag. So, this got me thinking: when spamming a map, we ban the main author (the user who created the map), but shouldn't we also ban co-authors (users who uploaded images on the map) if any?

[jywarren]

Hi @PeculiarE - sounds super, this is great to see.

to your questions, for 1) each of these takes some careful consideration. I think it's likely that an author posting spam will mean their previous maps are also spam. But it's not 100% the case. I think it's OK to spam all maps belonging to that author, as long as we a) clearly tell the admin that this happened, and b) link to an easy place to review what those maps were and let the admin undo the action if need be. This could just be that in the alert we show, we name the maps and link to them, and that we ensure that an "unspam" button is on each map page we link to. OR, maybe it's easier to just link to a sorted list of recently spammed maps? But if it's an old map, maybe it won't appear in a list of spammed maps, since we probably don't track the timestamp of when we spammed it? What do you think? I think maybe easier to link to each map and put a button on those pages (visible to mods only) which lets us undo the action? I believe there is a per-author page with a list of maps too. So that's a 3rd option perhaps - show map state there, and a button to republish the map or something? Isn't it supposed to be something like http://mapknitter.org/profile/warren ?

2. Hmm, co-authors... well, let's see -- looking over some spam maps, do we see that multiple authors is a common scenario? I might guess that spammers don't work in synchrony, or that a single spammer doesn't make multiple accounts... that would be my guess. So I think it's unnecessary... what do you think?