search

dwaring87 / rtm-api

Remember the Milk API Interface
MIT License
6 stars 5 forks source link

Version parameter isn't working, is operating as v1 #5

Closed pipedreambomb closed 2 years ago

pipedreambomb commented 5 years ago

I was having an issue trying to get rtm.tasks.setStartDate working, and I was getting the error message Error "[120] Method not valid for requested version. It turned out that it was because RTM-API was using version 1, as the parameter it was passing wasn't correct.

It is set up to pass version=2 as a GET parameter, but the correct parameter is simply v=2 as explained in the API documentation.

I found that I could fix this issue by changing line 177 of get.js from: args.params.version = version; to: args.params.v = version;

If you want, I can put this in a pull request.

pipedreambomb commented 5 years ago

Thinking about it, assuming I'm right about all this, you might not want to implement my fix as is, because it might be a breaking change for all your users whose apps are running on V1. I don't know if there is actually any issue running on V1 vs V2, but presumably RTM made them separate for a reason.

dwaring87 commented 5 years ago

Thanks for pointing this out. I actually though I was using v2 of the API this whole time. I'll have to check if switching to v2 will break anything.

pipedreambomb commented 5 years ago

The safest thing seems like updating it to pass v=1 by default and then letting users set it by configuration, if you get the chance.

EmmaMunter commented 3 years ago

There is one important difference it seems: v1 of the API does not return subtasks, v2 does.

Moving to v2 could impact some specific use cases, but staying on v1 means incompatibility with the official apps, where filters & smart lists do include subtasks. You can easily test this: create a subtask in the RTM web app or mobile (Android) app. Then look at any filter or smart list that includes the subtask but not its parent task, for example isSubtask:true. The subtask will show on the official applications, but not through rtm-api/rtm-cli.

One potential downside is that on v2 of the API, rtm-api includes both subtasks and their parent tasks, whereas the official apps exclude subtasks if the parent task is already included.

Unexpectedly hiding tasks could be quite bad though, so I would suggest just including this fix by default. Simply adding AND isSubtask:false to queries could potentially be used by anyone who depends on the old behavior.