Refactor request #4

weronika-kurczyna · 2022-11-17T08:07:32Z

Made refactor in other endpoints (merge 2 functions in 1), reduce the code.
Deleted the alert message because it makes no sense. If the campaign doesn't exists, the script won't run and the error will occur.
Add clear information about missing campaign in main server.js, in console (error message) and the instruction in main readme and tiered-cart-promotion readme.

…utes", remove "accessToStackingPromotionApp". Change names of "promotionStackableObj" and "defaultItems". Move "validateStackableParams" and "customer" objects to "addStackingPromotionRoutes" function.

Add validateStackableParams and redeemStackableParams instead of one global object

Assigning params directly in object.

I deleted the alert because it started generating bugs when checking if the rewardPromotion exists.

marcin-slezak · 2022-11-18T15:46:56Z

README.md

+- Keep the returned validation_rule id after executing each request. It will be required for the main request.
+
+```
+curl --location --request POST 'https://api.voucherify.io/v1/validation-rules' \


As we discussed on Friday, a better option will be to write a small Nodejs CLI script, as then we can automate putting generated validation_rule id to campaign endpoint.

here, you probably want to convince developer to use missing-campaign.js script instead of showing curls

main README: no cURLs and
Note: The application should automatically add the missing campaign "Reward Promotion" by using script called "missing-campaign.js", so there is no need to add it manually.

README id tiered-cart-promotions:
Information previously added by Patryk + info about 'missing-campaign' + I left the cURL there "just in case", but I can also delete them from the second README.

marcin-slezak · 2022-11-18T15:53:52Z

server.js

@@ -41,19 +41,31 @@ const checkCredentials = async () => {
        throw new Error(error);
    }
 };
+
+//Important!


It would be great to put a little attention to how we format code comments:

//Some comment vs // Some comment with space before the first letter //Multi line //comment without a space at the beginning // and starting with slashes vs /* Multiline (block) comments with proper spacing make it easier to read. */

marcin-slezak · 2022-11-22T09:48:11Z

missing-campaign.js

+    }
+
+    const createRewardPromotion = async () => {
+        await Promise.all([


You mixed two approaches:

chaining promises

const result1 = await somePromise1(); const result2 = await somePromise2(); const result3 = await somePromise3();

parallel promises

const results = await Promise.all([ somePromise1(), somePromise2(), somePromise3() ])

You also mixed async/await syntax with then syntax.

And you miss promise handling for client.campaigns.create

At the end:

const validationRules = await Promise.all([ client.validationRules.create(firstValidationRule), client.validationRules.create(secondValidationRule), client.validationRules.create(thirdValidationRule), ]) const rewardPromotion = createRewardPromotionObject(validationRules[0].id, validationRules[1].id, validationRules[2].id); const response = await client.campaigns.create(rewardPromotion); if (response.code !== 200) { return new Error; }

changed, but have to check one thing before pushing

marcin-slezak · 2022-11-22T09:51:37Z

README.md

+- Keep the returned validation_rule id after executing each request. It will be required for the main request.
+
+```
+curl --location --request POST 'https://api.voucherify.io/v1/validation-rules' \


here, you probably want to convince developer to use missing-campaign.js script instead of showing curls

marcin-slezak · 2022-11-22T09:52:27Z

tiered-cart-promotions/README.md

+- Keep the returned validation_rule id after executing each request. It will be required for the main request.
+
+```
+curl --location --request POST 'https://api.voucherify.io/v1/validation-rules' \


missing-campaign.js instead of curls?

We do not need to describe curl details if we have a JS script. Just leave a information that you need to run a script if you have missing campaigns

marcin-slezak · 2022-11-22T09:52:54Z

server.js

+        console.log("The Reward Campaign was successfully created.");
+      }
+      catch (error) {
+        const msg = "The 'Reward Promotion' campaign not found. This campaign is required for 'tiered-cart-promotion' to work properly. \r\nPlease create a 'Reward Promotion' campaign first. \r\nYou can check the details by visiting: https://github.com/voucherifyio/voucherify-examples/tree/main/tiered-cart-promotions#creating-a-reward-promotion-campaign";


suggest missing-campaign.js ?

more info about 'missing-campaign' script

marcin-slezak · 2022-11-23T10:21:17Z

README.md

@@ -53,6 +53,8 @@ npm run start / npm run dev || yarn start / yarn run dev
 ```
 7. Go to [http://localhost:3000](http://localhost:3000/) in your browser.

+Note: The application should automatically add the missing campaign "Reward Promotion" by using script called "missing-campaign.js", so there is no need to add it manually.


node missing-campaign? Also, it would be great to add a verb into the script name as we do some action, so maybe node add-missing-campaign?

marcin-slezak · 2022-11-23T10:21:45Z

missing-campaign.js

+        "context_type": "campaign.promotion.discount.apply_to_order"
+    }
+
+    // const createRewardPromotion = async () => {


do not leave the code that is commented out

From now if an error from V% occurs, it will be logged to the console, but also the informative "msg" defined by me.

marcin-slezak · 2022-12-06T07:38:06Z

add-missing-campaign.js

@@ -0,0 +1,207 @@
+export const addMissingCampaign = async (client) => {
+
+    const createRewardPromotionObject = (firstValidationRule, secondValidationRule, thirdValidationRule) => {


We do not have typescript support here, so even more important is to pay attention to naming. Please note that the createRewardPromotionObject function accepts as parameters id of three validation rules, not validation rule definitions from below. Therefore, the firstValidationRule name has two meanings in that file, once its rule id, and once its rule definition. I suggest changing a name from firstValidationRule to firstValidationRuleId

marcin-slezak · 2022-12-06T07:43:52Z

add-missing-campaign.js

@@ -0,0 +1,207 @@
+export const addMissingCampaign = async (client) => {


I like the idea of using closure to provide V% client 👍 Please note that only the addValidationRulesAndCampaign function needs that context. The createRewardPromotionObject, firstValidationRule, secondValidationRule and thirdValidationRule functions do not need this context so they can be put outside of the outer function.

marcin-slezak · 2022-12-06T07:48:46Z

server.js

 };
+
+/*
+Important!


This comment is outdated.

marcin-slezak · 2022-12-06T07:53:09Z

server.js

+for getting instructions on how to create the missing campaign.
+*/
+
+const checkCampaign = async () => {


The checkCampaign function actually adds a campaign to the V% account if it does not exists, which may be a little dangerous if someone by accident connects the production account. I suggest does not allow to run the example (like for checkCredentials) and displaying the message that you have already in the catch block.

marcin-slezak · 2022-12-06T07:53:37Z

tiered-cart-promotions/README.md

+- Keep the returned validation_rule id after executing each request. It will be required for the main request.
+
+```
+curl --location --request POST 'https://api.voucherify.io/v1/validation-rules' \


marcin-slezak · 2022-12-06T07:54:58Z

tiered-cart-promotions/README.md

+- Keep the returned validation_rule id after executing each request. It will be required for the main request.
+
+```
+curl --location --request POST 'https://api.voucherify.io/v1/validation-rules' \


We do not need to describe curl details if we have a JS script. Just leave a information that you need to run a script if you have missing campaigns

- Changed the naming from "firstValidationRule" to "firstValidationRuleId" etc. - Deleted unnecessary comments and text in Readme - Formatted code with tslint formatter - Moved "createRewardPromotionObject" and validation rules object outside the outer function

marcin-slezak · 2023-01-10T08:28:11Z

.vscode/launch.json

@@ -0,0 +1,17 @@
+{


This file should be excluded from the repo

marcin-slezak · 2023-01-10T08:38:25Z

server.js

+        await client.campaigns.get("Reward Promotion");
+    } catch (error) {
+        if (error.code === 404) {
+                const msg = "The 'Reward Promotion' campaign not found. \r\nPlease run the command 'node ./add-missing-campaign.js' in the terminal to create the missing campaign. \r\nWhen you receive a message that the campaign has been successfully created you can run the main script again. \r\nYou can get additional support here: https://github.com/voucherifyio/voucherify-examples/tree/main#get-support- \r\n";


It's a little hard to read it. Also, \r\n is Windows-specific. Maybe:

const { EOL } = require('os'); // ... const msgs = [ 'The \'Reward Promotion\' campaign not found', 'Please run the command \'node ./add-missing-campaign.js\' in the terminal to create the missing campaign.', 'When you receive a message that the campaign has been successfully created you can run the main script again.', 'You can get additional support here: https://github.com/voucherifyio/voucherify-examples/tree/main#get-support' ] // try console.log(msgs.join(EOL)) // or console.log(...msgs)

Thanks for the suggestion to use EOL! 🙂

changes have been made

Deleted unnecessary file

Marcin Slezak and others added 13 commits November 10, 2022 17:09

refactor requests

f998880

Change "attachEndpointsStackingPromotions" to "addStackingPromotionRo…

504948c

…utes", remove "accessToStackingPromotionApp". Change names of "promotionStackableObj" and "defaultItems". Move "validateStackableParams" and "customer" objects to "addStackingPromotionRoutes" function.

Add validateStackableParams and redeemStackableParams

dc0b21a

Add validateStackableParams and redeemStackableParams instead of one global object

Add check if campaign 'Reward Promotion' exists

99478ab

Add alert with information about lack of the campaign

4cb59c0

Reduction of the code

a7c78cb

Assigning params directly in object.

add cURL

241c9f8

reformat cURL

4ff1ab2

Update README.md

b17d1ad

Add information in server.js

11ea819

Delete the alert message

c3b45b1

I deleted the alert because it started generating bugs when checking if the rewardPromotion exists.

Refactor other endpoints

56bd737

Update README.md

fa12e78

weronika-kurczyna requested review from marcin-slezak and patryksmolarz November 17, 2022 08:07

marcin-slezak requested changes Nov 18, 2022

View reviewed changes

weronika-kurczyna added 4 commits November 21, 2022 14:50

Add creating missing campaign JS file

1eedf2d

Change the structure of the functions

84c1b30

Update server.js

9978cef

Update README.md

70a50ff

marcin-slezak requested changes Nov 22, 2022

View reviewed changes

weronika-kurczyna added 2 commits November 23, 2022 09:34

Change readme and main server.js file

0b20547

more info about 'missing-campaign' script

Change the structure and naming a bit

08fecd3

marcin-slezak requested changes Nov 23, 2022

View reviewed changes

weronika-kurczyna added 3 commits November 23, 2022 12:27

update missing-campaign

4606749

naming change

d2b8fb1

Change the way errors are displayed

d55f8fd

From now if an error from V% occurs, it will be logged to the console, but also the informative "msg" defined by me.

marcin-slezak requested changes Dec 6, 2022

View reviewed changes

weronika-kurczyna added 2 commits December 6, 2022 11:59

Change the way of running "add-missing-campaign.js" script

1679892

weronika-kurczyna added 2 commits December 12, 2022 15:20

Little change in message

43bcca6

Change format of the message in server.js (better readability)

e5f79d4

marcin-slezak requested changes Jan 10, 2023

View reviewed changes

Change the formatting of the console message using EOL

6f180eb

Deleted unnecessary file

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor request #4

Refactor request #4

weronika-kurczyna commented Nov 17, 2022

marcin-slezak Nov 18, 2022

marcin-slezak Nov 22, 2022

weronika-kurczyna Nov 23, 2022

marcin-slezak Nov 18, 2022

marcin-slezak Nov 22, 2022

weronika-kurczyna Nov 23, 2022 •

edited

Loading

marcin-slezak Nov 22, 2022

marcin-slezak Nov 22, 2022

marcin-slezak Dec 6, 2022

marcin-slezak Dec 6, 2022

marcin-slezak Nov 22, 2022

marcin-slezak Nov 23, 2022

marcin-slezak Nov 23, 2022

marcin-slezak Dec 6, 2022

marcin-slezak Dec 6, 2022

marcin-slezak Dec 6, 2022

marcin-slezak Dec 6, 2022

marcin-slezak Dec 6, 2022

marcin-slezak Dec 6, 2022

marcin-slezak Jan 10, 2023

marcin-slezak Jan 10, 2023

weronika-kurczyna Jan 10, 2023 •

edited

Loading

weronika-kurczyna Jan 10, 2023

		@@ -0,0 +1,207 @@
		export const addMissingCampaign = async (client) => {

		const createRewardPromotionObject = (firstValidationRule, secondValidationRule, thirdValidationRule) => {

Refactor request #4

Are you sure you want to change the base?

Refactor request #4

Conversation

weronika-kurczyna commented Nov 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weronika-kurczyna Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weronika-kurczyna Jan 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weronika-kurczyna Nov 23, 2022 •

edited

Loading

weronika-kurczyna Jan 10, 2023 •

edited

Loading