Skip to content
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

CA-1176 allow children to be deleted without remove_child on parent #503

Merged
merged 3 commits into from
Feb 8, 2021

Conversation

dvoet
Copy link
Collaborator

@dvoet dvoet commented Feb 8, 2021

Ticket: https://broadworkbench.atlassian.net/browse/CA-1176


PR checklist

  • I've followed the instructions if I've made any changes to the API, especially if they're breaking changes
  • I've updated the RC_XXX release ticket with any manual steps required to release this change
  • I've updated the FISMA documentation if I've made any security-related changes, including auth, encryption, or auditing

@@ -411,6 +411,43 @@ class ResourceRoutesV2Spec extends AnyFlatSpec with Matchers with TestSupport wi
}
}

it should "204 deleting a child resource" in {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test was moved from below and amended not to require the remove_child action

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for these comments!

}
}

it should "400 when attempting to delete a resource with children" in {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test was moved from below (it was in the wrong place)

}
}

it should "403 if user is missing remove_child on parent resource if it exists" in {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test was removed

@@ -1395,39 +1432,6 @@ class ResourceRoutesV2Spec extends AnyFlatSpec with Matchers with TestSupport wi
}
}

"DELETE /api/resources/v2/{resourceTypeName}/{resourceId}" should "204 on a child resource if the user has remove_child on the parent resource" in {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved up

@@ -1482,26 +1486,6 @@ class ResourceRoutesV2Spec extends AnyFlatSpec with Matchers with TestSupport wi
}
}

it should "400 when attempting to delete a resource with children" in {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved up

@dvoet dvoet changed the title Delete child CA-1176 allow children to be deleted without remove_child on parent Feb 8, 2021
@coveralls
Copy link

coveralls commented Feb 8, 2021

Coverage Status

Coverage increased (+0.2%) to 78.842% when pulling 8043bf4 on delete_child into 87df68e on develop.

Copy link
Contributor

@andy7i andy7i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 comment then 👍

requireParentAction(resource, None, SamResourceActions.removeChild, userInfo.userId, samRequestContext) {
complete(resourceService.deleteResource(resource, samRequestContext).map(_ => StatusCodes.NoContent))
}
complete(resourceService.deleteResource(resource, samRequestContext).map(_ => StatusCodes.NoContent))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be helpful to leave a comment about why we are intentionally not requiring removeChild here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

@@ -411,6 +411,43 @@ class ResourceRoutesV2Spec extends AnyFlatSpec with Matchers with TestSupport wi
}
}

it should "204 deleting a child resource" in {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for these comments!

@dvoet
Copy link
Collaborator Author

dvoet commented Feb 8, 2021

jenkins retest

@dvoet dvoet merged commit dcb248f into develop Feb 8, 2021
@dvoet dvoet deleted the delete_child branch February 8, 2021 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants