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

Enhancement: Add user policy objects to limit errant submission votes. #4121

Open
3 of 6 tasks
ZachBaird opened this issue Sep 7, 2023 · 10 comments · May be fixed by #4349
Open
3 of 6 tasks

Enhancement: Add user policy objects to limit errant submission votes. #4121

ZachBaird opened this issue Sep 7, 2023 · 10 comments · May be fixed by #4349

Comments

@ZachBaird
Copy link
Contributor

ZachBaird commented Sep 7, 2023

Complete the following REQUIRED checkboxes:

  • I have thoroughly read and understand The Odin Project Contributing Guide
  • The title of this issue follows the brief description of request format, e.g. Add dark mode to website

The following checkbox is OPTIONAL:

  • I would like to be assigned this issue to work on it

1. Description of the Feature Request:

@KevinMulhern and I were discussing how some users game their project submissions by creating dozens of accounts in order to dramatically inflate their project submission's votes.

I proposed adding policies on user accounts that would restrict a user from voting on project submissions until certain requirements were met. The following were suggested:

  • A user must have a project submission up in order to vote on any others.
  • A user's account must be at least 7 days old.

@KevinMulhern remarked that this was a good idea and could be achieved with policy objects fairly easily.

2. Acceptance Criteria:

  • A SubmissionVotePolicy (name can be anything really) should be created. It should contain logic validating if a user meets whatever criteria this task finalizes on.
  • The new policy should be initialized [in the LikesController]. Policy should then be used in the update method.
  • Update tests to reflect that a user can be unauthorized from voting on a submission.
@ZachBaird ZachBaird added the Status: Needs Review This issue/PR needs an initial or additional review label Sep 7, 2023
@KevinMulhern
Copy link
Member

Thanks for getting this going @ZachBaird. Some new thoughts this morning.

  • We have a current_user_ip which has been helpful in tracking these incidents - it may be worth checking that against the project submission owners ip in the policy too.
  • I wonder if it will be worth having a way of short circuiting the check after users pass the criteria for the first time. Maybe with a permission boolean on the user record like can_like. That would would reduce subsequent queries to the projects table and be handy for tests.

@rlmoser99 has dealt with a few of these incidents 💪 I'd value any thoughts she has about the rules being proposed here.

@rlmoser99
Copy link
Member

rlmoser99 commented Sep 8, 2023

Hello @ZachBaird! So good to see you again!

A user must have a project submission up in order to vote on any others

I wonder if having lesson completions would be a better check, because I could see people liking a project on their first project before even starting on it.

I wonder if we only need to look at additional details if two users (voter and votee) have the same ip addresses, which is the very first thing that I looked at when researching these incidents.

I'll think on this more, but these are my initial thoughts.

@ZachBaird
Copy link
Contributor Author

I think the ip address check should be sufficient. We could always roll with that initially and add further restrictions if it still presents an annoying timesink.

@rlmoser99
Copy link
Member

There are valid reasons that two people have the same ip address, like two roommates doing TOP together. When I was researching some of these incidents, I noticed this a handful of times so think it would be useful to also make sure that the account is used - like by checking lesson completions.

@ZachBaird
Copy link
Contributor Author

I think that's reasonable! The validation logic can always be refined as needed.

@ZachBaird
Copy link
Contributor Author

@KevinMulhern @rlmoser99 Do we feel comfortable moving forward with this task at all?

We can start with checking lesson completions and implementing @KevinMulhern 's following suggestion:

I wonder if it will be worth having a way of short circuiting the check after users pass the criteria for the first time. Maybe with a permission boolean on the user record like can_like. That would would reduce subsequent queries to the projects table and be handy for tests.

That should give us a good starting point to see what else can be done while alleviating the problem.

@fabulousgk
Copy link

I am just adding my 2 cents as I saw this come up in the discord....

I was just pondering the project lists the other day when, once again, a learner in Discord brought up how much better the projects were, and they could not figure out how to do their own. I realized that a) a lot of people look at these even though we tell them not to, b) they use the well-designed projects as a metric for their own, even though this is a dev course, not a design course, and c) as discussed here, people game the system.

So, my question...what value do listing these projects have to learners and the process? We do have the showcase is Discord. I just wonder if these are more of a distraction than help.

@rlmoser99
Copy link
Member

@ZachBaird Yes, I am comfortable moving forward with this idea & that approach sounds right.

@rlmoser99
Copy link
Member

@fabulousgk Thank you for sharing your thoughts. Not all of our learners are in Discord, probably less than 20% join Discord. So, the showcase in Discord is not accessible for everyone.

I understand your other points, but I don't think removing project submissions is the answer. Feel free to continue to ponder these issues and see if you come up with another idea that we can implement & then make an issue for everyone to discuss.

@fabulousgk
Copy link

Thank you! I am amazed that the Discord population is that small. Wow!

@ZachBaird ZachBaird removed the Status: Needs Review This issue/PR needs an initial or additional review label Dec 27, 2023
@ZachBaird ZachBaird linked a pull request Jan 19, 2024 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 📋 Backlog / Ideas
Development

Successfully merging a pull request may close this issue.

4 participants