CodeWithAsheville / court-notifications

GNU General Public License v3.0
9 stars 9 forks source link

Turned off Twilio message for register sub in develop mode #156 #159

Closed GKosheev closed 1 year ago

GKosheev commented 1 year ago

name: " #156 Turned off Twilio message for register subscription in development mode" about: "I'm ready to check in some code to fix an issue!" title: "[07/29/2022]: [Turned off Twilio message for development register]" assignee: colinalford

Closes #156

Please check if the PR fulfills these requirements

What changes are you introducing? I've wrapped Twilio message sender in registerSubscription function with an "if" closet, checking if current NODE_ENV mode !== "development". If mode equals "development", function skips sending a message to the client.

-let msg = Mustache.render(nameTemplate, defendantDetails);

+if (String(process.env.NODE_ENV) !== "development") {
+let msg = Mustache.render(nameTemplate, defendantDetails);
+...
+}
}

I also fixed the call of 2 async function without "await" keyword (line 64 and 67 on the screenshot):


-logSubscription(defendant, cases, req.language);
-unsubscribe(phone);

+await logSubscription(defendant, cases, req.language);
+await unsubscribe(phone);

I should have probably moved those last 2 fixes in other pull request.

Screenshots or Video: image

GKosheev commented 1 year ago

Changes in sample.env:

+TWILIO_NO_SMS=true

Changes in register-subscription.js:

-if (String(process.env.NODE_ENV) !== "development") {

+if (Boolean(process.env.TWILIO_NO_SMS)) {

Also, I undid "await" keyword (line 64 and 67):

-await logSubscription(defendant, cases, req.language);
-await unsubscribe(phone);

+logSubscription(defendant, cases, req.language);
+unsubscribe(phone);
GKosheev commented 1 year ago

Completely agree on the things you've mentioned. I'll make sample variable more readable: ENABLE_TWILIO_SMS=true

In that case, if statement will look like that:

 if (process.env.NODE_ENV == 'production' || process.env.ENABLE_TWILIO_SMS == 'true')
GKosheev commented 1 year ago

I've made final changes

sample.env:

-ENABLE_TWILIO_SMS=true

+DISABLE_SMS=false

register.subscription.js:

-if (process.env.NODE_ENV == 'production' || process.env.ENABLE_TWILIO_SMS == 'true')

+if (process.env.NODE_ENV == 'production' || process.env.DISABLE_SMS !== 'true')