From 1e82ff7f0690e89d8e7e3a3222d2d084eacce028 Mon Sep 17 00:00:00 2001 From: Lauren Barker Date: Wed, 18 Oct 2017 15:39:47 -0400 Subject: [PATCH 1/3] Update global_reviews notification display text Rename migrations Use variable for related node Use constant in mails.py Change message field on NotificationDigest from char to text Add management command for adding subscriptions to users --- .../commands/add_notification_subscription.py | 74 +++++++++++++++++++ ...1_add_reviews_notification_subscription.py | 20 +++++ osf/migrations/0061_auto_20171002_1438.py | 49 ------------ osf/migrations/0062_auto_20171004_1506.py | 2 +- ...oned_false_for_all_submitted_preprints.py} | 0 osf/migrations/0066_auto_20171019_0918.py | 20 +++++ osf/models/notifications.py | 2 +- reviews/models/mixins.py | 16 ++-- website/mails/mails.py | 5 ++ website/notifications/constants.py | 2 +- 10 files changed, 132 insertions(+), 58 deletions(-) create mode 100644 osf/management/commands/add_notification_subscription.py create mode 100644 osf/migrations/0061_add_reviews_notification_subscription.py delete mode 100644 osf/migrations/0061_auto_20171002_1438.py rename osf/migrations/{0065_auto_20171016_1201.py => 0065_set_abandoned_false_for_all_submitted_preprints.py} (100%) create mode 100644 osf/migrations/0066_auto_20171019_0918.py diff --git a/osf/management/commands/add_notification_subscription.py b/osf/management/commands/add_notification_subscription.py new file mode 100644 index 00000000000..790a0dabe42 --- /dev/null +++ b/osf/management/commands/add_notification_subscription.py @@ -0,0 +1,74 @@ +# -*- coding: utf-8 -*- +# This is a management command, rather than a migration script, for two primary reasons: +# 1. It makes no changes to database structure (e.g. AlterField), only database content. +# 2. It takes a long time to run and the site doesn't need to be down that long. + +from __future__ import unicode_literals +import logging + +import django +django.setup() + +from django.core.management.base import BaseCommand +from django.db import transaction + +from osf.models import OSFUser, NotificationSubscription + +from website.notifications.utils import to_subscription_key + +from scripts import utils as script_utils + +logger = logging.getLogger(__name__) + + +def add_reviews_notification_setting(notification_type): + active_users = OSFUser.objects.filter(date_confirmed__isnull=False).exclude(date_disabled__isnull=False).exclude(is_active=False).order_by('id') + total_active_users = active_users.count() + + logger.info('About to add a global_reviews setting for {} users.'.format(total_active_users)) + + total_created = 0 + for user in active_users.iterator(): + user_subscription_id = to_subscription_key(user._id, notification_type) + + subscription = NotificationSubscription.load(user_subscription_id) + if not subscription: + logger.info('No {} subscription found for user {}. Subscribing...'.format(notification_type, user._id)) + subscription = NotificationSubscription(_id=user_subscription_id, owner=user, event_name=notification_type) + subscription.save() # Need to save in order to access m2m fields + subscription.add_user_to_subscription(user, 'email_transactional') + else: + logger.info('User {} already has a {} subscription'.format(user._id, notification_type)) + total_created += 1 + + logger.info('Added subscriptions for {}/{} users'.format(total_created, total_active_users)) + + +class Command(BaseCommand): + """ + Add subscription to all active users for given notification type. + """ + def add_arguments(self, parser): + super(Command, self).add_arguments(parser) + parser.add_argument( + '--dry', + action='store_true', + dest='dry_run', + help='Run migration and roll back changes to db', + ) + + parser.add_argument( + '--notification', + type=str, + required=True, + help='Notification type to subscribe users to', + ) + + def handle(self, *args, **options): + dry_run = options.get('dry_run', False) + if not dry_run: + script_utils.add_file_logger(logger, __file__) + with transaction.atomic(): + add_reviews_notification_setting(notification_type=options['notification']) + if dry_run: + raise RuntimeError('Dry run, transaction rolled back.') diff --git a/osf/migrations/0061_add_reviews_notification_subscription.py b/osf/migrations/0061_add_reviews_notification_subscription.py new file mode 100644 index 00000000000..5338da23856 --- /dev/null +++ b/osf/migrations/0061_add_reviews_notification_subscription.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.4 on 2017-10-02 19:38 +from __future__ import unicode_literals + +from django.db import migrations +from django.core.management import call_command + + +def add_reviews_notification_subscription(apps, schema_editor): + call_command('add_notification_subscription', '--notification=global_reviews') + +class Migration(migrations.Migration): + + dependencies = [ + ('osf', '0060_reviews'), + ] + + operations = [ + migrations.RunPython(add_reviews_notification_subscription), + ] diff --git a/osf/migrations/0061_auto_20171002_1438.py b/osf/migrations/0061_auto_20171002_1438.py deleted file mode 100644 index d6ca5150aa5..00000000000 --- a/osf/migrations/0061_auto_20171002_1438.py +++ /dev/null @@ -1,49 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.11.4 on 2017-10-02 19:38 -from __future__ import unicode_literals - -import logging - -from django.db import migrations - -from osf.models import OSFUser -from osf.models import NotificationSubscription -from website.notifications.utils import to_subscription_key - -logger = logging.getLogger(__name__) -logging.basicConfig(level=logging.INFO) - - -def add_reviews_notification_setting(*args, **kwargs): - active_users = OSFUser.objects.filter(date_confirmed__isnull=False).exclude(date_disabled__isnull=False).exclude(is_active=False).order_by('id') - total_active_users = active_users.count() - reviews_notification = 'global_reviews' - - logger.info('About to add a global_reviews setting for {} users.'.format(total_active_users)) - - total_created = 0 - for user in active_users.iterator(): - user_subscription_id = to_subscription_key(user._id, reviews_notification) - - subscription = NotificationSubscription.load(user_subscription_id) - if not subscription: - logger.info('No {} subscription found for user {}. Subscribing...'.format(reviews_notification, user._id)) - subscription = NotificationSubscription(_id=user_subscription_id, owner=user, event_name=reviews_notification) - subscription.save() # Need to save in order to access m2m fields - subscription.add_user_to_subscription(user, 'email_transactional') - else: - logger.info('User {} already has a {} subscription'.format(user._id, reviews_notification)) - total_created += 1 - - logger.info('Added subscriptions for {}/{} users'.format(total_created, total_active_users)) - - -class Migration(migrations.Migration): - - dependencies = [ - ('osf', '0060_reviews'), - ] - - operations = [ - migrations.RunPython(add_reviews_notification_setting), - ] diff --git a/osf/migrations/0062_auto_20171004_1506.py b/osf/migrations/0062_auto_20171004_1506.py index b204ea3dcb0..b02422395c8 100644 --- a/osf/migrations/0062_auto_20171004_1506.py +++ b/osf/migrations/0062_auto_20171004_1506.py @@ -8,7 +8,7 @@ class Migration(migrations.Migration): dependencies = [ - ('osf', '0061_auto_20171002_1438'), + ('osf', '0061_add_reviews_notification_subscription'), ] operations = [ diff --git a/osf/migrations/0065_auto_20171016_1201.py b/osf/migrations/0065_set_abandoned_false_for_all_submitted_preprints.py similarity index 100% rename from osf/migrations/0065_auto_20171016_1201.py rename to osf/migrations/0065_set_abandoned_false_for_all_submitted_preprints.py diff --git a/osf/migrations/0066_auto_20171019_0918.py b/osf/migrations/0066_auto_20171019_0918.py new file mode 100644 index 00000000000..b6b786f1a3f --- /dev/null +++ b/osf/migrations/0066_auto_20171019_0918.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.4 on 2017-10-19 14:18 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('osf', '0065_set_abandoned_false_for_all_submitted_preprints'), + ] + + operations = [ + migrations.AlterField( + model_name='notificationdigest', + name='message', + field=models.TextField(), + ), + ] diff --git a/osf/models/notifications.py b/osf/models/notifications.py index abf78cc4399..ec1a230fb71 100644 --- a/osf/models/notifications.py +++ b/osf/models/notifications.py @@ -86,6 +86,6 @@ class NotificationDigest(ObjectIDMixin, BaseModel): timestamp = NonNaiveDateTimeField() send_type = models.CharField(max_length=50, db_index=True, validators=[validate_subscription_type, ]) event = models.CharField(max_length=50) - message = models.CharField(max_length=10000) + message = models.TextField() # TODO: Could this be a m2m with or without an order field? node_lineage = ArrayField(models.CharField(max_length=5)) diff --git a/reviews/models/mixins.py b/reviews/models/mixins.py index 6a5ad1ddd15..e5a1723e542 100644 --- a/reviews/models/mixins.py +++ b/reviews/models/mixins.py @@ -175,9 +175,10 @@ def update_last_transitioned(self, ev): self.reviewable.date_last_transitioned = now def save_changes(self, ev): + node = self.reviewable.node + node._has_abandoned_preprint = False now = self.action.date_created if self.action is not None else timezone.now() should_publish = self.reviewable.in_public_reviews_state - self.reviewable.node._has_abandoned_preprint = False if should_publish and not self.reviewable.is_published: if not (self.reviewable.node.preprint_file and self.reviewable.node.preprint_file.node == self.reviewable.node): raise ValueError('Preprint node is not a valid preprint; cannot publish.') @@ -191,7 +192,7 @@ def save_changes(self, ev): elif not should_publish and self.reviewable.is_published: self.reviewable.is_published = False self.reviewable.save() - self.reviewable.node.save() + node.save() def resubmission_allowed(self, ev): return self.reviewable.provider.reviews_workflow == workflow.Workflows.PRE_MODERATION.value @@ -199,7 +200,6 @@ def resubmission_allowed(self, ev): def notify_submit(self, ev): context = self.get_context() context['referrer'] = ev.kwargs.get('user') - context['template'] = 'reviews_submission_confirmation' user = ev.kwargs.get('user') auth = Auth(user) self.reviewable.node.add_log( @@ -266,12 +266,16 @@ def reviews_notification(self, context): # Handle email notifications for a new submission. @reviews_signals.reviews_email_submit.connect def reviews_submit_notification(self, context): - template = context['template'] event_type = utils.find_subscription_type('global_reviews') for user_id in context['email_recipients']: user = OSFUser.load(user_id) user_subscriptions = get_user_subscriptions(user, event_type) context['no_future_emails'] = user_subscriptions['none'] context['is_creator'] = user == context.get('reviewable').node.creator - email = mails.Mail(template, subject='Confirmation of your submission to {provider}'.format(provider=context.get('reviewable').provider.name)) - mails.send_mail(user.username, email, mimetype='html', user=user, **context) + mails.send_mail( + user.username, + getattr(mails, 'REVIEWS_SUBMISSION_CONFIRMATION')(context.get('reviewable').provider.name), + mimetype='html', + user=user, + **context + ) diff --git a/website/mails/mails.py b/website/mails/mails.py index 673a8584c10..6686ff2d24e 100644 --- a/website/mails/mails.py +++ b/website/mails/mails.py @@ -372,3 +372,8 @@ def get_english_article(word): 'send_data_share_preprint_error_desk', subject='Share Error' ) + +REVIEWS_SUBMISSION_CONFIRMATION = lambda provider_name: Mail( + 'reviews_submission_confirmation', + subject='Confirmation of your submission to {}'.format(provider_name) +) diff --git a/website/notifications/constants.py b/website/notifications/constants.py index c34d9e50c3b..9fa6a53098b 100644 --- a/website/notifications/constants.py +++ b/website/notifications/constants.py @@ -11,7 +11,7 @@ 'global_comments': 'Comments added', 'global_file_updated': 'Files updated', 'global_mentions': 'Mentions added', - 'global_reviews': 'Preprints submission updated' + 'global_reviews': 'Preprint submissions updated' } # Note: the python value None mean inherit from parent From 7472a82c57be5ef8a0617470737613a259273e48 Mon Sep 17 00:00:00 2001 From: Lauren Barker Date: Thu, 19 Oct 2017 15:27:56 -0400 Subject: [PATCH 2/3] Add provider_name to the context --- reviews/models/mixins.py | 5 +++-- website/mails/mails.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/reviews/models/mixins.py b/reviews/models/mixins.py index e5a1723e542..f5a84bf080e 100644 --- a/reviews/models/mixins.py +++ b/reviews/models/mixins.py @@ -271,10 +271,11 @@ def reviews_submit_notification(self, context): user = OSFUser.load(user_id) user_subscriptions = get_user_subscriptions(user, event_type) context['no_future_emails'] = user_subscriptions['none'] - context['is_creator'] = user == context.get('reviewable').node.creator + context['is_creator'] = user == context['reviewable'].node.creator + context['provider_name'] = context['reviewable'].provider.name mails.send_mail( user.username, - getattr(mails, 'REVIEWS_SUBMISSION_CONFIRMATION')(context.get('reviewable').provider.name), + mails.REVIEWS_SUBMISSION_CONFIRMATION, mimetype='html', user=user, **context diff --git a/website/mails/mails.py b/website/mails/mails.py index 6686ff2d24e..488ca5585fc 100644 --- a/website/mails/mails.py +++ b/website/mails/mails.py @@ -373,7 +373,7 @@ def get_english_article(word): subject='Share Error' ) -REVIEWS_SUBMISSION_CONFIRMATION = lambda provider_name: Mail( +REVIEWS_SUBMISSION_CONFIRMATION = Mail( 'reviews_submission_confirmation', - subject='Confirmation of your submission to {}'.format(provider_name) + subject='Confirmation of your submission to ${provider_name}' ) From 25e90142d8946b29071b67b461218ce509626548 Mon Sep 17 00:00:00 2001 From: Lauren Barker Date: Thu, 19 Oct 2017 16:07:30 -0400 Subject: [PATCH 3/3] Clean up the migrations --- ..._preprints.py => 0062_accept_preprints.py} | 2 +- osf/migrations/0062_auto_20171004_1506.py | 20 ------------ ...12_1215.py => 0063_merge_20171012_1215.py} | 2 +- ...019_0918.py => 0064_auto_20171019_0918.py} | 2 +- ...doned_false_for_all_submitted_preprints.py | 31 ------------------- 5 files changed, 3 insertions(+), 54 deletions(-) rename osf/migrations/{0063_accept_preprints.py => 0062_accept_preprints.py} (93%) delete mode 100644 osf/migrations/0062_auto_20171004_1506.py rename osf/migrations/{0064_merge_20171012_1215.py => 0063_merge_20171012_1215.py} (87%) rename osf/migrations/{0066_auto_20171019_0918.py => 0064_auto_20171019_0918.py} (84%) delete mode 100644 osf/migrations/0065_set_abandoned_false_for_all_submitted_preprints.py diff --git a/osf/migrations/0063_accept_preprints.py b/osf/migrations/0062_accept_preprints.py similarity index 93% rename from osf/migrations/0063_accept_preprints.py rename to osf/migrations/0062_accept_preprints.py index 6cc7a6ccea4..2e93be4e382 100644 --- a/osf/migrations/0063_accept_preprints.py +++ b/osf/migrations/0062_accept_preprints.py @@ -19,7 +19,7 @@ def accept_all_published_preprints(apps, schema_editor): class Migration(migrations.Migration): dependencies = [ - ('osf', '0062_auto_20171004_1506'), + ('osf', '0061_add_reviews_notification_subscription'), ] operations = [ diff --git a/osf/migrations/0062_auto_20171004_1506.py b/osf/migrations/0062_auto_20171004_1506.py deleted file mode 100644 index b02422395c8..00000000000 --- a/osf/migrations/0062_auto_20171004_1506.py +++ /dev/null @@ -1,20 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.11.4 on 2017-10-04 20:06 -from __future__ import unicode_literals - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('osf', '0061_add_reviews_notification_subscription'), - ] - - operations = [ - migrations.AlterField( - model_name='notificationdigest', - name='message', - field=models.CharField(max_length=10000), - ), - ] diff --git a/osf/migrations/0064_merge_20171012_1215.py b/osf/migrations/0063_merge_20171012_1215.py similarity index 87% rename from osf/migrations/0064_merge_20171012_1215.py rename to osf/migrations/0063_merge_20171012_1215.py index 7fa645e0a83..762f3d5ee2e 100644 --- a/osf/migrations/0064_merge_20171012_1215.py +++ b/osf/migrations/0063_merge_20171012_1215.py @@ -9,7 +9,7 @@ class Migration(migrations.Migration): dependencies = [ ('osf', '0060_add_nodelog_should_hide_nid_index'), - ('osf', '0063_accept_preprints'), + ('osf', '0062_accept_preprints'), ] operations = [ diff --git a/osf/migrations/0066_auto_20171019_0918.py b/osf/migrations/0064_auto_20171019_0918.py similarity index 84% rename from osf/migrations/0066_auto_20171019_0918.py rename to osf/migrations/0064_auto_20171019_0918.py index b6b786f1a3f..194a33efc1a 100644 --- a/osf/migrations/0066_auto_20171019_0918.py +++ b/osf/migrations/0064_auto_20171019_0918.py @@ -8,7 +8,7 @@ class Migration(migrations.Migration): dependencies = [ - ('osf', '0065_set_abandoned_false_for_all_submitted_preprints'), + ('osf', '0063_merge_20171012_1215'), ] operations = [ diff --git a/osf/migrations/0065_set_abandoned_false_for_all_submitted_preprints.py b/osf/migrations/0065_set_abandoned_false_for_all_submitted_preprints.py deleted file mode 100644 index 46dd75d957d..00000000000 --- a/osf/migrations/0065_set_abandoned_false_for_all_submitted_preprints.py +++ /dev/null @@ -1,31 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.11.4 on 2017-10-16 17:01 -from __future__ import unicode_literals - -from django.db import migrations - -from reviews.workflow import States - - -# Make sure all submitted preprints have _has_abandoned_preprint=False. -def set_abandoned_false_for_all_submitted_preprints(apps, schema_editor): - Node = apps.get_model('osf', 'AbstractNode') - Node.objects.filter( - _has_abandoned_preprint=True, - preprints__isnull=False - ).exclude( - preprints__reviews_state=States.INITIAL.value - ).update(_has_abandoned_preprint=False) - - -class Migration(migrations.Migration): - - dependencies = [ - ('osf', '0064_merge_20171012_1215'), - ] - - operations = [ - migrations.RunPython( - set_abandoned_false_for_all_submitted_preprints - ), - ]