mirror of
https://github.com/GSA/notifications-api.git
synced 2025-12-15 17:52:26 -05:00
Add a check that the folder is not moved into itself.
By the way, the database will not let this happen, but this is a nicer error and is explicit to read as an invalid move.
This commit is contained in:
@@ -116,13 +116,7 @@ def move_to_template_folder(service_id, target_template_folder_id=None):
|
|||||||
service_id
|
service_id
|
||||||
)
|
)
|
||||||
raise InvalidRequest(msg, status_code=400)
|
raise InvalidRequest(msg, status_code=400)
|
||||||
|
_validate_folder_move(target_template_folder, target_template_folder_id, template_folder, template_folder_id)
|
||||||
if target_template_folder and template_folder.is_parent_of(target_template_folder):
|
|
||||||
msg = 'Could not move to folder: {} is an ancestor of target folder {}'.format(
|
|
||||||
template_folder_id,
|
|
||||||
target_template_folder_id
|
|
||||||
)
|
|
||||||
raise InvalidRequest(msg, status_code=400)
|
|
||||||
|
|
||||||
template_folder.parent = target_template_folder
|
template_folder.parent = target_template_folder
|
||||||
|
|
||||||
@@ -137,9 +131,23 @@ def move_to_template_folder(service_id, target_template_folder_id=None):
|
|||||||
raise InvalidRequest(msg, status_code=400)
|
raise InvalidRequest(msg, status_code=400)
|
||||||
|
|
||||||
if template.archived:
|
if template.archived:
|
||||||
current_app.logger.error('Could not move to folder: Template {} is archived. (Skipping)'.format(
|
current_app.logger.info('Could not move to folder: Template {} is archived. (Skipping)'.format(
|
||||||
template_id
|
template_id
|
||||||
))
|
))
|
||||||
else:
|
else:
|
||||||
template.folder = target_template_folder
|
template.folder = target_template_folder
|
||||||
return '', 204
|
return '', 204
|
||||||
|
|
||||||
|
|
||||||
|
def _validate_folder_move(target_template_folder, target_template_folder_id, template_folder, template_folder_id):
|
||||||
|
if str(target_template_folder_id) == str(template_folder_id):
|
||||||
|
msg = 'Could not move to folder: {} to itself'.format(
|
||||||
|
template_folder_id
|
||||||
|
)
|
||||||
|
raise InvalidRequest(msg, status_code=400)
|
||||||
|
if target_template_folder and template_folder.is_parent_of(target_template_folder):
|
||||||
|
msg = 'Could not move to folder: {} is an ancestor of target folder {}'.format(
|
||||||
|
template_folder_id,
|
||||||
|
target_template_folder_id
|
||||||
|
)
|
||||||
|
raise InvalidRequest(msg, status_code=400)
|
||||||
|
|||||||
@@ -330,6 +330,22 @@ def test_move_to_folder_rejects_if_it_would_cause_folder_loop(admin_request, sam
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_move_to_folder_itself_is_rejected(admin_request, sample_service):
|
||||||
|
target_folder = create_template_folder(sample_service, name='target')
|
||||||
|
|
||||||
|
response = admin_request.post(
|
||||||
|
'template_folder.move_to_template_folder',
|
||||||
|
service_id=sample_service.id,
|
||||||
|
target_template_folder_id=target_folder.id,
|
||||||
|
_data={
|
||||||
|
'templates': [],
|
||||||
|
'folders': [str(target_folder.id)]
|
||||||
|
},
|
||||||
|
_expected_status=400
|
||||||
|
)
|
||||||
|
response['message'] == 'Could not move to folder: {} to itself'.format(target_folder.id)
|
||||||
|
|
||||||
|
|
||||||
def test_move_to_folder_skips_archived_templates(admin_request, sample_service):
|
def test_move_to_folder_skips_archived_templates(admin_request, sample_service):
|
||||||
target_folder = create_template_folder(sample_service)
|
target_folder = create_template_folder(sample_service)
|
||||||
other_folder = create_template_folder(sample_service)
|
other_folder = create_template_folder(sample_service)
|
||||||
|
|||||||
Reference in New Issue
Block a user