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

Projects: set abuse threshold for project assets #28899

Merged
merged 12 commits into from Jun 10, 2019
Merged

Conversation

Erin007
Copy link
Contributor

@Erin007 Erin007 commented Jun 3, 2019

LP-471

In #28455 I increased the project abuse score from 10 to 15 so that a project would need to be reported at least twice by a user (unless they are a verified teacher). However, I did not set the abuse score threshold for project assets. When a project was reported once and had an abuse score of 10, the project would still be shown, but the image thumbnail appeared broken, which was reported as 404s.
Screen Shot 2019-06-03 at 11 50 21 AM

This PR increased the abuse score threshold for project assets to 15 as well so that assets with abuse scores of 10 can still be retrieved and won't show as broken thumbnails.

@Erin007 Erin007 requested a review from davidsbailey June 3, 2019 22:43

# abuse_threshold here should match ABUSE_THRESHOLD in project.js to keep
# our abuse score threshold consistent for projects and their assets.
abuse_threshold = 15
Copy link
Member

Choose a reason for hiding this comment

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

there is a way to share constants programmatically. Please add this constant to shared_constants.rb, and then run cd apps ; grunt exec:generateSharedConstants. if that doesn't work try

`npm bin`/grunt exec:generateSharedConstants

this will make them show up in apps/src/util/sharedConstants.js .

Copy link
Member

@davidsbailey davidsbailey left a comment

Choose a reason for hiding this comment

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

Nice improvement Erin! Looks good to merge after addressing shared constants.

I am seeing one more problematic case to consider -- if my project is featured, and then I add an asset to it, it will be created with an abuse score of 0:

def create_or_replace(encrypted_channel_id, filename, body, version = nil, abuse_score = 0)

possible solution: when creating an asset, look up the project abuse score and then pass that score as an already-defined optional parameter to create_or_replace at these two call sites:

this seems fine to do in a later PR. the applab asset / gamelab animation case may be somewhat unlikely to ever occur, but since these apps are constantly regenerating their thumbnails, it seems worth tracking and addressing the files_put_file call site soon.

@Erin007 Erin007 merged commit 0d41880 into staging Jun 10, 2019
@Erin007 Erin007 deleted the featured-projects-fixes branch June 10, 2019 17:34
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.

None yet

2 participants