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

Ghana Adaptors for Wigal SMS, NIA and BDR #938

Open
wants to merge 21 commits into
base: epic/ghana
Choose a base branch
from
Open

Conversation

stevkky
Copy link
Collaborator

@stevkky stevkky commented Jan 24, 2025

Summary

I have added the adaptors for Wigal SMS, NIA and BDR

Details

The Wigal SMS adaptor have been tested against the live API to send messages successfully.
The Birth and Death Registry (BDR) and National Identification Authority (NIA) are yet to be tested.

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?

@mtuchi mtuchi changed the base branch from ghs-adaptor to epic/ghana January 24, 2025 16:47
@mtuchi mtuchi self-requested a review January 27, 2025 07:41
@mtuchi
Copy link
Collaborator

mtuchi commented Jan 27, 2025

Hiya @stevkky I have cleanup the wigal-sms adaptor, Please make sure you git pull before pushing any changes

Here is the list of the improvements i have made and i want you to apply those for the bdr and nia adaptor

  • Add jsdoc for sendSMS. This is the SMSRequestObject [For the data argument] and SendSMSState [For the API response]
  • Removed the option argument. From the API docs there is no any option described. We can add this later when we need it
  • Fixed the bug in util.request() that was causing CI to fail. Basically the jsdocs were incomplete so i have removed em since the function won't be used in job code. If they are important you can add them back but make sure you add @private tag
  • Improved error handling. Basically i have used throwError(code , {description, fix,...other}) function from common. This will make sure the error is properly formatted
  • Update the test to assert the returned response. It will be nice to also assert for 404 and 403 response but let's put a pin on this
  • Update the readme to include the link to API docs
  • Update version to v0.1.0 in package.json and their changelog

Here is what i need you to do for BDR & NIA adaptor

  • Update README.md to include API docs link if any and remove any template information that is not relevant
  • Update jsdocs for the added function in Adaptor.js. You don't have to map each object key just enough details for clarity
  • Remove unused imports and un used console.log if any
  • Double check the configuration-schema.json if it matches the credentials type for each adaptor

* @returns {Operation}
* @state {HttpState}
*/
export function sendBirthNotification(data, options = {}) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@stevkky do you know if the API has options like query params ?

Copy link
Member

Choose a reason for hiding this comment

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

we don't think it does, no. (also no public docs, so we should release this as version 0.1.0 and see how it goes... all bets are off until we hit 1.0)

Copy link
Member

@taylordowns2000 taylordowns2000 left a comment

Choose a reason for hiding this comment

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

happy with all of these as v0.1.0 adaptors. let get them used and tested in the workshop this week and see what changes we want later.

@stevkky stevkky marked this pull request as ready for review January 27, 2025 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: New Issues
Development

Successfully merging this pull request may close these issues.

3 participants