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

Fix GraphQL Nested UPDATE bug. #974

Merged
merged 7 commits into from
Sep 19, 2019
Merged

Conversation

aklish
Copy link
Member

@aklish aklish commented Sep 19, 2019

Description

GraphQL during an UPDATE or UPSERT of a nested entity will attempt to add the nested entity to its parent relationship. This is unnecessary for UPDATE (and also UPSERT where the object already exists).

Normally, this would be harmless, but SharePermission barfs.

Motivation and Context

Explained above.

How Has This Been Tested?

All existing GraphQL tests pass.

Manually tested the fix in another project. Note - SharePermission is being removed in Elide 5.0 - so this bug would never have manifested.

License

I confirm that this contribution is made under an Apache 2.0 license and that I have the authority necessary to make this contribution on behalf of its copyright owner.

@aklish aklish changed the title Fix bug in GraphQL where we add the parent relationship during an UPD… Fix GraphQL Nested UPDATE bug. Sep 19, 2019
@aklish aklish requested review from clayreimann and jkusa September 19, 2019 01:32
@coveralls
Copy link
Collaborator

coveralls commented Sep 19, 2019

Coverage Status

Coverage decreased (-0.04%) to 76.284% when pulling 078d28a on FixSharePermissionBugInGraphQLUpdate into e2de2a8 on master.

@wcekan
Copy link
Contributor

wcekan commented Sep 19, 2019

Can we use the new function here?
https://github.com/yahoo/elide/blob/FixSharePermissionBugInGraphQLUpdate/elide-core/src/main/java/com/yahoo/elide/security/executors/ActivePermissionExecutor.java#L129

            if (requestScope.getNewPersistentResources().contains(resource))

to

            if (resource.isNewlyCreated())

@aklish aklish merged commit 7b7aba5 into master Sep 19, 2019
@aklish aklish deleted the FixSharePermissionBugInGraphQLUpdate branch September 19, 2019 20:29
@wcekan wcekan mentioned this pull request Sep 20, 2019
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.

3 participants