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

Support optional relations #204

Merged
merged 4 commits into from
Feb 7, 2024
Merged

Conversation

PaulLeCam
Copy link
Contributor

Following support for optional relations in Ceramic, this PR allows setting optional fields to null in the GraphQL updates and apply logic to remove these fields for the JSON patch.

Opening as draft as the Ceramic dependencies will need to change from RC to stable versions, but should be OK to review already.

@PaulLeCam PaulLeCam marked this pull request as ready for review February 7, 2024 10:57
Copy link
Contributor

@JulissaDantes JulissaDantes left a comment

Choose a reason for hiding this comment

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

LGTM! Do you think that following this PR I should include a test case in #205 where an immutable field its updated to a null value?

@PaulLeCam
Copy link
Contributor Author

LGTM! Do you think that following this PR I should include a test case in #205 where an immutable field its updated to a null value?

Thanks!
I think if we implement the change that the update mutation object doesn't include immutable fields, it shouldn't even possible to set the fields to null using the ComposeDB APIs, would it?

@PaulLeCam PaulLeCam merged commit 2097b0c into main Feb 7, 2024
5 checks passed
@PaulLeCam PaulLeCam deleted the fix/WS2-2819-optional-relations branch February 7, 2024 19:47
@JulissaDantes
Copy link
Contributor

LGTM! Do you think that following this PR I should include a test case in #205 where an immutable field its updated to a null value?

Thanks! I think if we implement the change that the update mutation object doesn't include immutable fields, it shouldn't even possible to set the fields to null using the ComposeDB APIs, would it?

No, you are right, just checking in case I was missing something

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.

4 participants