#159762182 Integrate Payment #30

yomigeek · 2018-09-03T09:28:31Z

What does this PR do?

Integrate Payement System to allow users upgrade to a premium account on the platform

Description of Task to be completed?

consume Flutterwave Rave API
create a dummy front-end page to initiate the payment
create a middleware that checks if the payment reference is provided
prevent user account upgrade if payment reference is invalid during a check with the Flutterwave Rave API
upgrades user account if payment reference is valid during a check with the Flutterwave Rave API

How should this be manually tested?

clone the repo: Run git clone https://github.com/andela/elven-ah.git in the folder where you want the repo to be saved.
navigate into the newly created folder and install the dependencies: cd elven-ah && npm install
copy .env.example to .env: cp .env.example .env
provision a PostgreSQL database and add the credentials to the .env file.
start the server using npm start
initiate the payment using a front-end client to get the payment reference via the query parameter
upgrade the user account by making a GET request to /api/pay with ref as a query parameter

Any background context you want to provide?

for testing purpose use the rave test API keys by sigining up with flutterwave rave sandbox
on the front-end use the test javascript reference (https://ravesandboxapi.flutterwave.com/flwv3-pug/getpaidx/api/flwpbf-inline.js)

What are the relevant pivotal tracker stories?

#159762182

Screenshots (if appropriate)\

N/A

Questions:

N/A

[Finishes #159762182]

- add a sketchy UI for checking of the process - add a payment model - add a payment controller - install the flutterwave rave package

- modify payment controller file - modify payment model

- create comment reply as first level comment when invalid parentId is supplied [Delivers #160216041]

- add test for comment reply feature that creates comment reply with invalid parent id as first level comment - refactor comment tests to remove unnecessary fields in request body [Finishes #160216041]

- refactor CommentController to use aync/await - extract repeated slug validation code to middleware - modify test to read updated responses [Finishes #160216041]

- add more tests to cover more edge cases [Finishes #160216041]

- add more unit tests - create a check payment refeerence middleware - modify payment controller file [Delivers #159762182]

houndci-bot · 2018-09-03T09:28:46Z

server/migrations/20180831094900-create-payment.js

+  down: (queryInterface, Sequelize) => {
+    return queryInterface.dropTable('Payments');
+  }
+};


Newline required at end of file but not found eol-last

houndci-bot · 2018-09-03T09:28:46Z

server/migrations/20180831094900-create-payment.js

+      }
+    });
+  },
+  down: (queryInterface, Sequelize) => {


'Sequelize' is defined but never used no-unused-vars
Unexpected block statement surrounding arrow body; move the returned value immediately after the => arrow-body-style

houndci-bot · 2018-09-03T09:28:46Z

server/migrations/20180831094900-create-payment.js

@@ -0,0 +1,44 @@
+'use strict';
+module.exports = {
+  up: (queryInterface, Sequelize) => {


Unexpected block statement surrounding arrow body; move the returned value immediately after the => arrow-body-style

houndci-bot · 2018-09-03T09:28:46Z

server/migrations/20180831094900-create-payment.js

@@ -0,0 +1,44 @@
+'use strict';


Expected newline after "use strict" directive lines-around-directive
'use strict' is unnecessary inside of modules strict

houndci-bot · 2018-09-03T09:28:46Z

server/controllers/PaymentController.js

+        },
+        {
+          where: { userId },
+        });


Expected a newline before ')' function-paren-newline

houndci-bot · 2018-09-03T09:28:47Z

server/controllers/PaymentController.js

+          });
+          return PaymentController.PaymentResponse(res);
+        }
+        Payment.update({


Expected a newline after '(' function-paren-newline

coveralls · 2018-09-03T09:31:30Z

Pull Request Test Coverage Report for Build 378

129 of 139 (92.81%) changed or added relevant lines in 12 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.8%) to 90.579%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
server/v1/controllers/PaymentController.js	44	46	95.65%
server/v1/controllers/CommentController.js	40	48	83.33%

Totals
Change from base Build 344:	0.8%
Covered Lines:	714
Relevant Lines:	776

💛 - Coveralls

- fix hound issues - modify test [Delivers #159762182]

- improve test coverage removing redundant codes [Delivers #159762182]

…ntId-error #160216041 fix app crash on comment reply creation with invalid parentId

Oluwafayokemi

Nice job! @yomigeek. Well done

Veraclins

@yomigeek nice job here. Just check out my comments and implement the feedback where necessary.

Veraclins · 2018-09-03T10:37:02Z

server/controllers/PaymentController.js

+      .then((response) => {
+        const { id: userId, email } = req.user;
+        const transactionValues = response.body.data;
+        const subscriptionType = transactionValues.amount === SUBSCRIPTION_RATE ? 'month' : 'annual';


@yomigeek nice job here and quite expressive. I feel the SUBSCRIPTION_RATE constant should be MONTHLY_SUBSCRIPTION_RATE to add context and make the comparable unambiguous.

Thanks for this notification, I would implement this change..

Veraclins · 2018-09-03T10:50:11Z

server/controllers/PaymentController.js

+      });
+  }
+
+  static ProcessUpgrade(paymentInfo, res) {


Is there a reason why there is a ProcessUpgrade method and no method to create a payment? I see that you are actually creating the payment if none is found and updating otherwise. Don't you think ProcessUpgrade may not really be the best method name? I believe processPayment could be a better name. That could include both creating and upgrading the payment and still be descriptive of the whole action. Also, your method name is not in camelCase.

My bad as regards the camelCasing issue, I will implement the change. As regards the method name, I used that because the whole process is all about upgrading a user from a free account to a premium account.

Veraclins · 2018-09-03T11:41:17Z

server/middlewares/checkReference.js

+  * @param {object} next the next middleware function
+
+  */
+const checkReference = (req, res, next) => {


Is there a reason why this is implemented as a middleware instead of as a method in the controller? Given that it is used in just a single place I feel that it is better as a method in the PaymentController. This way everything about the payment is contained in a single place. (Single Responsibility Principle).

Okay, I did that to avoid longer codes to in the validate controller, more or less like every aspect having its own purpose within the whole process.

- add new models based on recent modification to accomodate article subscription - modify the user and article model to accomodate subscription and updates - modify the payment controller file [Delivers #159762182]

houndci-bot · 2018-09-05T11:06:40Z

server/controllers/PaymentController.js

+        if (transactionValues.status === 'successful' && transactionValues['customer.email'] === email) {
+          return PaymentController.accountUpgradeProcessor(
+            { userId, reference, subscriptionType }, res
+          );


Unexpected newline before ')' function-paren-newline

houndci-bot · 2018-09-05T11:06:40Z

server/controllers/PaymentController.js

+        const transactionValues = response.body.data;
+        const subscriptionType = transactionValues.amount === MONTHLY_SUBSCRIPTION_RATE ? 'month' : 'annual';
+        if (transactionValues.status === 'successful' && transactionValues['customer.email'] === email) {
+          return PaymentController.accountUpgradeProcessor(


Unexpected newline after '(' function-paren-newline

- add more units - modify payment controller [Delivers #159762182]

Veraclins

@yomigeek you have done a nice job here. I particularly love the way you arranged the methods in the order of their invocation. This makes it very easy to understand the flow of operation. There are, however, a few areas I feel you may want to look into.

One of the recommendations for writing clean codes is to use verbs (or actions) for method names. This you did well with the checkPaymentType method, however, I feel you can reword the other method names. For instance, accountUpgrade could be upgradeAccount, accountUpgradeProcessor could be processAccountUpgrade, etc.
Since all the failed payment responses return 400 status code, don't you think that defining them everywhere is rather an unnecessary repetition? I believe you can use the number (like you did for success responses) in the method itself.
On line 111, I see that you are checking that an articleId is supplied but you are not checking that the supplied id is a number. This could cause database errors which would break the app.
The userSubscriptionDetailUpdate method was declared to take two arguments but you called it on line 75 without any argument. How does that work?
Finally, you may want to check the comments (docs) for successPaymentResponse and failedPaymentResponse. From their description in the comments, they seem to be doing exactly what userSubscriptionDetailUpdate does.
Once again, nice job bro.

- add more unit tests - modify payment controller file [Deliver #159762182]

yomigeek · 2018-09-06T10:22:04Z

@Veraclins Thanks for the reviews you gave earlier. Reviews and changes have been implemented as deemed necessary...

chore(readme): add api docs link and update url - add link to the API docs and update the url on the readme file to reflect the versioning [Finishes #160127137]

madeofhuman

You did a really great work here, Yomi, especially given the changing scope of the task. Well done! Just address the conflicts arising from my PR merge and we're golden.

- add a sketchy UI for checking of the process - add a payment model - add a payment controller - install the flutterwave rave package

- modify payment controller file - modify payment model

- add more unit tests - create a check payment refeerence middleware - modify payment controller file [Delivers #159762182]

- fix hound issues - modify test [Delivers #159762182]

- improve test coverage removing redundant codes [Delivers #159762182]

- add new models based on recent modification to accomodate article subscription - modify the user and article model to accomodate subscription and updates - modify the payment controller file [Delivers #159762182]

- add more units - modify payment controller [Delivers #159762182]

- add more unit tests - modify payment controller file [Deliver #159762182]

- make changes to files and folder structures to acoomodate api versioning [Delivers #159762182]

- fix merge issues on branch [Delivers #159762182]

tonymontaro

Excellent work!

yomigeek and others added 7 commits August 31, 2018 10:28

feat(integrate-payment): integrate payment on the platform

8b079ac

- add a sketchy UI for checking of the process - add a payment model - add a payment controller - install the flutterwave rave package

feat(integrate-payment): integrate payment on the platform

6fd6336

- modify payment controller file - modify payment model

fix(comment): fix app crash on comment reply with invalid parentId

444d4ad

- create comment reply as first level comment when invalid parentId is supplied [Delivers #160216041]

chore(test): cover new comment reply feature edge case

b1631bd

- add test for comment reply feature that creates comment reply with invalid parent id as first level comment - refactor comment tests to remove unnecessary fields in request body [Finishes #160216041]

chore(refactor): refactor CommentController

5b9d52e

- refactor CommentController to use aync/await - extract repeated slug validation code to middleware - modify test to read updated responses [Finishes #160216041]

chore(test): fix falling test coverage

b4a252f

- add more tests to cover more edge cases [Finishes #160216041]

feat(integrate-payment): implement payment integration

a08ac97

- add more unit tests - create a check payment refeerence middleware - modify payment controller file [Delivers #159762182]

houndci-bot reviewed Sep 3, 2018

View reviewed changes

yomigeek added 2 commits September 3, 2018 10:53

feat(integrate-payment): implement changes based on reviews

2b27dd5

- fix hound issues - modify test [Delivers #159762182]

feat(integrate-payment): improve test coverage

94152bd

- improve test coverage removing redundant codes [Delivers #159762182]

yomigeek requested review from madeofhuman, Oluwafayokemi, Veraclins and chukwuemekachm September 3, 2018 10:34

Merge pull request #29 from andela/fix/160216041/handle-invalid-comme…

1db0cf6

…ntId-error #160216041 fix app crash on comment reply creation with invalid parentId

Oluwafayokemi approved these changes Sep 3, 2018

View reviewed changes

Veraclins suggested changes Sep 3, 2018

View reviewed changes

feat(integrate-payment): Implement modification to payment integration

677c12c

- add new models based on recent modification to accomodate article subscription - modify the user and article model to accomodate subscription and updates - modify the payment controller file [Delivers #159762182]

houndci-bot reviewed Sep 5, 2018

View reviewed changes

feat(integrate-payment): Implement modifications to payment integration

2612ca1

- add more units - modify payment controller [Delivers #159762182]

Veraclins suggested changes Sep 5, 2018

View reviewed changes

feat(integrate-payment): implement changes based on peer reviews

55b77c5

- add more unit tests - modify payment controller file [Deliver #159762182]

Veraclins approved these changes Sep 6, 2018

View reviewed changes

#160127137 version the API (#31)

202b003

chore(readme): add api docs link and update url - add link to the API docs and update the url on the readme file to reflect the versioning [Finishes #160127137]

madeofhuman reviewed Sep 7, 2018

View reviewed changes

yomigeek added 3 commits September 7, 2018 15:00

feat(integrate-payment): integrate payment on the platform

4156533

- add a sketchy UI for checking of the process - add a payment model - add a payment controller - install the flutterwave rave package

feat(integrate-payment): integrate payment on the platform

f7a37b7

- modify payment controller file - modify payment model

feat(integrate-payment): implement payment integration

72c9350

- add more unit tests - create a check payment refeerence middleware - modify payment controller file [Delivers #159762182]

yomigeek added 7 commits September 7, 2018 15:00

feat(integrate-payment): implement changes based on reviews

b00d320

- fix hound issues - modify test [Delivers #159762182]

feat(integrate-payment): improve test coverage

4d5db45

- improve test coverage removing redundant codes [Delivers #159762182]

feat(integrate-payment): Implement modification to payment integration

7f2fc27

- add new models based on recent modification to accomodate article subscription - modify the user and article model to accomodate subscription and updates - modify the payment controller file [Delivers #159762182]

feat(integrate-payment): Implement modifications to payment integration

3166f26

- add more units - modify payment controller [Delivers #159762182]

feat(integrate-payment): implement changes based on peer reviews

b51aaa1

- add more unit tests - modify payment controller file [Deliver #159762182]

feat(integrate-payment): implement changes to acommodate api versioning

8126e24

- make changes to files and folder structures to acoomodate api versioning [Delivers #159762182]

feat(integrate-payment): fix merge issues

9ce60df

- fix merge issues on branch [Delivers #159762182]

tonymontaro approved these changes Sep 10, 2018

View reviewed changes

tonymontaro merged commit c177165 into staging Sep 10, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#159762182 Integrate Payment #30

#159762182 Integrate Payment #30

yomigeek commented Sep 3, 2018

houndci-bot Sep 3, 2018

houndci-bot Sep 3, 2018

houndci-bot Sep 3, 2018

houndci-bot Sep 3, 2018

houndci-bot Sep 3, 2018

houndci-bot Sep 3, 2018

coveralls commented Sep 3, 2018 •

edited

Loading

Oluwafayokemi left a comment

Veraclins left a comment

Veraclins Sep 3, 2018

yomigeek Sep 3, 2018

Veraclins Sep 3, 2018

yomigeek Sep 3, 2018

Veraclins Sep 3, 2018

yomigeek Sep 3, 2018

houndci-bot Sep 5, 2018

houndci-bot Sep 5, 2018

Veraclins left a comment •

edited

Loading

yomigeek commented Sep 6, 2018

madeofhuman left a comment

tonymontaro left a comment

#159762182 Integrate Payment #30

#159762182 Integrate Payment #30

Conversation

yomigeek commented Sep 3, 2018

What does this PR do?

Description of Task to be completed?

How should this be manually tested?

Any background context you want to provide?

What are the relevant pivotal tracker stories?

Screenshots (if appropriate)\

Questions:

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

coveralls commented Sep 3, 2018 • edited Loading

Pull Request Test Coverage Report for Build 378

💛 - Coveralls

Oluwafayokemi left a comment

Choose a reason for hiding this comment

Veraclins left a 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

Veraclins left a comment • edited Loading

Choose a reason for hiding this comment

yomigeek commented Sep 6, 2018

madeofhuman left a comment

Choose a reason for hiding this comment

tonymontaro left a comment

Choose a reason for hiding this comment

coveralls commented Sep 3, 2018 •

edited

Loading

Veraclins left a comment •

edited

Loading