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

commcare add function #528

Closed
Tracked by #532
aleksa-krolls opened this issue Apr 25, 2024 · 21 comments
Closed
Tracked by #532

commcare add function #528

aleksa-krolls opened this issue Apr 25, 2024 · 21 comments
Assignees

Comments

@aleksa-krolls
Copy link
Member

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
Copy link
Collaborator

josephjclark commented May 10, 2024

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
Copy link
Collaborator

josephjclark commented May 10, 2024

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
Copy link
Member Author

@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
Copy link
Collaborator

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

@josephjclark
Copy link
Collaborator

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

@aleksa-krolls
Copy link
Member Author

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

@aleksa-krolls
Copy link
Member Author

@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
Copy link
Collaborator

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
Copy link
Contributor

@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
Copy link
Member Author

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
Copy link
Contributor

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

@josephjclark
Copy link
Collaborator

@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
Copy link
Member Author

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
Copy link
Contributor

@aleksa-krolls Sounds good

@josephjclark
Copy link
Collaborator

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.

@josephjclark josephjclark mentioned this issue May 16, 2024
5 tasks
@hunterachieng
Copy link
Contributor

@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
Copy link
Collaborator

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

@hunterachieng
Copy link
Contributor

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

@josephjclark
Copy link
Collaborator

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

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

@aleksa-krolls
Copy link
Member Author

aleksa-krolls commented May 18, 2024

@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
Copy link
Collaborator

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

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

No branches or pull requests

3 participants