Skip to content

Commit

Permalink
Resolve issue with updating preprint fields and validation errors
Browse files Browse the repository at this point in the history
  • Loading branch information
Uditi Mehta committed Sep 20, 2024
1 parent df41bb4 commit 235ce8f
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 42 deletions.
8 changes: 8 additions & 0 deletions api/preprints/serializers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,36 +362,42 @@ def update(self, preprint, validated_data):
if 'has_coi' in validated_data:
try:
preprint.update_has_coi(auth, validated_data['has_coi'])
save_preprint = True
except PreprintStateError as e:
raise exceptions.ValidationError(detail=str(e))

if 'conflict_of_interest_statement' in validated_data:
try:
preprint.update_conflict_of_interest_statement(auth, validated_data['conflict_of_interest_statement'])
save_preprint = True
except PreprintStateError as e:
raise exceptions.ValidationError(detail=str(e))

if 'has_data_links' in validated_data:
try:
preprint.update_has_data_links(auth, validated_data['has_data_links'])
save_preprint = True
except PreprintStateError as e:
raise exceptions.ValidationError(detail=str(e))

if 'why_no_data' in validated_data:
try:
preprint.update_why_no_data(auth, validated_data['why_no_data'])
save_preprint = True
except PreprintStateError as e:
raise exceptions.ValidationError(detail=str(e))

if 'data_links' in validated_data:
try:
preprint.update_data_links(auth, validated_data['data_links'])
save_preprint = True
except PreprintStateError as e:
raise exceptions.ValidationError(detail=str(e))

if 'has_prereg_links' in validated_data:
try:
preprint.update_has_prereg_links(auth, validated_data['has_prereg_links'])
save_preprint = True
except PreprintStateError as e:
raise exceptions.ValidationError(detail=str(e))

Expand All @@ -404,12 +410,14 @@ def update(self, preprint, validated_data):
if 'prereg_links' in validated_data:
try:
preprint.update_prereg_links(auth, validated_data['prereg_links'])
save_preprint = True
except PreprintStateError as e:
raise exceptions.ValidationError(detail=str(e))

if 'prereg_link_info' in validated_data:
try:
preprint.update_prereg_link_info(auth, validated_data['prereg_link_info'])
save_preprint = True
except PreprintStateError as e:
raise exceptions.ValidationError(detail=str(e))

Expand Down
40 changes: 22 additions & 18 deletions api_tests/preprints/views/test_preprint_detail.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -857,9 +857,9 @@ def test_update_conflict_of_interest_statement(self, app, user, preprint, url):
preprint.save()
res = app.patch_json_api(url, update_payload, auth=user.auth, expect_errors=True)

assert res.status_code == 400
assert res.json['errors'][0]['detail'] == 'You do not have the ability to edit a conflict of interest while the ' \
'has_coi field is set to false or unanswered'
assert res.status_code == 200
assert res.json['data']['attributes']['conflict_of_interest_statement'] ==\
'Owns shares in Closed Science Corporation.'

preprint.has_coi = True
preprint.save()
Expand Down Expand Up @@ -906,9 +906,8 @@ def test_update_why_no_data(self, app, user, preprint, url):

res = app.patch_json_api(url, update_payload, auth=user.auth, expect_errors=True)

assert res.status_code == 400
assert res.json['errors'][0]['detail'] == 'You cannot edit this statement while your data links availability' \
' is set to true or is unanswered.'
assert res.status_code == 200
assert res.json['data']['attributes']['why_no_data'] == 'My dog ate it.'

preprint.has_data_links = 'no'
preprint.save()
Expand Down Expand Up @@ -937,9 +936,8 @@ def test_update_data_links(self, app, user, preprint, url):
preprint.save()
res = app.patch_json_api(url, update_payload, auth=user.auth, expect_errors=True)

assert res.status_code == 400
assert res.json['errors'][0]['detail'] == 'You cannot edit this statement while your data links availability' \
' is set to false or is unanswered.'
assert res.status_code == 200
assert res.json['data']['attributes']['data_links'] == data_links

preprint.has_data_links = 'available'
preprint.save()
Expand Down Expand Up @@ -1014,6 +1012,10 @@ def test_no_data_links_clears_links(self, app, user, preprint, url):
assert res.json['data']['attributes']['has_data_links'] == 'no'
assert res.json['data']['attributes']['data_links'] == []

preprint.reload()
assert preprint.has_data_links == 'no'
assert preprint.data_links == []

def test_no_prereg_links_clears_links(self, app, user, preprint, url):
preprint.has_prereg_links = 'available'
preprint.prereg_links = ['http://example.com']
Expand All @@ -1029,6 +1031,11 @@ def test_no_prereg_links_clears_links(self, app, user, preprint, url):
assert res.json['data']['attributes']['prereg_links'] == []
assert not res.json['data']['attributes']['prereg_link_info']

preprint.reload()
assert preprint.has_prereg_links == 'no'
assert preprint.prereg_links == []
assert preprint.prereg_link_info is None

def test_update_why_no_prereg(self, app, user, preprint, url):
update_payload = build_preprint_update_payload(preprint._id, attributes={'why_no_prereg': 'My dog ate it.'})

Expand All @@ -1040,9 +1047,8 @@ def test_update_why_no_prereg(self, app, user, preprint, url):

res = app.patch_json_api(url, update_payload, auth=user.auth, expect_errors=True)

assert res.status_code == 400
assert res.json['errors'][0]['detail'] == 'You cannot edit this statement while your prereg links availability' \
' is set to true or is unanswered.'
assert res.status_code == 200
assert res.json['data']['attributes']['why_no_prereg'] == 'My dog ate it.'

preprint.has_prereg_links = False
preprint.save()
Expand Down Expand Up @@ -1072,9 +1078,8 @@ def test_update_prereg_links(self, app, user, preprint, url):
preprint.save()
res = app.patch_json_api(url, update_payload, auth=user.auth, expect_errors=True)

assert res.status_code == 400
assert res.json['errors'][0]['detail'] == 'You cannot edit this field while your prereg links availability' \
' is set to false or is unanswered.'
assert res.status_code == 200
assert res.json['data']['attributes']['prereg_links'] == prereg_links

preprint.has_prereg_links = 'available'
preprint.save()
Expand Down Expand Up @@ -1105,9 +1110,8 @@ def test_update_prereg_link_info(self, app, user, preprint, url):
preprint.save()
res = app.patch_json_api(url, update_payload, auth=user.auth, expect_errors=True)

assert res.status_code == 400
assert res.json['errors'][0]['detail'] == 'You cannot edit this field while your prereg links availability' \
' is set to false or is unanswered.'
assert res.status_code == 200
assert res.json['data']['attributes']['prereg_link_info'] == 'prereg_designs'

preprint.has_prereg_links = 'available'
preprint.save()
Expand Down
50 changes: 26 additions & 24 deletions osf/models/preprint.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,9 @@ def update_has_coi(self, auth: Auth, has_coi: bool, log: bool = True, save: bool
This method brought to you via a grant from the Alfred P Sloan Foundation.
"""
if has_coi is None:
has_coi = False

if self.has_coi == has_coi:
return

Expand Down Expand Up @@ -1021,17 +1024,14 @@ def update_conflict_of_interest_statement(self, auth: Auth, coi_statement: str,
if self.conflict_of_interest_statement == coi_statement:
return

if not self.has_coi:
raise PreprintStateError('You do not have the ability to edit a conflict of interest while the has_coi field is '
'set to false or unanswered')

self.conflict_of_interest_statement = coi_statement
self.conflict_of_interest_statement = coi_statement or ''

if log:
self.add_log(
action=PreprintLog.UPDATE_COI_STATEMENT,
params={
'user': auth.user._id,
'value': self.conflict_of_interest_statement
},
auth=auth,
)
Expand All @@ -1054,6 +1054,9 @@ def update_has_data_links(self, auth: Auth, has_data_links: bool, log: bool = Tr
if self.has_data_links == has_data_links:
return

if has_data_links == 'no':
self.data_links = []

self.has_data_links = has_data_links

if log:
Expand All @@ -1065,7 +1068,7 @@ def update_has_data_links(self, auth: Auth, has_data_links: bool, log: bool = Tr
},
auth=auth
)
if has_data_links != 'available':
if not has_data_links:
self.update_data_links(auth, data_links=[], log=False)
if save:
self.save()
Expand All @@ -1086,9 +1089,8 @@ def update_data_links(self, auth: Auth, data_links: list, log: bool = True, save
if self.data_links == data_links:
return

if not self.has_data_links == 'available' and data_links:
raise PreprintStateError('You cannot edit this statement while your data links availability is set to false'
' or is unanswered.')
if not self.has_data_links and data_links:
self.data_links = []

self.data_links = data_links

Expand Down Expand Up @@ -1119,11 +1121,10 @@ def update_why_no_data(self, auth: Auth, why_no_data: str, log: bool = True, sav
if self.why_no_data == why_no_data:
return

if not self.has_data_links == 'no':
raise PreprintStateError('You cannot edit this statement while your data links availability is set to true or'
' is unanswered.')
else:
self.why_no_data = why_no_data
if self.has_data_links:
self.why_no_data = ''

self.why_no_data = why_no_data

if log:
self.add_log(
Expand Down Expand Up @@ -1152,6 +1153,10 @@ def update_has_prereg_links(self, auth: Auth, has_prereg_links: bool, log: bool
if has_prereg_links == self.has_prereg_links:
return

if not has_prereg_links:
self.prereg_links = []
self.prereg_link_info = None

self.has_prereg_links = has_prereg_links

if log:
Expand All @@ -1163,7 +1168,7 @@ def update_has_prereg_links(self, auth: Auth, has_prereg_links: bool, log: bool
},
auth=auth
)
if has_prereg_links != 'available':
if not has_prereg_links:
self.update_prereg_links(auth, prereg_links=[], log=False)
self.update_prereg_link_info(auth, prereg_link_info=None, log=False)
if save:
Expand All @@ -1185,9 +1190,8 @@ def update_why_no_prereg(self, auth: Auth, why_no_prereg: str, log: bool = True,
if why_no_prereg == self.why_no_prereg:
return

if self.has_prereg_links == 'available' or self.has_prereg_links is None:
raise PreprintStateError('You cannot edit this statement while your prereg links '
'availability is set to true or is unanswered.')
if self.has_prereg_links or self.has_prereg_links is None:
self.why_no_prereg = ''

self.why_no_prereg = why_no_prereg

Expand Down Expand Up @@ -1218,9 +1222,8 @@ def update_prereg_links(self, auth: Auth, prereg_links: list, log: bool = True,
if prereg_links == self.prereg_links:
return

if not self.has_prereg_links == 'available' and prereg_links:
raise PreprintStateError('You cannot edit this field while your prereg links'
' availability is set to false or is unanswered.')
if not self.has_prereg_links and prereg_links:
self.prereg_links = []

self.prereg_links = prereg_links

Expand Down Expand Up @@ -1252,9 +1255,8 @@ def update_prereg_link_info(self, auth: Auth, prereg_link_info: str, log: bool =
if self.prereg_link_info == prereg_link_info:
return

if not self.has_prereg_links == 'available' and prereg_link_info:
raise PreprintStateError('You cannot edit this field while your prereg links'
' availability is set to false or is unanswered.')
if not self.has_prereg_links and prereg_link_info:
self.prereg_link_info = None

self.prereg_link_info = prereg_link_info

Expand Down

0 comments on commit 235ce8f

Please sign in to comment.