-
Notifications
You must be signed in to change notification settings - Fork 17
hotfix: send invitation email validation #1180
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
hotfix: send invitation email validation #1180
Conversation
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
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.
Important
Looks good to me! 👍
Reviewed everything up to d3b77c2 in 1 minute and 22 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/components/organization/invitations/SendInvitationModal.tsx:161
- Draft comment:
The new regex (/^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+.[a-zA-Z]{2,}$/) is simpler but no longer prevents consecutive dots in the local-part. Confirm that allowing addresses like 'john..doe@example.com' is acceptable. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% 1. The code has two layers of email validation - the regex and yup's built-in email validator. 2. The yup .email() validator likely handles consecutive dots appropriately. 3. Consecutive dots in email addresses are technically valid according to RFC 5322, even if uncommon. 4. The comment is speculative, asking for confirmation rather than pointing out a clear issue. I might be underestimating the importance of preventing consecutive dots. There could be security or UX implications I'm not considering. While consecutive dots could be a concern, the presence of yup's email validator provides a second layer of validation, and the comment is phrased as a request for confirmation rather than identifying a clear issue. Delete the comment because it's speculative and asks for confirmation rather than identifying a clear issue. The code already has robust email validation through yup.
Workflow ID: wflow_JTM6XQ5Uf3Wqz0yx
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
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.
Important
Looks good to me! 👍
Reviewed aa7e8ff in 30 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/workflows/deploy.yml:79
- Draft comment:
Confirm 'sovio-studio' is the intended project for Deno Deploy and that corresponding changes (secrets/documentation) are updated. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm their intention regarding the project 'sovio-studio' for Deno Deploy. This violates the rule against asking the PR author to confirm their intention or to ensure that changes are documented or secrets are updated.
Workflow ID: wflow_rA9io08HKQBwdCaV
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Signed-off-by: pranalidhanavade <pranali.dhanavade@ayanworks.com>
|
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.
Important
Looks good to me! 👍
Reviewed c99bdd4 in 1 minute and 17 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/workflows/deploy.yml:79
- Draft comment:
Confirm that updating the Deno Deploy project to 'credebl-studio' is intentional and that its configuration (secrets, settings) is updated accordingly. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment violates several rules. First, it's asking for confirmation ("Confirm that...") which is explicitly against the rules. Second, it's speculative about potential configuration issues. Third, the change appears intentional given all the CREDEBL-specific configuration in the file. Fourth, if there were missing secrets or incorrect configuration, this would fail during deployment anyway. Maybe there could be legitimate configuration concerns that wouldn't be caught by the deployment process. Maybe this is a critical production deployment that needs extra caution. The rules explicitly state not to ask for confirmations or make speculative comments. If there are configuration issues, they will be caught during deployment. This comment should be deleted as it violates multiple rules - it asks for confirmation and makes speculative suggestions about configuration issues.
2. .github/workflows/deploy.yml:81
- Draft comment:
Add a newline at the end of the file to follow best practices. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_2YxN6v3YFQHUnkTh
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.



What
Important
Fix email validation regex in
SendInvitationModal.tsxand update project name indeploy.yml.SendInvitationModal.tsxto/^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/.credebl-uitocredebl-studioindeploy.yml.This description was created by
for c99bdd4. You can customize this summary. It will automatically update as commits are pushed.