-
Notifications
You must be signed in to change notification settings - Fork 70
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
feat(discount-codes): implement function to generate codes #147
Conversation
affects: @commercetools/discount-code-generator
Codecov Report
@@ Coverage Diff @@
## 142-code-generator-main #147 +/- ##
===========================================================
+ Coverage 97.06% 97.11% +0.04%
===========================================================
Files 53 53
Lines 989 1004 +15
===========================================================
+ Hits 960 975 +15
Misses 29 29
Continue to review full report at Codecov.
|
CodeData, | ||
CodeDataArray, | ||
CodeOptions, | ||
} from 'types/sdk' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the types/sdk
file is getting large, separate out your type declarations to another file...
// en: 'bar' | ||
// }, | ||
// cartDiscounts: [], | ||
// cartPredicate: 'some predicate', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use multi-line comment in javascript
// } | ||
|
||
export default function discountCodeGenerator ( | ||
options: CodeOptions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can wrap all params in one object, except there is possibiliity you reuse each later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Params are currently in one options
object. The data
object is the one being mutated
if (!prefix.length) | ||
return code | ||
|
||
const codePrefix = prefix.toUpperCase() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why you forcing to uppercase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the discount codes be in lowercase or mixed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Semantics 😇😇
It is case-sensitive. Thought it wasn't so I was forcing it all to uppercase 😃
|
||
codes.forEach((codeObject) => { | ||
expect(codeObject).toMatchObject(data) | ||
expect(codeObject.code).toBeTruthy() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The next assertion made this assertion redundant
affects: @commercetools/discount-code-generator
@hisabimbola updated changes, kindly review :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some small comments, but it looks good 👍
* cartDiscounts: [], | ||
* cartPredicate: 'some predicate', | ||
* isActive: true, | ||
* maxApplicationsPerCustomer: 10, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is called maxApplications
http://dev.commercetools.com/http-api-projects-discountCodes.html#discountcode
types/discountCodes.js
Outdated
cartDiscounts: []; | ||
cartPredicate?: string; | ||
isActive: boolean; | ||
maxApplicationsPerCustomer?: number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is called maxApplications
http://dev.commercetools.com/http-api-projects-discountCodes.html#discountcode
* prefix: CT, | ||
* } | ||
|
||
* The {data} should have the attributes of the discount codes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Provide a link to the documentation
http://dev.commercetools.com/http-api-projects-discountCodes.html#discountcode
affects: @commercetools/discount-code-generator
* } | ||
* | ||
* More information about the discount codes can be found here: http:// | ||
* dev.commercetools.com/http-api-projects-discountCodes.html#discountcode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this be the best way to add this link without annoying eslint? 🤔 Cc @hisabimbola @emmenko
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can simply disable eslint for such comments ;)
affects: @commercetools/discount-code-generator
affects: @commercetools/discount-code-generator
Summary
Implement the discount code generator function as part of this checklist