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

Prevent multiple source fields even if list is not leaf field #503

Merged
merged 2 commits into from
Dec 21, 2023

Conversation

gnawf
Copy link
Collaborator

@gnawf gnawf commented Dec 19, 2023

No description provided.

@gnawf gnawf marked this pull request as draft December 19, 2023 23:02
@@ -170,6 +170,97 @@ class NadelHydrationValidationTest2 {
assertTrue(errors.single().subject.name == "data")
}

@Test
fun `prohibit multiple source fields where list not the leaf`() {
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 should actually be an allowed feature, but it's not supported right now.

@gnawf gnawf force-pushed the fix-source-field-validation branch from e1d70e0 to abc7594 Compare December 19, 2023 23:12
@gnawf gnawf changed the base branch from refactor-remote-arg-source to master December 19, 2023 23:12
@gnawf gnawf marked this pull request as ready for review December 19, 2023 23:17
parentType.getFieldAt(path)?.type?.unwrapNonNull()?.isList == true
.mapNotNull { it.remoteArgumentSource.pathToField }
}
.toList()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Pulled the paths out to a list

parentType
.getFieldsAlong(pathToField)
.any { field ->
field.type.unwrapNonNull().isList
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 is the actual change.

e.g. for $source.contexts.userId where contexts is a list but userId is not a list

Previously we only checked the end field to see if it was a list, this checks everything.

@gnawf gnawf requested a review from sbarker2 December 20, 2023 00:56
@gnawf gnawf merged commit 9e58b81 into master Dec 21, 2023
1 check passed
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.

2 participants