Skip to content

Setup: Stripe API #60#97

Open
uche-okoroafor wants to merge 9 commits into
mainfrom
Setup-Stripe-API-#60-/full-stack
Open

Setup: Stripe API #60#97
uche-okoroafor wants to merge 9 commits into
mainfrom
Setup-Stripe-API-#60-/full-stack

Conversation

@uche-okoroafor

Copy link
Copy Markdown
Collaborator

What this PR does (required):

Setup: Stripe API :

  • Create a stripe customer Account.
  • Add Payment Methods of the Customer to the stripe account
  • Retrieve customer Payment methods
  • Create a payment intent

Screenshots / Videos (required):

Screenshot (113)
Screenshot (114)
Screenshot (115)
Screenshot (116)
Screenshot (117)

Screenshot (118)
Screenshot (119)
Screenshot (120)

Any information needed to test this feature (required):

  • Create a stripe account, copy your public Key and Secret Key
  • Install the dependencies by running npm install on the client folder and the server folder
  • Go to the App.tsx and paste the public key at "loadStripe"( loadStripe(publicKey) )
  • Go to the .env file in the server folder and add your secret key STRIPE_SK

Any issues with the current functionality (optional):

  • List any problems where you were not able to complete all tasks on ticket
  • List any problems this PR may cause for the project or other active PRs
  • Post a screenshot/video of the problem, along with detailed console outputs where applicable

@sundayezeilo sundayezeilo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work to this stage 👍
Just check out a few comments I left and take a look.
Happy coding!

Comment thread client/src/pages/Payment/Payment.tsx Outdated
setUserIds({ stripeId, userId: user.id });
return stripeId;
} catch (err) {
console.log(err);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove console log

Comment thread client/src/pages/Payment/Payment.tsx Outdated

SetPaymentProfileExist(true);
} catch (err) {
console.log(err);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove console log

Comment thread server/controllers/payment.js Outdated
const asyncHandler = require("express-async-handler");

exports.createPaymentIntent = asyncHandler(async (req, res) => {
if (req.method === "POST") {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once you create your routes, you don't need this kind of check.

Comment thread server/controllers/payment.js Outdated
exports.createPaymentIntent = asyncHandler(async (req, res) => {
if (req.method === "POST") {
try {
const { amount } = req.body;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to validate amount

Comment thread server/controllers/paymentProfile.js Outdated
const { userId } = req.params;
if (!userId || !id) {
return res
.status(404)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A missing field in the request should be a bad request status 400

Comment thread server/controllers/paymentProfile.js Outdated
const { customerId } = req.params;

if (!customerId) {
return res.status(404).json({ err: "customerId is undefined" });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a bad request and return 400

Comment thread server/controllers/paymentProfile.js Outdated
const customer = await stripe.customers.retrieve(customerId);
res.status(200).json({ customer, paymentMethods });
} catch (err) {
res.status(400).json({ err: err.message });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be a 422

Comment thread server/controllers/paymentProfile.js Outdated
.attach(id, {
customer: customer.id,
})
.catch((err) => res.status(404).json({ err: err.message }));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be a 422

Comment thread server/controllers/payment.js Outdated
Comment on lines +17 to +19
} else {
res.setHeader("Allow", "POST");
res.status(405).end("Method Not Allowed");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines are irrelevant, as long as your routes are properly created.

Comment thread server/controllers/paymentProfile.js Outdated
const { paymentMethodId, userStripeId } = req.body;
if (!paymentMethodId || !userStripeId) {
return res
.status(404)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a 400

Comment thread client/src/context/usePaymentProfilesContext.tsx Outdated

export const PaymentProfilesProvider: FunctionComponent = ({ children }): JSX.Element => {
const [paymentProfiles, setPaymentProfiles] = useState<IPaymentProfiles | undefined>(undefined);
const [loggedInuserDetails, setLoggedInuserDetails] = useState<IPaymentProfiles['loggedInuserDetails']>(undefined);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of the loggedInuserDetails state? Our AuthContext already has a state which holds the logged in user's information

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used it for resetting the data stored in the context because when I log in with a different user on the same computer , the data of the previous user will be there, so I have to reset the data

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we need the loggeInUserDetails you can get it from the AuthContext, it is not advisable to store the same data within another context as this may cause confusion.

const { loggedInUser } = useAuth();
const { updatePaymentProfiles, paymentProfiles, updateLoggedInUser } = usePaymentProfiles();
const [paymentProfileExist, SetPaymentProfileExist] = useState<boolean>(false);
const [userIds, setUserIds] = useState<any>(false);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid using 'any' as a type

Ditto throughout!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will make changes but please how can I declare the data type of a promise containing an object with lots of Properties?

@rajivtitus rajivtitus Oct 8, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can define an interface and use it 👍🏽

Ref: https://www.tutorialsteacher.com/typescript/typescript-interface

</Grid>
<Grid item container md={10} className={classes.gridContainer}>
<Typography variant="h6" className={classes.containerInfo}>
{paymentProfiles && 'Saved Payment Profiles:'}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{paymentProfiles && 'Saved Payment Profiles:'}
{paymentProfiles ? "Saved Payment Profiles:" : "You don't have any Payment Profile"}

Comment thread client/src/pages/Payment/Payment.tsx Outdated
paymentProfiles.map((paymentMethod: any) => (
<Grid item xs={12} sm={12} md={6} key={paymentMethod.id}>
<Card className={classes.payCard}>
<div className={classes.cardHeader}>

@rajivtitus rajivtitus Oct 6, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use MUI Box components instead of HTML divs. A lot of the CSS properties can be passed as props to the Box component

Eg: <Box display="flex" alignItems="center".....>

Ditto throughout!

import * as Yup from 'yup';
import { iframeStyles } from './CardElementStyles/CardElementStyles';

interface IState {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
interface IState {
interface BillingInfo{

Co-authored-by: Rajiv Titus <75184965+rajivtitus@users.noreply.github.com>

@rajivtitus rajivtitus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uche-okoroafor Good work with this! Just some minor comment to address :)

@rajivtitus

Copy link
Copy Markdown
Contributor

@uche-okoroafor There are a couple of pending comments, once they are addressed please tag me in so that we can get this approved :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants