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

Optional opts parameter is not defined as optional #21

Closed
emadgit opened this issue Jan 4, 2021 · 3 comments · Fixed by #22
Closed

Optional opts parameter is not defined as optional #21

emadgit opened this issue Jan 4, 2021 · 3 comments · Fixed by #22

Comments

@emadgit
Copy link

emadgit commented Jan 4, 2021

Hey guys, happy new year 🥳

I noticed in your latest version for your JavaScript SDK you start adding the Typescript definitions, which is awesome 🎉 as we also using Typescript in our side. I find something which you might want to take a look as it might be a type issue.

As you can see here ( and other places where opts is defined ) it meant to be an optional parameter ( Based on the comments you have there ), but it's not defined as optional. I think you probably need to use ? like opts? to make it optional, otherwise it will be a mandatory parameter.

@fmoeckel
Copy link

fmoeckel commented Jan 4, 2021

Hello @emadgit.

Thank you very much for your suggestion. We are currently checking how much effort will be needed in order to change it accordingly. How critical is this for you?

@emadgit
Copy link
Author

emadgit commented Jan 4, 2021

Hey @fmoeckel,

Thanks for the response, currently we are live with an older sdk version which have no type definitions and we typed it in our side for the most parts, so the issue is not hitting us at the moment, but if we going to upgrade to the latest version we will face to some issues, which I hope you can resolve it by then :)

@altJake
Copy link
Member

altJake commented Feb 2, 2021

Hey @emadgit,

Sorry it took us so long to make ends meet. It was harder than one might assume to be honest, and it's still not perfect.

Thank you very much for using the SDK and reporting the issue!

#22 should introduce most type functionality around optional parameters, and for the rest we are already working on a patch for the OpenAPI Generator.

Stay tuned to the PR, the version is in final testing stages and we will probably release v4.3.0 by the end of this week 🎉

altJake added a commit that referenced this issue Feb 3, 2021
…rrals counters and Export Endpoints (#22)

<i id="toc"></i>
## Summary
  - [Management API](#user-content-management-api)
    - [Introduce `createCouponsForMultipleRecipients` Endpoint](#user-content-introduce-createcouponsformultiplerecipients)
    - [Expose Export Endpoints](#user-content-expose-export-endpoints)
    - [Expose `destroySession` Endpoint](#user-content-expose-destroysession)
    - [Introduce loyalty effects related and referrals creation counters](#user-content-introduce-counters)
  - [Integration API](#user-content-integration-api)
    - [Improve Responses Transparency](#user-content-improve-responses)
    - [Attach Loyalty Program ID in responses](#user-content-attach-loyalty-id)
    - [A reminder of The Deprecation Notice: Integration API@v1 endpoints](#user-content-deprecation-reminder)
  - [Improve Typescript definitions for optional request parameters](#user-content-improve-ts)
  - [Misc: OpenAPI Generator version upgrade](#user-content-misc)

<i id="management-api"></i>
## Management API

<i id="introduce-createCouponsForMultipleRecipients"></i>
### Introduce [`createCouponsForMultipleRecipients`](https://developers.talon.one/Management-API/API-Reference#createCouponsForMultipleRecipients) Endpoint

An endpoint to allow creation of multiple coupons of the same configuration for up to 1,000 recipients at once.

[☝️ Back to Table of Contents](#user-content-toc)

<i id="expose-export-endpoints"></i>
### Expose export endpoints as integral part of the SDK

All of our CSV export endpoints are accessible via the Web Application from the corresponding entity pages (refer to our [Help Center](https://help.talon.one/hc/en-us/articles/360010114599-Import-and-Export-Coupons#ExportExistingCodes) for an example regarding Coupons).

Now these are also available endpoints as part of the SDK (links to our developer docs):
 - [Coupons Export](https://developers.talon.one/Management-API/API-Reference#exportCoupons)
 - [Customer Sessions Export](https://developers.talon.one/Management-API/API-Reference#exportCustomerSessions)
 - [Effects Export](https://developers.talon.one/Management-API/API-Reference#exportEffects)
 - [Customer Loyalty Balance Export](https://developers.talon.one/Management-API/API-Reference#exportLoyaltyBalance)
 - [Customer Loyalty Ledgers Log Export](https://developers.talon.one/Management-API/API-Reference#exportLoyaltyLedger)

Example code snippet demonstrating consuming and printing the lines of a _Customer Loyalty Balance Export_ using the [`csv-parse` package](https://github.com/adaltas/node-csv-parse):
```js
const TalonOne = require('talon_one')
const csvParse = require('csv-parse')

// ...preparing api client...
// An example could be seen at the repository's README file: https://github.com/talon-one/talon_one.js#management-api

managerApi.exportLoyaltyBalance('1')
  .then((data) => {
    return new Promise((resolve, reject) => {
      csvParse(data, {
        columns: true,
        skipEmptyLines: true,
      }, (err, output) => {
        if (err) {
          return reject(err)
        }

        resolve(output)
      })
    })
  })
  .then((records) => {
    // process data using parsed records...
    records.forEach(record => {
      console.log(record) /* outputs:
      { 
        loyaltyProgramID: '1',
        loyaltySubledger: '',
        profileIntegrationID: 'customer_1',
        currentBalance: '34.89',
        pendingBalance: '42.1',
        expiredBalance: '0',
        spentBalance: '10'
      }
      */
    })
  })
  .catch(err => console.log(`Error while exporting and processing csv:\n${JSON.stringify(err, null, 2)}`))
```

[☝️ Back to Table of Contents](#user-content-toc)

<i id="expose-destroySession"></i>
### Expose [`destroySession`](https://developers.talon.one/Management-API/API-Reference#destroySession) Endpoint

Expose an existing endpoint to allow destroying a bearer token used in the context of the management-api.
This endpoint imitates a "logout" function and will make the attached token invalid for consequent requests.

[☝️ Back to Table of Contents](#user-content-toc)

<i id="introduce-counters"></i>
### Introduce loyalty effects related and referrals creation counters on Campaign entities

As part of the newly added budgets to campaigns (see relevant [Help Center Section](https://help.talon.one/hc/en-us/articles/360010114779-Campaign-Budget#LoyaltyLimits)), we have added new counters on campaigns with regard to loyalty and referrals:

 - `createdLoyaltyPointsCount` : Total number of loyalty points created by rules in this campaign
 - `createdLoyaltyPointsEffectCount` : Total number of loyalty point creation effects triggered by rules in this campaign
 - `redeemedLoyaltyPointsCount` : Total number of loyalty points redeemed by rules in this campaign
 - `redeemedLoyaltyPointsEffectCount` : Total number of loyalty point redemption effects triggered by rules in this campaign
 - `referralCreationCount` : Total number of referrals created by rules in this campaign

[☝️ Back to Table of Contents](#user-content-toc)

<i id="integration-api"></i>
## Integration API

<i id="improve-responses"></i>
### Improve Responses Transparency
We are constantly extending and improving our integration API to provide our consumers with the best transparency regarding what exactly has happened within their requests.

We have added new data points to our **v2 endpoints** effects in order to improve the transparency we aspire for:
 - If an effect was triggered because of a specific coupon the effect will now include this coupon ID, see [`Effect.md`](https://github.com/talon-one/talon_one.js/blob/1bb8eb8270f2f281f2a262e31f7484ff4801f920/docs/Effect.md#L12)
 - When a coupon is rejected we attach more details regarding the origin of the failure in [`RejectCouponEffectProps`](https://github.com/talon-one/talon_one.js/blob/1bb8eb8270f2f281f2a262e31f7484ff4801f920/docs/RejectCouponEffectProps.md#L9-L11):
    - `ConditionIndex` - The index of the condition that caused the rejection of the coupon
    - `EffectIndex` - The index of the effect that caused the rejection of the coupon
    - `Details` - More details about the failure (if available)
  - The same applies for referrals, when a referral is rejected we attach more details regarding the origin of the failure in  [`RejectReferralEffectProps`](https://github.com/talon-one/talon_one.js/blob/1bb8eb8270f2f281f2a262e31f7484ff4801f920/docs/RejectReferralEffectProps.md#L9-L11):
    - `ConditionIndex` - The index of the condition that caused the rejection of the referral
    - `EffectIndex` - The index of the effect that caused the rejection of the referral
    - `Details` - More details about the failure (if available)

Moreover, we have introduced a new [response content](https://github.com/talon-one/talon_one.js/blob/1bb8eb8270f2f281f2a262e31f7484ff4801f920/src/model/IntegrationRequest.js#L87-L136), `ruleFailureReasons`, which when requested will attach to the response a collection containing **all failed rules**, with details (see the [`ruleFailureReason` model](https://github.com/talon-one/talon_one.js/blob/1bb8eb8270f2f281f2a262e31f7484ff4801f920/docs/RuleFailureReason.md)  to help narrowing down failures and further debugging efforts to a specific single condition or effect that caused the failure. 

_One "gotcha" to keep in mind_: in order to maximize transparency, and due to the fact that we do not know in advance which campaign in the application the request targets, the list contains a collection of **all** failure reasons.

Meaning that, it might have "white noise" with data about failures that could be considered as "obvious" to the consumer. Therefore, we suggest always filtering the list by the campaign id that was expected to trigger and did not.

[☝️ Back to Table of Contents](#user-content-toc)

<i id="attach-loyalty-id"></i>
### Attach Loyalty Program ID in responses
When the consumer requires that the response will contain the details of loyalty programs involved in processing the requests, we now attach the identifier of the loyalty program to the returned [`loyaltyProgramLedgers` models](https://github.com/talon-one/talon_one.js/blob/1bb8eb8270f2f281f2a262e31f7484ff4801f920/docs/LoyaltyProgramLedgers.md#L7).

The idea behind attaching the identifier is to help streamline further potential requests to our Management API with regard to details about a Loyalty Program, for example [getLoyaltyStatistics](https://developers.talon.one/Management-API/API-Reference#getLoyaltyStatistics) or [getLoyaltyPoints](https://developers.talon.one/Management-API/API-Reference#getLoyaltyPoints), that require the program identifier as part of the URI of the endpoint.

[☝️ Back to Table of Contents](#user-content-toc)

<i id="deprecation-reminder"></i>
### ⚠️ A reminder of The Deprecation Notice: Integration API@v1 endpoints
The deprecation was introduced already in the last release of the SDK, here is a kind reminder of the deprecation notices for Integration API@v1 endpoints:

 - [Update a Customer Session (V1)](https://github.com/talon-one/talon_one.js/blob/master/docs/IntegrationApi.md#updateCustomerSession)
 - [Update a Customer Profile (V1)](https://github.com/talon-one/talon_one.js/blob/master/docs/IntegrationApi.md#updateCustomerProfile)

These endpoints will be flagged deprecated on _15.07.2021_, meaning support for requests to these endpoints will end on that date. **We will not remove the endpoints**, and they will still be accessible for you to use.

We highly encourage migrating to the correspondent v2 endpoints for easier and more granular integration, as well as new features support (See [our developer docs section](https://developers.talon.one/Getting-Started/APIV2) about API V2.0).

[☝️ Back to Table of Contents](#user-content-toc)

<i id="improve-ts"></i>
### Improve Typescript definitions for optional request parameters
Thanks to @emadgit and their reported [issue](#21) regarding the topic, we have conducted some ground work to improve the JSDoc definitions the OpenAPI Generator produces.

This allows us to emit optional parameters type definitions correctly and flag them as optional arguments to the different API functions.
However, there is a bug in the current Generator functionality, which doesn't infer types of enum-typed parameters correctly in optional bags, which unfortunately results in not fully correct emitted type definitions for those bags.
For example, one can see that the the `campaignState` parameter of the `getCampaigns` request is wrongly inferred as `module:model/String` instead of simply `String` or the correct Enum definition.
https://github.com/talon-one/talon_one.js/blob/58deea2f1860e3e3da5af31adc6802f48b821c06/src/api/ManagementApi.js#L2424-L2439

**That in turn leads to their emitted typescript definition inference as `any`, and ignoring the fact that they are optional.**

We are investigating the issue and will hopefully be able to open a bug report and suggest a patch to the [OpenAPI Generator](https://github.com/OpenAPITools/openapi-generator/) team.

In the meantime, a workaround could be to type the optional bags as `any` before passing them to the requests to avoid the TS compiler from complaining:
```ts
const managerApi = new TalonOne.ManagementApi();

// no opts provided at all - now valid 🎉 
managerApi.getCampaigns(/* applicationId */ 1)

// untyped opts provided with selective parameters - still fails 😞 due to the above-mentioned issue
managerApi.getCampaigns(/* applicationId */ 1, { tags: 'retargeting'})

// opts typed as `any` - valid  🎉 
const opts: any = {
  tags: 'retargeting'
}

managerApi.getCampaigns(/* applicationId */ 1, opts)
```

resolves #21

[☝️ Back to Table of Contents](#user-content-toc)

<i id="misc"></i>
### Misc: OpenAPI Generator version upgrade
We have upgraded the OpenAPI Generator used to release this SDK from [v4.2.3](https://github.com/OpenAPITools/openapi-generator/releases/tag/v4.2.3) to [v4.3.1](https://github.com/OpenAPITools/openapi-generator/releases/tag/v4.3.1) which includes a few subtle improvements in the generated code, for full list of changes, please consult the release logs' under the _Javascript_ section.

[☝️ Back to Table of Contents](#user-content-toc)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants