Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
 into add-gv-messaging

* 'develop' of https://github.com/CenterForOpenScience/osf.io:
  add waffling for GV (CenterForOpenScience#10571)
  Prevent iterable TypeError
  Exclude self when looking for users to merge
  [ENG-5356] Unlimit the number of highlighted subjects are displayed (CenterForOpenScience#10569)
  ENG-5429 fix styling for advisory board section (CenterForOpenScience#10570)
  Use public schema for function
  allow request `request_identifier_update` to create DOIs
  • Loading branch information
John Tordoff committed Mar 28, 2024
2 parents b8fe1b7 + 36a9e47 commit 7a016b0
Show file tree
Hide file tree
Showing 16 changed files with 54 additions and 49 deletions.
4 changes: 2 additions & 2 deletions admin/collection_providers/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def clean_description(self, *args, **kwargs):
self.data.get('description'),
tags=['a', 'br', 'em', 'p', 'span', 'strong'],
attributes=['class', 'style', 'href', 'title', 'target'],
styles=['text-align', 'vertical-align'],
styles=['text-align', 'vertical-align', 'color'],
strip=True
)

Expand All @@ -58,7 +58,7 @@ def clean_footer_links(self, *args, **kwargs):
self.data.get('footer_links'),
tags=['a', 'br', 'div', 'em', 'p', 'span', 'strong'],
attributes=['class', 'style', 'href', 'title', 'target'],
styles=['text-align', 'vertical-align'],
styles=['text-align', 'vertical-align', 'color'],
strip=True
)

Expand Down
6 changes: 3 additions & 3 deletions admin/preprint_providers/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def clean_advisory_board(self, *args, **kwargs):
self.data.get('advisory_board'),
tags=['a', 'b', 'br', 'div', 'em', 'h2', 'h3', 'li', 'p', 'strong', 'ul'],
attributes=['class', 'style', 'href', 'title', 'target'],
styles=['text-align', 'vertical-align'],
styles=['text-align', 'vertical-align', 'color'],
strip=True
)

Expand All @@ -70,7 +70,7 @@ def clean_description(self, *args, **kwargs):
self.data.get('description'),
tags=['a', 'br', 'em', 'p', 'span', 'strong'],
attributes=['class', 'style', 'href', 'title', 'target'],
styles=['text-align', 'vertical-align'],
styles=['text-align', 'vertical-align', 'color'],
strip=True
)

Expand All @@ -81,7 +81,7 @@ def clean_footer_links(self, *args, **kwargs):
self.data.get('footer_links'),
tags=['a', 'br', 'div', 'em', 'p', 'span', 'strong'],
attributes=['class', 'style', 'href', 'title', 'target'],
styles=['text-align', 'vertical-align'],
styles=['text-align', 'vertical-align', 'color'],
strip=True
)

Expand Down
4 changes: 2 additions & 2 deletions admin/registration_providers/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def clean_description(self, *args, **kwargs):
self.data.get('description'),
tags=['a', 'br', 'em', 'p', 'span', 'strong'],
attributes=['class', 'style', 'href', 'title', 'target'],
styles=['text-align', 'vertical-align'],
styles=['text-align', 'vertical-align', 'color'],
strip=True
)

Expand All @@ -70,7 +70,7 @@ def clean_footer_links(self, *args, **kwargs):
self.data.get('footer_links'),
tags=['a', 'br', 'div', 'em', 'p', 'span', 'strong'],
attributes=['class', 'style', 'href', 'title', 'target'],
styles=['text-align', 'vertical-align'],
styles=['text-align', 'vertical-align', 'color'],
strip=True
)

Expand Down
13 changes: 8 additions & 5 deletions osf/features.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@
# the old flipping code can be removed.
# 5. When a flag name is no longer referenced anywhere in this repo or in the Ember app remove it from this list.
flags:

- flag_name: ENABLE_GV
name: gravy_waffle
note: This is used to enable GravyValet, the system responible for addons, this will remove the files widget on the
project overview page. Will be used with EMBER_USER_SETTINGS_ADDONS and EMBER_NODE_SETTINGS_ADDONS to flip all
UI elements to the new addons system.
everyone: false

- flag_name: EMBER_FILE_PROJECT_DETAIL
name: ember_file_project_detail_page
note: This is part of the upcoming files page redesign
Expand Down Expand Up @@ -107,11 +115,6 @@ flags:
note: This flag controls wheter the `User Settings Page` view routes to the Ember app
everyone: false

- flag_name: EMBER_USER_SETTINGS_ADDONS
name: ember_user_settings_addons_page
note: This flag controls wheter the `User Addons Page` view routes to the Ember app
everyone: false

- flag_name: EMBER_USER_SETTINGS_NOTIFICATIONS
name: ember_user_settings_notifications_page
note: This flag controls wheter the `User Notifications Page` view routes to the Ember app
Expand Down
10 changes: 5 additions & 5 deletions osf/metrics/reporters/storage_addon_usage.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,14 @@ def storage_addon_user_counts(date, usersettings_model):
'deleted_daily': Count('pk', filter=(deleted_today)),
}
if issubclass(usersettings_model, BaseOAuthUserSettings):
# adding a temporary function (in the pg_temp schema) to contain
# all jsonb shenanigans -- this function counts the number of
# '<ExternalAccount._id>' keys nested one level deep
# adding a function to contain all jsonb shenanigans
# this function counts the number of '<ExternalAccount._id>'
# keys nested one level deep
# (see addons.base.models.BaseOAuthUserSettings for the expected
# structure of `oauth_grants`)
temp_function__count_oauth_grants = '''
CREATE OR REPLACE FUNCTION
pg_temp.count_oauth_grants(usersettings_oauth_grants jsonb)
public.count_oauth_grants(usersettings_oauth_grants jsonb)
RETURNS bigint AS $$
SELECT count(*)
FROM jsonb_object_keys(usersettings_oauth_grants) AS guid
Expand All @@ -96,7 +96,7 @@ def storage_addon_user_counts(date, usersettings_model):
cursor.execute(temp_function__count_oauth_grants)

usersettings_qs = usersettings_qs.annotate(
grant_count=Func('oauth_grants', function='pg_temp.count_oauth_grants'),
grant_count=Func('oauth_grants', function='public.count_oauth_grants'),
)
# each "grant" is a "link" to a node
has_link = ~deleted_before & Q(grant_count__gt=0)
Expand Down
5 changes: 3 additions & 2 deletions osf/models/identifiers.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,10 @@ def request_identifier(self, category):
if client:
return client.create_identifier(self, category)

def request_identifier_update(self, category):
def request_identifier_update(self, category, create=False):
'''Noop if no existing identifier value for the category.'''
if not self.get_identifier_value(category):

if not self.get_identifier_value(category) and not create:
return

client = self.get_doi_client()
Expand Down
2 changes: 1 addition & 1 deletion osf/models/private_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def node_ids(self):

def node_scale(self, node):
# node may be None if previous node's parent is deleted
if node is None or node.parent_id not in self.node_ids:
if node is None or not self.node_ids.filter(guids___id=node.parent_id).exists():
return -40
else:
offset = 20 if node.parent_node is not None else 0
Expand Down
15 changes: 2 additions & 13 deletions osf/models/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,9 @@ def has_highlighted_subjects(self):
@property
def highlighted_subjects(self):
if self.has_highlighted_subjects:
return self.subjects.filter(highlighted=True).order_by('text')[:10]
return self.subjects.filter(highlighted=True).order_by('text')
else:
return sorted(self.top_level_subjects, key=lambda s: s.text)[:10]
return sorted(self.top_level_subjects, key=lambda s: s.text)

@property
def top_level_subjects(self):
Expand Down Expand Up @@ -406,17 +406,6 @@ def all_subjects(self):
# TODO: Delet this when all PreprintProviders have a mapping
return rules_to_subjects(self.subjects_acceptable)

@property
def has_highlighted_subjects(self):
return self.subjects.filter(highlighted=True).exists()

@property
def highlighted_subjects(self):
if self.has_highlighted_subjects:
return self.subjects.filter(highlighted=True).order_by('text')[:10]
else:
return sorted(self.top_level_subjects, key=lambda s: s.text)[:10]

@property
def top_level_subjects(self):
if self.subjects.exists():
Expand Down
3 changes: 1 addition & 2 deletions osf/models/subject.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from website.util import api_v2_url

from .base import BaseModel, ObjectIDMixin
from .validators import validate_subject_hierarchy_length, validate_subject_highlighted_count
from .validators import validate_subject_hierarchy_length

class SubjectQuerySet(QuerySet):
def include_children(self):
Expand Down Expand Up @@ -86,7 +86,6 @@ def object_hierarchy(self):

def save(self, *args, **kwargs):
saved_fields = self.get_dirty_fields() or []
validate_subject_highlighted_count(self.provider, bool('highlighted' in saved_fields and self.highlighted))
if 'text' in saved_fields and self.pk and (self.preprints.exists() or self.abstractnodes.exists()):
raise ValidationError('Cannot edit a used Subject')
return super(Subject, self).save()
Expand Down
4 changes: 2 additions & 2 deletions osf/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -1371,9 +1371,9 @@ def confirm_email(self, token, merge=False):
# If this email is confirmed on another account, abort
try:
if check_select_for_update():
user_to_merge = OSFUser.objects.filter(emails__address=email).select_for_update().get()
user_to_merge = OSFUser.objects.exclude(id=self.id).filter(emails__address=email).select_for_update().get()
else:
user_to_merge = OSFUser.objects.get(emails__address=email)
user_to_merge = OSFUser.objects.exclude(id=self.id).get(emails__address=email)
except OSFUser.DoesNotExist:
user_to_merge = None

Expand Down
4 changes: 0 additions & 4 deletions osf/models/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,6 @@ def validate_email(value):
raise BlockedEmailError('Invalid Email')


def validate_subject_highlighted_count(provider, is_highlighted_addition):
if is_highlighted_addition and provider.subjects.filter(highlighted=True).count() >= 10:
raise DjangoValidationError('Too many highlighted subjects for PreprintProvider {}'.format(provider._id))

def validate_subject_hierarchy_length(parent):
from osf.models import Subject
parent = Subject.objects.get(id=parent)
Expand Down
23 changes: 22 additions & 1 deletion tests/test_preprints.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,19 @@
import pytz
import itsdangerous
from importlib import import_module
import pytest_socket

from django.contrib.auth.models import Group
from django.core.exceptions import ValidationError
from django.conf import settings as django_conf_settings

from website import settings, mails
from website.preprints.tasks import on_preprint_updated, update_or_create_preprint_identifiers, update_or_enqueue_on_preprint_updated
from website.preprints.tasks import (
on_preprint_updated,
update_or_create_preprint_identifiers,
update_or_enqueue_on_preprint_updated,
should_update_preprint_identifiers
)
from website.identifiers.clients import CrossRefClient, ECSArXivCrossRefClient, crossref
from website.identifiers.utils import request_identifiers
from framework.auth import signing
Expand Down Expand Up @@ -1924,6 +1930,7 @@ def setUp(self):

self.auth = Auth(user=self.user)
self.preprint = PreprintFactory()
self.private_preprint = PreprintFactory(is_published=False, creator=self.user)
thesis_provider = PreprintProviderFactory(share_publish_type='Thesis')
self.thesis = PreprintFactory(provider=thesis_provider)

Expand Down Expand Up @@ -1970,6 +1977,20 @@ def test_update_or_enqueue_on_preprint_updated(self):
assert 'title' in updated_task.kwargs['saved_fields']
assert 'contributors' in updated_task.kwargs['saved_fields']

def test_update_or_enqueue_on_preprint_doi_created(self):
assert not should_update_preprint_identifiers(self.private_preprint, {})
assert not self.private_preprint.identifiers.all()

with mock.patch.object(settings, 'CROSSREF_URL', 'https://test.crossref.org/servlet/deposit'):
with mock.patch.object(settings, 'CROSSREF_USERNAME', 'TestCrossrefUsername'):
with mock.patch.object(settings, 'CROSSREF_PASSWORD', 'TestCrossrefPassword'):
# This exception proved it tried to contact Crossref, testing the mailgun route is elsewhere, so no
# mocking when the DOI is confirmed.
with pytest.raises(pytest_socket.SocketConnectBlockedError):
self.private_preprint.set_published(True, self.auth, save=True)

assert should_update_preprint_identifiers(self.private_preprint, {})


class TestPreprintConfirmationEmails(OsfTestCase):
def setUp(self):
Expand Down
5 changes: 0 additions & 5 deletions tests/test_subjects.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,6 @@ def test_delete_used_subject(self):
with assert_raises(ValidationError):
self.subject.delete()

def test_max_highlighted_count(self):
highlights = [SubjectFactory(provider=self.subject.provider, highlighted=True) for _ in range(10)]
with assert_raises(ValidationError):
self.subject.highlighted=True
self.subject.save()

class TestSubjectProperties(OsfTestCase):
def setUp(self):
Expand Down
2 changes: 1 addition & 1 deletion website/preprints/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def should_update_preprint_identifiers(preprint, saved_fields):

def update_or_create_preprint_identifiers(preprint):
try:
preprint.request_identifier_update(category='doi')
preprint.request_identifier_update(category='doi', create=True)
except HTTPError as err:
sentry.log_exception()
sentry.log_message(err.args[0])
Expand Down
2 changes: 1 addition & 1 deletion website/profile/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ def user_account_password(auth, **kwargs):


@must_be_logged_in
@ember_flag_is_active(features.EMBER_USER_SETTINGS_ADDONS)
@ember_flag_is_active(features.ENABLE_GV)
def user_addons(auth, **kwargs):

user = auth.user
Expand Down
1 change: 1 addition & 0 deletions website/project/views/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ def node_setting(auth, node, **kwargs):
@must_not_be_registration
@must_be_logged_in
@must_have_permission(WRITE)
@ember_flag_is_active(features.ENABLE_GV)
def node_addons(auth, node, **kwargs):

ret = _view_project(node, auth, primary=True)
Expand Down

0 comments on commit 7a016b0

Please sign in to comment.