-
Notifications
You must be signed in to change notification settings - Fork 35
add payment project integration #1994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
Co-authored-by: adarsh <110150037+adarshm11@users.noreply.github.com>
charred-70
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm tuffany is also my hero
* initial commit * fixed spacing * fixed spacing 2 * fixed spacing 2 * updated logic and merged find and Verify functions * fix spacing * refactor: improve payment verification and rejection logic * fix * refactor: improve error handling in membership payment verification and rejection * fix * final fixes * final fixes * refactored updateMembershipExpiration * fix spacing
* i pray i fixed my origin * added instructions * fixed import error * fixed template formatting * made function to call email route --------- Co-authored-by: Charlynn Nguyen <charred@Charlynns-MacBook-Pro.local>
* verify-button-test-1 * fix lint
evanugarte
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we split this into 3 prs
- the changes to the email code
- backend changes
- react changes
| return resolve({ | ||
| from: user, | ||
| to: recipient, | ||
| subject: 'SCE Membership Confirmation', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we put the code in the subject too
Your SCE Membership Code is ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| createdAt: { | ||
| type: Date, | ||
| default: Date.now | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does mongodb already store this field? this might work
const mySchema = new mongoose.Schema({
name: String,
// other fields...
}, {
timestamps: true
});| if (amount < 20){ | ||
| const rejected = await rejectPayment(paymentId); | ||
| if (rejected === null){ | ||
| logger.error('Error rejecting payment with ID:', paymentId); | ||
| return res.status(SERVER_ERROR).send('Error rejecting payment.'); | ||
| } | ||
| if (rejected === false){ | ||
| logger.error('No payment found to reject with ID:', paymentId); | ||
| return res.status(NOT_FOUND).send('No payment found to reject.'); | ||
| } | ||
| logger.info('Payment rejected due to insufficient amount. Payment ID:', paymentId); | ||
| return res.status(BAD_REQUEST).send('Payment amount insufficient.'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we dont need this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
|
||
| const confirmationCode = crypto.randomBytes(4).toString('hex').toUpperCase(); | ||
| const newPayment = { | ||
| userId: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| userId: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| async function membershipConfirmationCode(confirmCode, email) { | ||
| return new Promise((resolve) => { | ||
| axios | ||
| .post(`${MAILER_API_URL}/Mailer/sendMembershipConfirmationCode`, { | ||
| recipientEmail: email, | ||
| confirmationCode: confirmCode | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| async function membershipConfirmationCode(confirmCode, email) { | |
| return new Promise((resolve) => { | |
| axios | |
| .post(`${MAILER_API_URL}/Mailer/sendMembershipConfirmationCode`, { | |
| recipientEmail: email, | |
| confirmationCode: confirmCode | |
| }) | |
| async function membershipConfirmationCode(confirmationCode, recipientEmail) { | |
| return new Promise((resolve) => { | |
| axios | |
| .post(`${MAILER_API_URL}/Mailer/sendMembershipConfirmationCode`, { | |
| recipientEmail, | |
| confirmationCode, | |
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| router.post('/verifyMembership', async (req, res) => { | ||
| const decoded = await decodeToken(req, membershipState.PENDING); | ||
| if (decoded.status !== OK) { | ||
| return res.sendStatus(decoded.status); | ||
| } | ||
|
|
||
| const { confirmationCode } = req.body; | ||
| const userId = decoded.token._id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we rate limit this by user email, we dont have to solve that rn, just open an issue like using the decoded email from the token, if theres more than 5 requests in the past 30 min, send 429 or something
| const [confirmationCode, setConfirmationCode] = useState(''); | ||
| const { user } = useSCE(); | ||
|
|
||
| const INPUT_CLASS_NAME = 'indent-2 block w-full rounded-md border-0 py-1.5 bg-white text-black shadow-sm ring-1 ring-inset ring-gray-300 placeholder-gray-400 focus:ring-2 focus:ring-inset focus:ring-indigo-600 sm:text-sm sm:leading-6'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this above the function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| confirmationCode, | ||
| ); | ||
| if (apiResponse.error) { | ||
| bannerCallback('Unable to verify membership. Please try again later.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include the http code in the title? in case they take a screenshot and post in the discord?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| bannerCallback('Unable to verify membership. Please try again later.'); | ||
| return; | ||
| } | ||
| bannerCallback('Congrats, you confirmed your membership!'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add the date like
you are now a member (expiration DATE_GOES_HERE)
like could the api return the date that the expiration is, if the code worked
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expiration date already shows on their profile when membership is confirmed vro
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as soon as the modal closes, is the profile page updated or do they need to refresh
| <dialog id="verify-membership-modal" className="modal modal-bottom sm:modal-middle"> | ||
| <div className="modal-box"> | ||
| <h3 className="font-bold text-lg">Verify Membership</h3> | ||
| {accessLevel > 0 ? ( | ||
| <> | ||
| <p className="text-sm text-gray-500 mt-2"> | ||
| You are already verified!!! | ||
| </p> | ||
| <div className="modal-action"> | ||
| <form method="dialog"> | ||
| <div className="px-4 py-3 sm:flex sm:flex-row-reverse sm:px-6"> | ||
| <button | ||
| className="btn inline-flex w-full justify-center rounded-md px-3 py-2 text-sm font-semibold shadow-sm ring-1 ring-inset sm:w-auto" | ||
| > | ||
| Close | ||
| </button> | ||
| </div> | ||
| </form> | ||
| </div> | ||
| </> | ||
| ) : ( | ||
| <> | ||
| <p className="text-sm text-gray-500 mt-2"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (something) {
return dialog_goes_here
}
return p_class_etc_goes_here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
holy nitpick ✌️
| const { findVerifyPayment, storePayment } = require('../util/membershipPaymentQueries.js'); | ||
| const { decodeToken } = require('../util/token-functions.js'); | ||
| const { membershipPayment = {} } = require('../../config/config.json'); | ||
| const { API_KEY = 'TUFFANYCHAR' } = membershipPayment; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
be sure to update config example json to include this field
| if (amount < 20) { | ||
| return res.status(BAD_REQUEST).send('Payment amount must be at least $20.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we dont need this? like the validation should happen before we call this api
we should assume that an incoming request with a valid api key means we are trying to store a payment of the correct amount
| return true; | ||
| } catch (error) { | ||
| logger.error('Error updating membership:', error); | ||
| return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets return false instead of null
true if worked, false if didnt work for any reason
| return resolve({ | ||
| from: user, | ||
| to: recipient, | ||
| subject: 'SCE Membership Confirmation: Your SCE Membership Code Is...', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i meant like put the code in the subject
sorry that wasnt clear
| subject: 'SCE Membership Confirmation: Your SCE Membership Code Is...', | |
| subject: `Your SCE Membership Code Is ${confirmCode}`, |
| bannerCallback('Unable to verify membership. Please try again later.'); | ||
| return; | ||
| } | ||
| bannerCallback('Congrats, you confirmed your membership!'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as soon as the modal closes, is the profile page updated or do they need to refresh
| } | ||
|
|
||
| function modalContent() { | ||
| if (accessLevel > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the membership state enum in
Line 23 in 35fc855
| const membershipState = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also how about we dont even show the modal button if the user is a valid member?
what do you think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea mr evan ugarte ❤️🩹
for issue #1965