Project 3 - Backend (Ian Lau, Qian Ling, Charles Lee)#53
Project 3 - Backend (Ian Lau, Qian Ling, Charles Lee)#53charlesleezhaoyi wants to merge 177 commits intorocketacademy:mainfrom
Conversation
…astName, email) on top of default fields
… insert User completed
…t before creating, using findOrCreate method
Test twilio sms
created search logic and email
Updated with email Consent and email notification in accpetRequest
Test check jwt
Test final fix
Fix transaction
| @@ -1 +1,2 @@ | |||
| node_modules/ No newline at end of file | |||
| app.use("/categories", checkJwt, categoriesRouter.routes()); | ||
| app.use("/users", checkJwt, usersRouter.routes()); | ||
| app.use("/posts", checkJwt, postsRouter.routes()); | ||
| app.use("/books", checkJwt, booksRouter.routes()); | ||
| app.use("/comments", checkJwt, commentsRouter.routes()); | ||
| app.use("/requests", checkJwt, requestsRouter.routes()); | ||
| app.use("/donations", checkJwt, donationsRouter.routes()); |
There was a problem hiding this comment.
if all your routes are protected, why not just do
app.use(checkJwt)
instead of defining this on every base route?
| key: "id", | ||
| }, | ||
| }, | ||
| beneId: { |
There was a problem hiding this comment.
i wouldn't name a property like such, as it is not clear to everyone. Everytime I read this, I need to think what bene is
| } | ||
| like.init( | ||
| { | ||
| likerId: { |
| } | ||
| comment.init( | ||
| { | ||
| commentedPostId: { |
| if (recipient.smsConsent && recipient.phone) { | ||
| await client.messages.create({ | ||
| body: `Your request has been accepted. Please contact this number ${donation.donor.phone} or email ${donation.donor.email} to arrange a pick up time & location.`, | ||
| from: process.env.DB_TWILIO_TEST_NUMBER, | ||
| to: recipient.phone, | ||
| }); | ||
| } | ||
| if (recipient.emailConsent) { | ||
| await mailjetRequest.request({ | ||
| Messages: [ | ||
| { | ||
| From: { | ||
| Email: process.env.MAILJET_SENDER, | ||
| Name: "Book Swap", | ||
| }, | ||
| To: [ |
There was a problem hiding this comment.
When building integrations with such services, I would definitely recommend more fine-grained structure to your github repository. We can introduce multiple layers to our architecture. Here we could make use of services, which handle the business logic and we only use the controllers for accepting requests and sending responses.
You can look up three-tier architecture, hexagonal architecture, domain driven design, and other concepts for this.
| email: email, | ||
| }, | ||
| }); | ||
| await requester.addRequesterDonation(donation, { |
| }, | ||
| } | ||
| ); | ||
| return res.json("Okay"); |
There was a problem hiding this comment.
Why the Okay string? Seems like status code 200 would already be "Okay". What use does this string serve? :)
| try { | ||
| this.checkStringFromParams(email, "email"); | ||
| const data = await this.userModel.findOne({ where: { email: email } }); | ||
| return res.json(data); |
There was a problem hiding this comment.
here you are returning the user's email and the user's phone number, just based off receiving a token. I could be a user, and enter any other user's email address in the params, and get all their sensitive information. Please be aware of this :)
| await this.userModel.update( | ||
| { firstName, lastName, phone, smsConsent, emailConsent }, | ||
| { where: { email: email } } | ||
| ); |
There was a problem hiding this comment.
Same here. I can probably update another user's information on this endpoint, as long as I have an own account/token, that I pass with the request. You should try it :)
- Login
- Make a request (check network tab for token attached to request)
- Copy token
- Open Postman
- Make request against this endpoint with your token and another user's email
- Check DB if that user's data has been updated
4 participants
No description provided.