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 property_NAME in MediaQuery #78

Open
Reonekot opened this issue Apr 26, 2022 · 0 comments
Open

Support property_NAME in MediaQuery #78

Reonekot opened this issue Apr 26, 2022 · 0 comments

Comments

@Reonekot
Copy link

As far as I can tell the SDK is missing the option filter by property_NAME, as per the API documentation. As we need that I added this to my already existing copy of the SDK (waiting on #74 to move back to the official SDK). I am considering doing a PR on it, but I'm unsure how you prefer to add this as it is a bit of an odd API IMO. So I'm running with what currently works locally and thought it better to open an issue for discussion first before finishing a PR on it.

The issue is that the Key is not static so the current ApiField attribute doesn't really fit that well. What I ended up doing is adding a new ITypeToPropertyConverter (and backing converter implementation) and special handling this in QueryDecoder.ConvertProperty(). In ConvertProperty() I pass the parameters dictionary to the converted, so the Converter is responsible for adding the parameters instead of this happening in QueryDecoder.AddParam(), so I can control the name added to the dictionary.

It does feel a bit "out of place" though IMO and I also have a bit of a hard time coming up with a proper abstraction, so right now it is only serving that one use case. It also doesn't align with methods on ITypeToStringConverter/ITypeToDictionaryConverter which is shame IMO. One solution to that would ofc. be to change those types. I basically has a method call AddParameters(), which is called instead of Convert(), like so:

else if (Activator.CreateInstance(apiField.Converter) is ITypeToPropertyConverter namePropertyConverter
    && namePropertyConverter.CanConvert(propertyInfo.PropertyType))
{
    namePropertyConverter.AddParameters(parameters, apiField.ApiName, value);
}

Thoughts on how to add this API?

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

No branches or pull requests

1 participant