-
Notifications
You must be signed in to change notification settings - Fork 321
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
[PLAT-1181] Waffle all Prereg Challenge specific parts of the codebase #8833
Changes from 5 commits
3f32b5b
b2d26f4
d6afc55
d854276
c0514ba
30594d9
4a29ea8
bc920db
4486c60
60aa3d0
795f2d0
9bfe36a
1895ad5
d97ced0
b021d50
039c0b2
551d857
965c95e
7c89dc3
876c235
816c61b
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 |
---|---|---|
@@ -1,6 +1,11 @@ | ||
from nose.tools import * # noqa: F403 | ||
|
||
from waffle.testutils import override_switch | ||
from waffle.models import Switch | ||
from django.core.cache import cache | ||
|
||
from osf.models import RegistrationSchema | ||
from osf.features import OSF_PREREGISTRATION | ||
from website.prereg import prereg_landing_page as landing_page | ||
from website.prereg.utils import get_prereg_schema | ||
from website.registries.utils import drafts_for_user | ||
|
@@ -21,23 +26,55 @@ def test_not_logged_in(self): | |
'has_projects': False, | ||
'has_draft_registrations': False, | ||
'campaign_long': 'Prereg Challenge', | ||
'campaign_short': 'prereg', | ||
'campaign_short': 'prereg_challenge', | ||
'is_logged_in': False, | ||
} | ||
) | ||
|
||
cache.delete(Switch._cache_key(OSF_PREREGISTRATION)) | ||
|
||
with override_switch(name=OSF_PREREGISTRATION, active=True): | ||
assert_equal( | ||
landing_page(), | ||
{ | ||
'has_projects': False, | ||
'has_draft_registrations': False, | ||
'campaign_long': 'OSF Preregistration', | ||
'campaign_short': 'prereg', | ||
'is_logged_in': False, | ||
} | ||
) | ||
|
||
cache.delete(Switch._cache_key(OSF_PREREGISTRATION)) | ||
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. These are unnecessary, too, right? |
||
|
||
def test_no_projects(self): | ||
assert_equal( | ||
landing_page(user=self.user), | ||
{ | ||
'has_projects': False, | ||
'has_draft_registrations': False, | ||
'campaign_long': 'Prereg Challenge', | ||
'campaign_short': 'prereg', | ||
'campaign_short': 'prereg_challenge', | ||
'is_logged_in': True, | ||
} | ||
) | ||
|
||
cache.delete(Switch._cache_key(OSF_PREREGISTRATION)) | ||
|
||
with override_switch(name=OSF_PREREGISTRATION, active=True): | ||
assert_equal( | ||
landing_page(user=self.user), | ||
{ | ||
'has_projects': False, | ||
'has_draft_registrations': False, | ||
'campaign_long': 'OSF Preregistration', | ||
'campaign_short': 'prereg', | ||
'is_logged_in': True, | ||
} | ||
) | ||
|
||
cache.delete(Switch._cache_key(OSF_PREREGISTRATION)) | ||
|
||
def test_has_project(self): | ||
factories.ProjectFactory(creator=self.user) | ||
|
||
|
@@ -47,10 +84,25 @@ def test_has_project(self): | |
'has_projects': True, | ||
'has_draft_registrations': False, | ||
'campaign_long': 'Prereg Challenge', | ||
'campaign_short': 'prereg', | ||
'campaign_short': 'prereg_challenge', | ||
'is_logged_in': True, | ||
} | ||
) | ||
cache.delete(Switch._cache_key(OSF_PREREGISTRATION)) | ||
|
||
with override_switch(name=OSF_PREREGISTRATION, active=True): | ||
assert_equal( | ||
landing_page(user=self.user), | ||
{ | ||
'has_projects': True, | ||
'has_draft_registrations': False, | ||
'campaign_long': 'OSF Preregistration', | ||
'campaign_short': 'prereg', | ||
'is_logged_in': True, | ||
} | ||
) | ||
|
||
cache.delete(Switch._cache_key(OSF_PREREGISTRATION)) | ||
|
||
def test_has_project_and_draft_registration(self): | ||
prereg_schema = RegistrationSchema.objects.get(name='Prereg Challenge') | ||
|
@@ -65,11 +117,32 @@ def test_has_project_and_draft_registration(self): | |
'has_projects': True, | ||
'has_draft_registrations': True, | ||
'campaign_long': 'Prereg Challenge', | ||
'campaign_short': 'prereg', | ||
'campaign_short': 'prereg_challenge', | ||
'is_logged_in': True, | ||
} | ||
) | ||
|
||
cache.delete(Switch._cache_key(OSF_PREREGISTRATION)) | ||
|
||
with override_switch(name=OSF_PREREGISTRATION, active=True): | ||
prereg_schema = RegistrationSchema.objects.get(name='OSF Preregistration') | ||
factories.DraftRegistrationFactory( | ||
initiator=self.user, | ||
registration_schema=prereg_schema | ||
) | ||
assert_equal( | ||
landing_page(user=self.user), | ||
{ | ||
'has_projects': True, | ||
'has_draft_registrations': True, | ||
'campaign_long': 'OSF Preregistration', | ||
'campaign_short': 'prereg', | ||
'is_logged_in': True, | ||
} | ||
) | ||
|
||
cache.delete(Switch._cache_key(OSF_PREREGISTRATION)) | ||
|
||
def test_drafts_for_user_omits_registered(self): | ||
prereg_schema = RegistrationSchema.objects.get(name='Prereg Challenge', schema_version=2) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,9 +9,12 @@ | |
from osf.models import DraftRegistration, QueuedMail | ||
from osf.models.queued_mail import PREREG_REMINDER, PREREG_REMINDER_TYPE, queue_mail | ||
|
||
import waffle | ||
from website.app import init_app | ||
from website import settings | ||
|
||
from osf import features | ||
|
||
from scripts.utils import add_file_logger | ||
|
||
logger = logging.getLogger(__name__) | ||
|
@@ -69,8 +72,9 @@ def find_neglected_prereg_within_reminder_limit(): | |
|
||
@celery_app.task(name='scripts.remind_draft_preregistrations') | ||
def run_main(dry_run=True): | ||
init_app(routes=False) | ||
if not dry_run: | ||
add_file_logger(logger, __file__) | ||
main(dry_run=dry_run) | ||
if not waffle.switch_is_active(features.OSF_PREREGISTRATION): | ||
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. It might seem better to waffle this in the Celery config in 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. Discussed with David M. in person and we can completely remove this reminder email now (no sense reminding people to finish their prereg draft when there's two weeks remaining...). Go ahead and remove the script, task, and email template. |
||
init_app(routes=False) | ||
if not dry_run: | ||
add_file_logger(logger, __file__) | ||
main(dry_run=dry_run) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
from website.registries.utils import get_campaign_schema | ||
|
||
|
||
def get_prereg_schema(campaign='prereg'): | ||
def get_prereg_schema(campaign='prereg_challenge'): | ||
return get_campaign_schema(campaign) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,8 +9,13 @@ | |
* website/static/css/prereg.css | ||
""" | ||
|
||
import waffle | ||
|
||
from website.registries import views | ||
from osf import features | ||
|
||
def prereg_landing_page(**kwargs): | ||
"""Landing page for the prereg challenge""" | ||
return views._view_registries_landing_page('prereg', **kwargs) | ||
"""Landing page for osf prereg""" | ||
if waffle.switch_is_active(features.OSF_PREREGISTRATION): | ||
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 the changes to this function necessary? The same mako template is returned regardless of the switch and it doesn't look like 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. Ah, campaign is used to determine which draft registrations to load on the landing page. Got it. |
||
return views._view_registries_landing_page('prereg', **kwargs) | ||
return views._view_registries_landing_page('prereg_challenge', **kwargs) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,8 @@ | |
from osf.utils.functional import rapply | ||
from osf.models import NodeLog, RegistrationSchema, DraftRegistration, Sanction | ||
|
||
import waffle | ||
|
||
from website.exceptions import NodeStateError | ||
from website.project.decorators import ( | ||
must_be_valid_project, | ||
|
@@ -121,6 +123,9 @@ def submit_draft_for_review(auth, node, draft, *args, **kwargs): | |
:rtype: dict | ||
:raises: HTTPError if embargo end date is invalid | ||
""" | ||
if waffle.switch_is_active(features.OSF_PREREGISTRATION): | ||
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. Can you add a test to ensure that this function doesn't execute when the switch is active? 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. 👍 |
||
raise DeprecationWarning('The Prereg Challenge is over this endpoint is deprecated.') | ||
|
||
data = request.get_json() | ||
meta = {} | ||
registration_choice = data.get('registrationChoice', 'immediate') | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,14 +25,23 @@ | |
<div class="row-md-12 scripted"> | ||
<span data-bind="ifnot: draft.isPendingApproval"> | ||
<a type="button" class="btn btn-default pull-left" href="${draft['urls']['edit']}">Continue editing</a> | ||
|
||
%if waffle.switch_is_active(features.OSF_PREREGISTRATION): | ||
<button type="button" class="btn btn-success pull-right" | ||
data-toggle="tooltip" data-placement="top" title="The Prereg Challenge has expired" | ||
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. How about "The Prereg Challenge has ended."? Ended sounds a bit more accurate than expired imo. |
||
style="margin-left: 5px; | ||
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. Weird use of style tags here because the |
||
box-shadow: none; | ||
opacity: .65;"> | ||
Submit for review | ||
</button> | ||
%else: | ||
<button id="register-submit" type="button" class="btn btn-success pull-right" | ||
style="margin-left: 5px;" | ||
data-bind="visible: draft.requiresApproval, | ||
click: draft.submitForReview.bind(draft), | ||
enable: editor.canSubmit"> | ||
Submit for review | ||
</button> | ||
%endif | ||
</span> | ||
<span data-bind="if: draft.isPendingApproval"> | ||
<a type="button" class="btn btn-default pull-left" href="${web_url_for('node_registrations', pid=node['id'], tab='drafts', _guid=True)}"> Back </a> | ||
|
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.
Is this necessary? If you're using
override_switch
to set the value of a switch, you shouldn't have to worry about the cache.