ACDEV-4956: refactoring, adding dailyBonus, availability - #3
Conversation
There was a problem hiding this comment.
@woolfi182 hey, there are 62 files changes in this PR. i have no idea how to review it like this or how to run sanity on that amount of changes.
can you please open new PRs with smaller amount of changes?
and please add unit tests if needed, like this example : https://github.com/Appcharge/common/commit/b1b0d2d4df7b8d54e178301c6ed93397b1c99fdc
| ]); | ||
| const response = await axios.put( | ||
| this.offerUrl + offerId, | ||
| modifiedOfferDataset, |
There was a problem hiding this comment.
looks complicated to understand what the object should look like, maybe we should put plain objects instead
|
|
||
| const Position = ["right", "left", "center"]; | ||
|
|
||
| const CreateOfferSchema = Joi.object({ |
| }; | ||
|
|
||
| router.post("/", async (req, res) => { | ||
| const { error } = GetOrdersRequestSchema.validate(req.body); |
There was a problem hiding this comment.
why are we making it that complicated?
if we want to show examples of how to use orders API / assets upload, wouldn't it make more sense to put examples with plain objects instead of doing double validations on the input and letting them figure out the request body from the Joi validation?
There was a problem hiding this comment.
it was written by @shaiboujuappcharge a long time ago, I didn't touch this. I can remove this to make it easier to understand
| requestBody, | ||
| { | ||
| headers: { | ||
| signature: signature, |
There was a problem hiding this comment.
remove all signatures from requests to the gateways, publisher token should be enough
There was a problem hiding this comment.
I'm not sure we removed it from all our packages. I'll discuss it with @talkot1
3 participants
No description provided.