Update registration methods to use V2 API #30
Conversation
Codecov Report
@@ Coverage Diff @@
## master #30 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 5 5
Lines 393 408 +15
Branches 51 51
=========================================
+ Hits 393 408 +15
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
src/index.js
Outdated
| const headers = {} | ||
| const requestOptions = this.__createRequest('POST', headers, JSON.stringify(body)) | ||
| const url = this.__getUrl(`/events/organizations/${consumerOrgId}/integrations/${integrationId}/registrations`) | ||
| const url = this.__getUrl(`/events/${consumerOrgId}/${projectId}/${workspaceId}/registrations`) | ||
| const sdkDetails = { requestOptions: requestOptions, url: url } | ||
| return this.__handleRequest(url, requestOptions, sdkDetails, codes.ERROR_CREATE_REGISTRATION) |
There was a problem hiding this comment.
why we pass the url and requestOptions to this method, these are already contained in the sdkDetails ?
There was a problem hiding this comment.
i fixed it. thanks!
src/SDKErrors.js
Outdated
| E('ERROR_GET_REGISTRATION', '%s') | ||
| E('ERROR_GET_ALL_REGISTRATION', '%s') | ||
| E('ERROR_GET_ALL_REGISTRATION_FOR_ORG', '%s') |
There was a problem hiding this comment.
nitpick: change 'ERROR_GET_ALL_REGISTRATION_FOR_ORG' to 'ERROR_GET_ALL_REGISTRATIONS_FOR_ORG .
Same for 'ERROR_GET_ALL_REGISTRATION' above
|
@sangeetha5491 changes look good. Just one concern, these look like breaking changes to aio-lib-events as methods are getting renamed etc. |
|
@sandeep-paliwal yes these are breaking changes and will impact its use in the CLI. |
| * [.createWebhookRegistration(consumerOrgId, integrationId, body)](#EventsCoreAPI+createWebhookRegistration) ⇒ <code>Promise.<object></code> | ||
| * [.getWebhookRegistration(consumerOrgId, integrationId, registrationId)](#EventsCoreAPI+getWebhookRegistration) ⇒ <code>Promise.<object></code> | ||
| * [.getAllWebhookRegistrations(consumerOrgId, integrationId)](#EventsCoreAPI+getAllWebhookRegistrations) ⇒ <code>Promise.<object></code> | ||
| * [.deleteWebhookRegistration(consumerOrgId, integrationId, registrationId)](#EventsCoreAPI+deleteWebhookRegistration) ⇒ <code>Promise.<object></code> |
There was a problem hiding this comment.
I am wondering since these are breaking changes, shouldn't we mark these as deprecated first and only then remove them completely.
There was a problem hiding this comment.
if they want to use the older methods, they can use the previous version. WDYT? @sandeep-paliwal is that the right way to go?
There was a problem hiding this comment.
I disagree.
A version update should not be reason enough for clients to update their integrations.
Even with the digital signature verification feature, we first added support (on 20th Jan), and after the complete deprecation of the old signature verification, we removed the HMAC signature verification from SDK (on 16th July).
So clients atleast had a 6 month window for updating their integrations.
There was a problem hiding this comment.
I would favor removing the v1 and updating the version number accordingly and putting a warning in the release note
Fix delivery type options Registration create and update models in the Readme file.
Description
Uses the registration V2 API to create , delete, get a registration.
Adds additional features:
References: https://developer.adobe.com/events/docs/api/#tag/Registrations
Motivation and Context
All APIs were using the V2 model of projects and workspaces except for registrations. This PR migrates the registrations to use the V2 version.
How Has This Been Tested?
E2E and unit tests
Screenshots (if appropriate):
Types of changes
Checklist: