-
Notifications
You must be signed in to change notification settings - Fork 816
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
feat: add recipient roles #716
Conversation
Important Auto Review SkippedAuto reviews are limited to the following labels: coderabbit. Please add one of these labels to enable auto reviews. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
@thehanimo is attempting to deploy a commit to the Documenso Team Team on Vercel. A member of the Team first needs to authorize it. |
Hey there, it looks like you haven't accepted our contributor license agreement yet. In order for us to accept your pull request we ask that you please fill out the CLA: |
I'll have to review this one when I'm back at a computer since it's a bit hard to follow. I understand there's a bit of nuance to this issue so I'll try and get back to you asap |
@@ -72,6 +73,8 @@ export const DataTableActionButton = ({ row }: DataTableActionButtonProps) => { | |||
window.URL.revokeObjectURL(link.href); | |||
}; | |||
|
|||
if (recipient?.role === RecipientRole.CC && isComplete === false) return <div></div>; |
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.
This one sticks out to me as a question but until I run it locally I'm unsure what the question is. Just seems odd to return nothing 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.
This is just assuming that the CC'er shouldn't be able to view the document until it has been completed.
packages/lib/server-only/document/complete-document-with-token.ts
Outdated
Show resolved
Hide resolved
apps/web/src/app/(dashboard)/documents/data-table-action-button.tsx
Outdated
Show resolved
Hide resolved
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.
Noting that we shouldn't be able to add fields for Viewers and Copy'ers
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.
Noting that the CC'er received an invitation to sign the document but they should only receive an email once the document has been completed.
round 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.
Change since last review, now doing the whole PR once more incase I've missed anything before running the UI
packages/lib/server-only/document/complete-document-with-token.ts
Outdated
Show resolved
Hide resolved
Email sending is working as expected although I think we should update the templates to reflect the new roles! A viewer will never sign the document 😄 Likewise when adding fields it'd be nice to know what role a specific recipient has when I select them. |
Do you think a normal font weight would do? What if we went smaller than |
Noting that Eric dropped out but had a really cool thing for recipient selection in #717. He was kind enough to grant permission to use any code you deem useful there as well 😄 |
I'd consider text-xs with a normal font weight. I'd probably also use our |
Noting that this will be the winner, just working through minor items now. |
That is unfortunate :/
It was |
- Updated styling - Updated email templates to render correct roles - Extracted logic to render role verbs and actions
|
GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
---|---|---|---|---|---|
- | Generic Password | a63f560 | .github/workflows/e2e-tests.yml | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Our GitHub checks need improvements? Share your feedbacks!
Fixes #705