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
4 stars 8 forks source link

`commcare` add function #528

Closed aleksa-krolls closed 1 month ago

aleksa-krolls commented 2 months ago

Request

Currently our adaptor does not support extracting case, form, user, group and lookup table data out of CommCare.

See CommCare API docs:

  1. Get cases
  2. Get a single case
  3. Get users
  4. Get lookup table data
  5. Get forms
  6. Get a single form

Plus we should also ensure that all http functions are available in this adaptor, in case users want to send http requests to other api endpoints not listed above.

Examples

To test

See LP CommCare Demo Project for a credential to our test project.

josephjclark commented 1 month ago

Note from mtuchi: we may want to rename "application name" to "domain" to better reflect commcare's own language.

Also app id and name are probably optional, and may not be needed in each request

We should consider fixing both of these

josephjclark commented 1 month ago

Hi @aleksa-krolls,

How important is this ticket? I am thinking about asking Hunter to do it now as she's already working on the commcare adaptor. Make sense to me to pack a few features in while shes in the headspace for it.

After a quick chat with @mtuchi , I would suggest we expose general get and post helper operations, so that users can get at any resources they need.

get('cases', { query: { name: 'Buck Rogers' }})

or

get('/cases?name="Buck Rogers")  // (note to me, we need to ensure the URL is safely escaped, does undici do that for us?)

Then later we can think about adding special getCase, getCases etc operations for the most commonly used ones.

Sound cool?

aleksa-krolls commented 1 month ago

@josephjclark I think extra time spent here to get it right is well spent. There are handful of customers integrating with Commcare, but using http ... so I want to get them using this adaptor. Plus we have a big CommCare integration project kicking off end of month that will need this too :)

josephjclark commented 1 month ago

Great - we'll invest a couple of days into this then and have a shiny new release for you next week. Thanks!

josephjclark commented 1 month ago

@christad92 just a heads up that we'd like Hunter to tackle this issue after she's done the current commcare one

aleksa-krolls commented 1 month ago

cool and fyi @AishaHassen will be able to provide QA and implementation feedback :)

aleksa-krolls commented 1 month ago

@hunterachieng I want to check on the estimate here. I'm seeing 96 hours - is that right that this will take 12 days (12 days * 8 hrs = 96hrs) to implement these functions? That feels really high

cc @josephjclark @mtuchi

josephjclark commented 1 month ago

Hmm. I wouldn't want to give you a number in hours, but I don't think this is a very big job.

We need one generalised helper function for this, which should be super easy to add on top of the existing request we just added.

There are a couple of tidyups that Mutchi suggests, but they're only small.

There will be a review cycle but in terms of focused working hours that should be low. And again the scope for the work is limited so I don't expect this to be too high.

@hunterachieng I'm happy to talk through the exact scoping, but- my feeling is that this is gonna be less than ten hours.

hunterachieng commented 1 month ago

@aleksa-krolls apologies, I was estimating with 24 hours and not 8 hours. I included extra time for reviews as well but I agree with @josephjclark 's assessment. If its not large then I can change the estimate to 8 hours

aleksa-krolls commented 1 month ago

hey @hunterachieng is this what you're working on today? If yes, friendly reminder to move over to the In Progress column so we can monitor progress on getting through the backlog. Thanks :)

hunterachieng commented 1 month ago

@aleksa-krolls I am finalising on a few improvements for commcare then I hit the ground running with this task

josephjclark commented 1 month ago

@aleksa-krolls That's my fault - I've got Hunter doing a couple of extra bits on the other PR before she moves on. Want to make sure we're building on stable ground here!

aleksa-krolls commented 1 month ago

No problem - ty!

Nothing was in In Progress today on the Zenhub board, so I wasn't clear what is being worked on (and there have been new requests coming up - so I'm trying to prioritize Hunter's backlog and see where she's at). @hunterachieng once you get feedback from Code Review and you're working on changes --> please move the issue back to In Progress so we know you're busy working on feedback ... sound good?

hunterachieng commented 1 month ago

@aleksa-krolls Sounds good

josephjclark commented 1 month ago

I've just spun #562 out of this thread to capture the config schema changes. It's only a small change and we have to do it alongside the upcoming release.

hunterachieng commented 1 month ago

@aleksa-krolls I have completed the get, just writing tests and testing for all scenarios before I pass it on for review from @josephjclark

josephjclark commented 1 month ago

@hunterachieng Just one test on get please! getCases will be fine - it just proves out the generic get handler

hunterachieng commented 1 month ago

@josephjclark Yes I am writing one test. I was just testing the scenarios manually first

josephjclark commented 1 month ago

Hey @aleksa-krolls , the generic get() helper is in.

Do we want a post helper too? Or is that for another day?

aleksa-krolls commented 1 month ago

@josephjclark yes, please! But let's close out this issue first and track than on another. For the post, I created this issue #570 - great if you and hunter can put an estimate there to help us understand what's left and what Hunter can take on next week.

josephjclark commented 1 month ago

Closing this issue as this work has been done (we'll handle Aisha's feedback over in that issue)