flag app for rereview if rating changes to adult (bug 939220) #1409
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,7 +52,7 @@ | |
from market.models import AddonPremium, Price, PriceCurrency | ||
from translations.models import Translation | ||
from versions.models import ApplicationsVersions, Version | ||
from users.models import RequestUser | ||
from users.models import RequestUser, UserProfile | ||
|
||
import mkt | ||
from mkt.constants import regions | ||
|
@@ -601,17 +601,6 @@ def assert_no_validation_errors(validation): | |
error.rstrip().split("\n")[-1]) | ||
|
||
|
||
def app_factory(**kw): | ||
kw.update(type=amo.ADDON_WEBAPP) | ||
app = amo.tests.addon_factory(**kw) | ||
if waffle.switch_is_active('iarc') and kw.get('rated'): | ||
ContentRating.objects.create(addon=app, ratings_body=0, rating=0) | ||
app.set_iarc_info(123, 'abc') | ||
app.set_descriptors([]) | ||
app.set_interactives([]) | ||
return app | ||
|
||
|
||
def _get_created(created): | ||
""" | ||
Returns a datetime. | ||
|
@@ -674,32 +663,15 @@ def addon_factory(version_kw={}, file_kw={}, **kw): | |
return a | ||
|
||
|
||
def version_factory(file_kw={}, **kw): | ||
min_app_version = kw.pop('min_app_version', '4.0') | ||
max_app_version = kw.pop('max_app_version', '5.0') | ||
version = kw.pop('version', '%.1f' % random.uniform(0, 2)) | ||
v = Version.objects.create(version=version, **kw) | ||
v.created = v.last_updated = _get_created(kw.pop('created', 'now')) | ||
v.save() | ||
if kw.get('addon').type not in (amo.ADDON_PERSONA, amo.ADDON_WEBAPP): | ||
a, _ = Application.objects.get_or_create(id=amo.FIREFOX.id) | ||
av_min, _ = AppVersion.objects.get_or_create(application=a, | ||
version=min_app_version) | ||
av_max, _ = AppVersion.objects.get_or_create(application=a, | ||
version=max_app_version) | ||
ApplicationsVersions.objects.get_or_create(application=a, version=v, | ||
min=av_min, max=av_max) | ||
file_factory(version=v, **file_kw) | ||
return v | ||
|
||
|
||
def file_factory(**kw): | ||
v = kw['version'] | ||
p, _ = Platform.objects.get_or_create(id=amo.PLATFORM_ALL.id) | ||
status = kw.pop('status', amo.STATUS_PUBLIC) | ||
f = File.objects.create(filename='%s-%s' % (v.addon_id, v.id), | ||
platform=p, status=status, **kw) | ||
return f | ||
def app_factory(**kw): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. did you just move these? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i alphabetized them |
||
kw.update(type=amo.ADDON_WEBAPP) | ||
app = amo.tests.addon_factory(**kw) | ||
if waffle.switch_is_active('iarc') and kw.get('rated'): | ||
app.content_ratings.create(ratings_body=0, rating=0) | ||
app.set_iarc_info(123, 'abc') | ||
app.set_descriptors([]) | ||
app.set_interactives([]) | ||
return app | ||
|
||
|
||
def collection_factory(**kw): | ||
|
@@ -725,6 +697,15 @@ def collection_factory(**kw): | |
return c | ||
|
||
|
||
def file_factory(**kw): | ||
v = kw['version'] | ||
p, _ = Platform.objects.get_or_create(id=amo.PLATFORM_ALL.id) | ||
status = kw.pop('status', amo.STATUS_PUBLIC) | ||
f = File.objects.create(filename='%s-%s' % (v.addon_id, v.id), | ||
platform=p, status=status, **kw) | ||
return f | ||
|
||
|
||
def req_factory_factory(url, user=None, post=False, data=None): | ||
"""Creates a request factory, logged in with the user.""" | ||
req = RequestFactory() | ||
|
@@ -741,6 +722,38 @@ def req_factory_factory(url, user=None, post=False, data=None): | |
return req | ||
|
||
|
||
user_factory_counter = 0 | ||
|
||
|
||
def user_factory(**kw): | ||
global user_factory_counter | ||
|
||
username = 'factoryuser%d' % user_factory_counter | ||
user = UserProfile.objects.create( | ||
username=username, email='%s@mozilla.com' % username, **kw) | ||
user_factory_counter = user.id + 1 | ||
return user | ||
|
||
|
||
def version_factory(file_kw={}, **kw): | ||
min_app_version = kw.pop('min_app_version', '4.0') | ||
max_app_version = kw.pop('max_app_version', '5.0') | ||
version = kw.pop('version', '%.1f' % random.uniform(0, 2)) | ||
v = Version.objects.create(version=version, **kw) | ||
v.created = v.last_updated = _get_created(kw.pop('created', 'now')) | ||
v.save() | ||
if kw.get('addon').type not in (amo.ADDON_PERSONA, amo.ADDON_WEBAPP): | ||
a, _ = Application.objects.get_or_create(id=amo.FIREFOX.id) | ||
av_min, _ = AppVersion.objects.get_or_create(application=a, | ||
version=min_app_version) | ||
av_max, _ = AppVersion.objects.get_or_create(application=a, | ||
version=max_app_version) | ||
ApplicationsVersions.objects.get_or_create(application=a, version=v, | ||
min=av_min, max=av_max) | ||
file_factory(version=v, **file_kw) | ||
return v | ||
|
||
|
||
class ESTestCase(TestCase): | ||
"""Base class for tests that require elasticsearch.""" | ||
# ES is slow to set up so this uses class setup/teardown. That happens | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,9 +3,14 @@ | |
|
||
import cronjobs | ||
from celery.task.sets import TaskSet | ||
from tower import ugettext as _ | ||
|
||
import lib.iarc | ||
import amo | ||
from amo.utils import chunked | ||
from editors.models import RereviewQueue | ||
|
||
import lib.iarc | ||
|
||
from lib.iarc.utils import DESC_MAPPING, RATINGS_MAPPING | ||
from mkt.developers.tasks import region_email, region_exclude | ||
from mkt.webapps.models import AddonExcludedRegion, Webapp | ||
|
@@ -87,7 +92,8 @@ def process_iarc_changes(date=None): | |
ratings_body = row.get('rating_system') | ||
rating = RATINGS_MAPPING[ratings_body].get(row['new_rating']) | ||
|
||
# TODO: If rating is adult, flag for re-review. | ||
_flag_rereview_adult(app, ratings_body, rating) | ||
|
||
# Save new rating. | ||
app.set_content_ratings({ratings_body: rating}) | ||
|
||
|
@@ -108,3 +114,15 @@ def process_iarc_changes(date=None): | |
except Exception as e: | ||
log.debug('Exception: %s' % e) | ||
continue | ||
|
||
|
||
def _flag_rereview_adult(app, ratings_body, rating): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we going to rely on doing this in the cron only? not during IARC upload? I'm not super clear on this flow. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only during the cron, if the IARC does a manual review on an app and "asynchronously" changes its rating to Adult. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok |
||
"""Flag app for rereview if it receives an Adult content rating.""" | ||
old_rating = app.content_ratings.filter(ratings_body=ratings_body.id) | ||
if not old_rating.exists(): | ||
return | ||
|
||
if rating.adult and not old_rating[0].get_rating().adult: | ||
RereviewQueue.flag( | ||
app, amo.LOG.CONTENT_RATING_TO_ADULT, | ||
message=_('Content rating changed to Adult.')) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,11 +4,12 @@ | |
from nose.tools import eq_ | ||
|
||
import amo.tests | ||
from devhub.models import ActivityLog | ||
|
||
import mkt | ||
import mkt.constants | ||
from mkt.developers.cron import (exclude_new_region, process_iarc_changes, | ||
send_new_region_emails) | ||
from mkt.developers.cron import (_flag_rereview_adult, exclude_new_region, | ||
process_iarc_changes, send_new_region_emails) | ||
from mkt.webapps.models import IARCInfo | ||
|
||
|
||
|
@@ -90,7 +91,6 @@ def test_processing(self): | |
}) | ||
|
||
process_iarc_changes() | ||
|
||
app = app.reload() | ||
|
||
# Check ratings. | ||
|
@@ -107,3 +107,34 @@ def test_processing(self): | |
['has_classind_shocking', 'has_classind_sex_content', | ||
'has_classind_drugs', 'has_classind_lang', 'has_classind_nudity', | ||
'has_classind_violence_extreme']) | ||
|
||
def test_rereview_flag_adult(self): | ||
amo.set_user(amo.tests.user_factory()) | ||
app = amo.tests.app_factory() | ||
|
||
app.set_content_ratings({ | ||
mkt.ratingsbodies.ESRB: mkt.ratingsbodies.ESRB_E, | ||
mkt.ratingsbodies.CLASSIND: mkt.ratingsbodies.CLASSIND_18, | ||
}) | ||
_flag_rereview_adult(app, mkt.ratingsbodies.ESRB, | ||
mkt.ratingsbodies.ESRB_T) | ||
assert not app.rereviewqueue_set.count() | ||
assert not ActivityLog.objects.filter( | ||
action=amo.LOG.CONTENT_RATING_TO_ADULT.id).exists() | ||
|
||
# Adult should get flagged to rereview. | ||
_flag_rereview_adult(app, mkt.ratingsbodies.ESRB, | ||
mkt.ratingsbodies.ESRB_A) | ||
eq_(app.rereviewqueue_set.count(), 1) | ||
eq_(ActivityLog.objects.filter( | ||
action=amo.LOG.CONTENT_RATING_TO_ADULT.id).count(), 1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like how you separated this logic so testing is nice and easy. 👍 |
||
|
||
# Test things same same if rating stays the same as adult. | ||
app.set_content_ratings({ | ||
mkt.ratingsbodies.ESRB: mkt.ratingsbodies.ESRB_A, | ||
}) | ||
_flag_rereview_adult(app, mkt.ratingsbodies.ESRB, | ||
mkt.ratingsbodies.ESRB_A) | ||
eq_(app.rereviewqueue_set.count(), 1) | ||
eq_(ActivityLog.objects.filter( | ||
action=amo.LOG.CONTENT_RATING_TO_ADULT.id).count(), 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line break pls