Jeff schneider add powercomman sdkv3 #62
Conversation
| const tasks = await taskProvider.getTaskRefs(tasksApiInstance, projectId); | ||
|
|
||
| for(let task of tasks.data) { | ||
| if (filters.some(filter => !filter.matchesTask(task))) return; |
There was a problem hiding this comment.
This should probably be a continue rather than return it doesn't make sense to return in this for loop. You want to continue the loop if the task does not match the filter.
The return makes sense in the previous code since it's a tasks.stream().on(...) takes in a function https://github.com/Asana/devrel-examples/blob/master/javascript/powercommands/asana_power.js#L69-L73
There was a problem hiding this comment.
If you try to run asana-power complete --assignee "Kenan Kigunda" this won't work because the tasks being returned is missing the assignee.name. even though you requested for assignee in opt_fields in your task_provider.jshttps://github.com/Asana/devrel-examples/pull/62/files#diff-c55db41f9d5a798d2282e92481cd6de3642c3ebb4d25c7bb1d48de8ae51315a7R4
The Asana API does not return the full assignee object so it'll error when it tries to filter. Pretty sure this was broken in the v1 as well.
There was a problem hiding this comment.
Added the extra check. It could be worth coming back in the future if we get meaningful usage to add another request to fetch each task with getTask to get the assignee name. Could also maybe do it with assignee gid.
| key: 'assignee', | ||
| value: null, | ||
| matchesTask: function(task) { | ||
| return task.assignee && task.assignee.name.includes(this.value); |
There was a problem hiding this comment.
You might want to add another check in case the task has an assignee but no assignee.name. This was probably broken before as well since client.projects.tasks(projectId); does not return the full task object with assignee
return task.assignee && task.assignee.name.includes(this.value); ->
return task.assignee && task.assignee.name && task.assignee.name.includes(this.value);
| const getTaskRefs = async (tasksApiInstance, projectId) => { | ||
| console.log('Retrieving all task references for project', projectId); | ||
| let opts = { | ||
| opt_fields: 'name,assignee,completed,resource_type,resource_subtype' |
There was a problem hiding this comment.
The assignee filter and notes filters won't work unless you ask for those properties in opt_fields
FROM:
opt_fields: 'name,assignee,completed,resource_type,resource_subtype'
TO:
opt_fields: 'name,assignee.name,completed,resource_type,resource_subtype,notes'
There was a problem hiding this comment.
updated the opt_fields to: 'name,assignee.name,completed,resource_type,resource_subtype,notes'
|
NIT: To be consistent with |
Add assignee and notes to opt_fields so those filters work.
|
I'll update the folder name (i.e. add an s to powercommand) on a subsequent commit once these changes are merged. |
why not send a commit for it now? |
My mac is in a weird state with github and mac_configure doesn't work to fix it. I'm underwater with what came in while I was out so can't dedicate the time at the moment to try and fix it. This means I can't checkout the pull request to make the name change. It's an easy fix to make in the xcode web editor once the changes are merged. I just tried to change it in the github website by changing the path of the license. This kind of worked but will take more manual work now to rename all of the paths for all of the files and create a commit for each of them. |
Ok, for now let's remove the extra |
|
Can you, please add a |
|
Not sure how important this is but we're also missing a |
No description provided.