OpenFn / adaptors

The new home for OpenFn adaptors; re-usable connectors for the most common DPGs and DPI building blocks.
GNU General Public License v3.0
7 stars 8 forks source link

Commcare Adaptor Fixes #591

Closed AishaHassen closed 3 months ago

AishaHassen commented 4 months ago

@hunterachieng QA testing comments for #528

  1. Documentations: add more examples on the get function. The current example only shows fetching a list of cases, add an example that shows fetching a single value(you can make this second example on user).
  2. Documentations: List out the possible params either in the docs, e.g(Limit, offset...)
  3. Error Handling: I tried to do get("users") instead of get("user") and got the error message below in the log. A better response would be something that indicates that the requested resource does not exist on Commcare image
  4. Error handling: When getting a user by Id that doesn't exist, I am getting a Job Error. A better response would be that that resource with the specified id does not exisit
  5. running the fetchReportData manages to post data to the desination but fails with this error afterwards: Screenshot 2024-05-30 at 15 27 26
  6. I don't see post function on the docs on the job editor modal on app.openfn.org but i see it on doc.openfn.org
  7. Examples on docs: Nawiri and Registration forms seem to be empty
josephjclark commented 4 months ago

Thanks for the feedback @AishaHassen! This stuff is so valuable.

Some notes/comments from me

  1. looks like the get function docs do actually have two examples, but they are badly formatted. The JSDoc needs fixing!

image

I think Prettier may have formatted this code weirdly

  1. Yes, we should document those parameters. @hunterachieng the best way to do this is a typedef, see RequestOptions in the HTTP adaptor (look at http.get in the docs side and the http jsdoc in the code). All references to the same parameters should use the same typedef.

  2. interesting, the error comes back as a html doc. I suppose this is just a regular get on commcarehq.org or whatever, such as a browser would make. We'll have to add some special handling for this. I expect POST to return JSON but maybe GET needs to check if the response if HTML and, if so, just say "resource not found." It would be nice to include the actual requested url in the error message.

  3. Same as (3) really - let's just handle 404s better on get

  4. @hunterachieng We'll have to reproduce and debug that locally.

  5. @hunterachieng the POST function needs to be marked @public (like get is )

  6. I'm sorry I'm not sure what that means! :blush:

hunterachieng commented 4 months ago

@AishaHassen Thank you for the valuable feedback!

@josephjclark I will look at the issues and fix them accordingly

AishaHassen commented 4 months ago

@josephjclark @hunterachieng Clarification on point 7: The following links (Registration Form Example, Nawiri Example)found under example section in commcare seem to be empty. I've created it along with this issue because I feel it's a bug that's preventing the examples being shown and not lack of content. Please let me know if the examples actually do lack content so I can create a separate issue for these two.

hunterachieng commented 4 months ago

@AishaHassen I will look into all the feedback. Thank you

josephjclark commented 4 months ago

Ah thanks @AishaHassen - I don't even know how the examples are managed. Looks to me like they should just be removed. I'll dig into it and post back here

hunterachieng commented 4 months ago

Fixes: