-
Notifications
You must be signed in to change notification settings - Fork 365
Add textual signature to generated PDFs #1514
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: main
Are you sure you want to change the base?
Conversation
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.
@ershad _a Could you have a look please? Here's a sample PDF:
| head :created | ||
| end | ||
|
|
||
| def signed |
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.
TRPC calls this endpoint to generate a PDF for the signed document.
| @@ -0,0 +1,108 @@ | |||
| <style> | |||
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.
| await documentsFactory.create({ companyId: company.id, text: "Test document text" }); | ||
| const { user: recipient } = await usersFactory.create({ legalName: "Recipient 1" }); | ||
| await companyContractorsFactory.create({ companyId: company.id, userId: recipient.id }); | ||
| await companyContractorsFactory.create({ companyId: company.id, userId: recipient.id }, { withoutContract: true }); |
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.
Makes the below cleaner as otherwise the user already has a contract.
| .leftJoin(activeStorageBlobs, eq(activeStorageAttachments.blobId, activeStorageBlobs.id)) | ||
| .where(where) | ||
| .orderBy(desc(documents.id)); | ||
| .orderBy(desc(documents.id), desc(activeStorageAttachments.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.
To match document.live_attachment.
|
|
||
| def active? = deactivated_at.nil? | ||
|
|
||
| def primary_admin = super || company_administrators.first |
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.
Falling back to the current behaviour if the field isn't set.
ershad
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.
@MayaRainer _a added a few comments, please take a look.
| end | ||
|
|
||
| def signed | ||
| document = Document.find(params[: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.
to be safe -
| document = Document.find(params[:id]) | |
| document = Current.company.documents.find(params[:id]) |
| const response = await fetch(signed_company_document_url(ctx.company.externalId, document.documents.id), { | ||
| method: "POST", | ||
| headers: ctx.headers, | ||
| }); |
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.
When we make this API call, we are still inside the DB transaction, and the signedAt update hasn't been committed yet. Could be why I saw an empty field for contractor Date signed in the PDF when tested locally. But if the job is delayed for some reason, we won't see this issue. Could you please check? Ideally, we need to generate the doc outside the transaction.
| t.jsonb "json_data", default: {"flags" => []}, null: false | ||
| t.boolean "equity_enabled", default: false, null: false | ||
| t.string "invite_link" | ||
| t.bigint "primary_admin_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.
We need to add this to frontend/db/schema.ts as well?
|
|
||
| def active? = deactivated_at.nil? | ||
|
|
||
| def primary_admin = super || company_administrators.first |
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.
To be consistent with the previous behavior:
| def primary_admin = super || company_administrators.first | |
| def primary_admin = super || company_administrators.order(:id).first |

As per internal discussion, PDFs generated when signing documents should contain a textual signature of both parties. This PR adds that.