Skip to content

Conversation

@leonardocustodio
Copy link
Contributor

No description provided.

@leonardocustodio leonardocustodio added the bug Something isn't working label Sep 19, 2023
@leonardocustodio leonardocustodio self-assigned this Sep 19, 2023
{
writer.WritePropertyName(variable.Key);
writer.WriteRawValue(JsonSerializer.Serialize(variable.Value, options));
if (variable.Value != null && variable.Value.GetType().IsGenericType && variable.Value.GetType().GetGenericTypeDefinition() == typeof(List<>))
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be great if there could be test cases for various scenarios here. Looks like maybe this unit test is meant to cover this functionality and could be expanded?

https://github.com/enjin/platform-csharp-sdk/blob/master/src/Enjin.Platform.Sdk/Enjin.Platform.Sdk.Tests/Unit/Json/GraphQlParameterJsonConverterTest.cs

writer.WriteStartArray();
writer.WriteStartObject();

foreach (KeyValuePair<string, object?> parameter in graphQlParameter.Parameters)
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably doesn't work if there are nested arrays. I don't know if that ever happens in Enjin schemas though so maybe that is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, I will make a recursive function though I wonder if there isn't another way, I mean this whole function is called recursively already and it goes inside each the objects and the objects of ojects and decodes them fine. Only for arrays it seems to fail, so I wonder if this can't be fixed in another way, I will give another thought tomorrow.

@leonardocustodio
Copy link
Contributor Author

Superseeded by #10

@Bradez Bradez deleted the fix-list-dict branch September 20, 2023 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Development

Successfully merging this pull request may close these issues.

3 participants