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

kobotoolbox: implement generic name-spaced http methods #966

Merged
merged 4 commits into from
Feb 5, 2025

Conversation

hunterachieng
Copy link
Contributor

@hunterachieng hunterachieng commented Jan 31, 2025

Summary

Implement generic name-spaced http helpers: get, post and put

Fixes #934

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to
know!):

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our
Responsible AI Policy

Review Checklist

Before merging, the reviewer should check the following items:

  • Does the PR do what it claims to do?
  • If this is a new adaptor, added the adaptor on marketing website ?
  • If this PR includes breaking changes, do we need to update any jobs in
    production? Is it safe to release?
  • Are there any unit tests?
  • Is there a changeset associated with this PR? Should there be? Note that
    dev only changes don't need a changeset.
  • Have you ticked a box under AI Usage?

@hunterachieng hunterachieng marked this pull request as ready for review February 3, 2025 07:36
Signed-off-by: Hunter Achieng <[email protected]>
Base automatically changed from feature/695-kobotoolbox-common to epic/kobo February 3, 2025 13:45
packages/kobotoolbox/src/http.js Outdated Show resolved Hide resolved
packages/kobotoolbox/src/http.js Outdated Show resolved Hide resolved
packages/kobotoolbox/src/http.js Show resolved Hide resolved
username: 'test',
password: 'strongpassword',
apiVersion: 'v2',
baseURL: 'https://test.kobotoolbox.org',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, interesting that this is baseURL and not baseUrl. This comment is a reminder to me to maybe change this

packages/kobotoolbox/test/index.js Outdated Show resolved Hide resolved
@@ -130,7 +202,7 @@ describe('getSubmissions', () => {
});
});

describe('getForms', () => {
describe.skip('getForms', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it's Ok to skip these for now, we'll come in and re-write them a bit later. We need to remember to remove nock from this adaptor. Maybe we should do it in this PR?

Copy link
Collaborator

@josephjclark josephjclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me @hunterachieng I'll sleep on the other questions I've raised and decide whether we want to do something now or later. But you can move on form here

@josephjclark josephjclark merged commit 577d009 into epic/kobo Feb 5, 2025
1 check passed
@josephjclark josephjclark deleted the feat/934-kobo-generic-helpers branch February 5, 2025 15:58
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

Successfully merging this pull request may close these issues.

2 participants