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

Searching task with custom fields paramaters. #100

Open
randoum opened this issue Jan 16, 2021 · 3 comments
Open

Searching task with custom fields paramaters. #100

randoum opened this issue Jan 16, 2021 · 3 comments

Comments

@randoum
Copy link

randoum commented Jan 16, 2021

Hey,

TasksBase#search_tasks_for_workspace does not allow to filter results based on custom fields.

Though, the documentation indicates it's a possibility here: https://developers.asana.com/docs/search-tasks-in-a-workspace chapter "Custom field parameters".

In other words, I would like to be able to write:

result = client.tasks.search_tasks_for_workspace workspace_gid: 'wrk123', teams_any: 'team123,team456', 'custom_fields.123.value': '456'

The end-point accepts this syntax, so why the API is not allowing custom_fields... parameters?

I could write and propose a PR, but I need to know if such a feature would be accepted.

Thanks

@rossgrambo-zz
Copy link
Contributor

rossgrambo-zz commented Jan 21, 2021

If you want to propose a PR to resolve this, that would be great! A short term solution would be adding **kwargs on this method and pushing it through. If we take that approach, I think we should add the optional **kwargs on all functions (as there's unlisted fields / misc. params that aren't captured in the method definition).

But if you can simply propose a working version for the search endpoint, I'll deal with adding it to the rest.

@randoum
Copy link
Author

randoum commented Feb 12, 2021

Hey @rossgrambo,

Yes, I could do that, but then while I keep working with Asana API, I noticed other inconsistencies, such as missing and misspelt attribute in other client's methods. I was too concentrated to take a chance to write down which method that was.

But that brings me to a more profound point: why validating params at all? The end-point is capable of returning error messages to notify on incorrect params, so a developer is guided to get it right anyhow. I feel like it's a waste of energy to implement verification of the client's params, a source of errors when the params are misspelt, and may introduce missing feature when params are not implemented (such as the reason I opened this ticket).

Perhaps this should be reassessed, and I would like to inspire your team to think about giving-up field validation at the clients level and let the end-point take care of it. This won't overload the end-point, because errors happen once, we get the error message, and we fix it.

@rossgrambo-zz
Copy link
Contributor

rossgrambo-zz commented Feb 12, 2021

I would like to inspire your team to think about giving-up field validation at the clients level and let the end-point take care of it.

Devs are always free to simply hit the API endpoint without use of this client library. This client library exists to help devs quickly understand our API and empower them to build entirely from their IDE. Params offer the advantage of autocomplete/IntelliSense displaying what's possible. If we simply removed all params and changed them to kwargs, then there is little advantage to using the client library over simply making the HTTP calls.

why validating params at all?

The only validation we do is setting certain params to required. Again, I think this provides value via the IDE of the person building. Rather than waiting for them to test, see it failed, and then make the update.

I feel like it's a waste of energy to implement verification of the client's params, a source of errors when the params are misspelt, and may introduce missing feature when params are not implemented

This Client Library is generated from an OpenAPI spec of our API. It takes very little effort to manage things like required params, params in general, new endpoints, etc. Your original issue is valid, because the auto-generation doesn't provide a dynamic way to add params (which search clearly needs). Altering the way things are genned is the way this should actually be resolved. I was planning to alter the gen to match the PR you created.

This OpenAPI Spec is the same spec that generates our docs. Meaning anything that's newly documented is automatically available in these libraries (assuming you're on the latest version of the library).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants