LastDragon-ru / lara-asp

Awesome Set of Packages for Laravel
MIT License
11 stars 1 forks source link

`@rename` doesn't work as expected: no filters #88

Closed LastDragon-ru closed 1 year ago

LastDragon-ru commented 1 year ago

I am using 4.5.0 and actually just upgraded. However, I think I found where the value is lost:

image

The input is still country_code but the type's property is countryCode and hence the arguments are empty and the filter is not applied:

image

Originally posted by @jaulz in https://github.com/LastDragon-ru/lara-asp/issues/87#issuecomment-1656837905

LastDragon-ru commented 1 year ago

The problem is the $value passed into ArgBuilderDirective::handleBuilder() contains renamed properties, not the original. Thus, we cannot create the valid Argument.

The second problem: the tests are ok.

jaulz commented 1 year ago

Yeah, I was also wondering why the tests were running fine but I could not trace it down yet. While running the tests, $value actually contains the unrenamed keys.

jaulz commented 1 year ago

Why do we need the getArgumentSet at all? 🤔 Wouldn't it just convert the property names back and forth?

LastDragon-ru commented 1 year ago

Why do we need the getArgumentSet at all?

To work directly with directives which also allows to add operators inside the schema, it is also allow create complex queries like #11.

Seems I have an idea... I will try to create PR for lighthouse within a few days. If it will be accepted the Argument will be passed directly into ArgBuilderDirective that should resolve the issue.

jaulz commented 1 year ago

Great, sounds good. Thanks for the library and your efforts! ✌️

LastDragon-ru commented 1 year ago

Should be fixed in v4.5.1, please try

jaulz commented 1 year ago

Wow, nice, that worked as far as I can see 😊 Thanks a lot!