Skip to content
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

fixing referrals #75

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

fixing referrals #75

wants to merge 8 commits into from

Conversation

parth-agrawal
Copy link
Contributor

@parth-agrawal parth-agrawal commented Aug 23, 2024

  • working on referrals
  • almost there but need to update middleware for redirect if user has referral applied
  • referrals working brace for merge conflicts
  • fix logic and stuff
  • updated applypage

🚀 This description was created by Ellipsis for commit 66475d5

Summary:

This pull request enhances the referral system by adding backend endpoints for referral management, updating the database schema, and modifying the frontend to handle referral links and user authentication.

Key points:

  • Added /referral/code/:userId and /referral/apply endpoints in backend/lib/controllers/referrals/controller.ts for referral code generation and application.
  • Updated backend/prisma/schema.prisma to include count and usageLimit fields in Referral model.
  • Modified backend/middleware/auth.ts to include referral logic in user authentication.
  • Updated frontend/src/components/base/Referral.tsx to fetch and display referral codes.
  • Enhanced frontend/src/components/pages/UnprotectedHomepage.tsx to handle referral links from URL.
  • Adjusted frontend/src/components/layouts/UnprotectedLayout.tsx to apply referral codes on user login.
  • Updated frontend/network/api.ts to include referral-related API functions.

Generated with ❤️ by ellipsis.dev

@parth-agrawal parth-agrawal changed the title referral3 fixing referrals Aug 23, 2024
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 66475d5 in 46 seconds

More details
  • Looked at 1285 lines of code in 25 files
  • Skipped 1 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. backend/lib/controllers/referrals/controller.ts:7
  • Draft comment:
    Ensure that the userId from the request parameters is validated and authorized to prevent unauthorized access.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is speculative and suggests a potential improvement rather than pointing out a definite issue. It does not align with the rules as it asks to ensure something rather than pointing out a clear code change required. The comment is about a change made in the diff, but it is not a definite issue.
    The comment could be seen as a valid security concern, which might justify its presence. However, it does not point out a specific issue with the current code but rather suggests a best practice.
    While security is important, the comment does not point out a specific flaw in the current implementation. It is more of a general suggestion, which does not align with the rules for review comments.
    The comment should be removed as it is speculative and does not point out a definite issue with the code change.
2. backend/lib/services/User/service.ts:18
  • Draft comment:
    Add validation for the input data in the createUser function to ensure data integrity.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is relevant to the changes made in the diff, as it addresses the createUser function, which was modified. Adding validation is a reasonable suggestion to ensure data integrity, which is a code quality improvement. The comment is actionable and clear, making it a useful suggestion.
    The comment does not specify what kind of validation should be added, which might make it less actionable. However, the general suggestion to add validation is still a valid point for improving code quality.
    While the comment could be more specific, the suggestion to add validation is a common and useful practice in software development, especially when dealing with user input.
    The comment should be kept as it is relevant to the changes made in the diff and suggests a useful code quality improvement by adding validation.
3. frontend/src/components/base/Referral.tsx:29
  • Draft comment:
    Ensure userId is defined before making the API call to fetch the referral code to prevent errors.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The code already includes a check for userId before proceeding with the API call, which addresses the concern raised by the comment. Therefore, the comment is not necessary as the issue is already resolved in the code.
    I might be overlooking a scenario where userId could still be undefined despite the check, but the current logic seems to handle it adequately.
    The logic in the code ensures that userId is only set if it is defined, which should prevent the API call from being made with an undefined userId.
    The comment should be removed as the code already handles the concern raised by the comment, making it redundant.
4. frontend/src/components/compound/SignInbutton.tsx:49
  • Draft comment:
    Consider navigating the user to a different page after a successful sign-in to improve user experience.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    In frontend/src/components/compound/SignInbutton.tsx, the handleSignIn function does not navigate the user after a successful sign-in. This could lead to a confusing user experience.

Workflow ID: wflow_VbE0EgD5kYQCV8j8


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@@ -52,48 +57,69 @@ router.get('/code/:userId', async (req, res) => {

// Apply a referral code when a new user signs up
router.post('/apply', async (req, res) => {
const { newUserId, referralCode } = req.body;
const { userId, referralCode, referrerId } = req.body;
Copy link
Contributor

Choose a reason for hiding this comment

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

Validate that the userId in the request body is authorized to apply the referral to prevent unauthorized applications.

@@ -14,15 +23,15 @@ const app = admin.initializeApp({
credential: admin.credential.cert(serviceAccountKey as admin.ServiceAccount),
});

export const requireAuth = async (req: Request, res: Response, next: NextFunction) => {
export const requireAuth = async (req: AuthenticatedRequest, res: Response, next: NextFunction) => {
const token = req.headers.authorization?.split(" ")[1];
if (!token) {
return res.status(401).json({ message: "Token not found" });
}
const details = await auth.verifyIdToken(token);
Copy link
Contributor

Choose a reason for hiding this comment

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

Add error handling for auth.verifyIdToken and auth.getUser to prevent unhandled promise rejections.

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.

2 participants