feat(endorsement-system): Export pdf template look #16184
Conversation
…dorsement-system/export-pdf-template-look
…dorsement-system/export-pdf-template-look
…dorsement-system/export-pdf-template-look
WalkthroughThis pull request introduces several modifications to the Endorsements API. Key changes include updates to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16184 +/- ##
==========================================
- Coverage 36.86% 36.66% -0.21%
==========================================
Files 6803 6784 -19
Lines 140646 140479 -167
Branches 39995 40000 +5
==========================================
- Hits 51852 51507 -345
- Misses 88794 88972 +178
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 206 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 4 Total Test Services: 0 Failed, 4 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (1)
|
rafnarnason
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (2)
apps/services/endorsements/api/README.md (1)
46-49: Enhance AWS environment variable setup instructionsGreat addition of AWS environment variable setup instructions. This will help developers with local S3 testing. However, I have a few suggestions to improve these instructions:
Remove the example values to prevent accidental use of placeholder credentials. Instead, use a format like:
export AWS_ACCESS_KEY_ID=your_access_key_id_here export AWS_SECRET_ACCESS_KEY=your_secret_access_key_here export AWS_SESSION_TOKEN=your_session_token_hereAdd guidance on how to obtain these values, e.g., "You can find these values in your AWS account settings or obtain them from your system administrator."
Include a security note, such as: "IMPORTANT: Keep these values secret and never commit them to version control."
Consider adding a link to AWS documentation for more information, e.g., "For more details on AWS credentials, see: https://docs.aws.amazon.com/general/latest/gr/aws-sec-cred-types.html"
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts (1)
608-609: Remove unnecessary empty lines for code clarityThere are multiple empty lines that might not be necessary and could be removed to enhance code readability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (4)
apps/services/endorsements/api/src/assets/ibm-plex-sans-v7-latin-600.ttfis excluded by!**/*.ttfapps/services/endorsements/api/src/assets/ibm-plex-sans-v7-latin-regular.ttfis excluded by!**/*.ttfapps/services/endorsements/api/src/assets/island.pngis excluded by!**/*.pngapps/services/endorsements/api/src/assets/thjodskra.pngis excluded by!**/*.png
📒 Files selected for processing (3)
- apps/services/endorsements/api/README.md (1 hunks)
- apps/services/endorsements/api/src/app/modules/endorsement/endorsement.service.ts (1 hunks)
- apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/services/endorsements/api/README.md (2)
Pattern
apps/services/**/*: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/services/endorsements/api/src/app/modules/endorsement/endorsement.service.ts (2)
Pattern
apps/services/**/*: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts (2)
Pattern
apps/services/**/*: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (4)
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts (4)
31-31: Approved: Importing the 'path' moduleThe import of the
pathmodule is appropriate for constructing file paths in the PDF generation function.
680-685: Adjust Y-position after adding a new pageWhen adding a new page due to content overflow,
currentYPositionis reset to60. Ensure that this aligns with the initial content positioning and that any necessary headers (like table headers) are redrawn correctly.Confirm that resetting
currentYPositionand redrawing headers maintains consistent formatting across pages.
514-514: Ensure all usages of 'createDocumentBuffer' handle the updated return typeThe method
createDocumentBuffernow explicitly returns aPromise<Buffer>. Ensure that all callers of this method handle the promise correctly by usingawaitor proper promise handling to prevent any unexpected behavior.
658-660: Define constants for repeated coordinate valuesDefining constants for the column positions (
dateX,nameX,localityX) enhances readability and makes future adjustments easier. This practice is good and consistent with maintainable code standards.
apps/services/endorsements/api/src/app/modules/endorsement/endorsement.service.ts
Outdated
Show resolved
Hide resolved
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts
Outdated
Show resolved
Hide resolved
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts
Outdated
Show resolved
Hide resolved
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts
Outdated
Show resolved
Hide resolved
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts
Outdated
Show resolved
Hide resolved
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts
Show resolved
Hide resolved
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts (1)
348-348: Review hardcoded values in PDF generationThere are several hardcoded values used for positioning and sizing elements in the PDF. Consider extracting these into named constants or configuration objects to improve readability and make future adjustments easier.
For example:
const HEADER_IMAGE_HEIGHT = 40; const PAGE_MARGIN = 60; const COLUMN_X = { date: 60, name: 160, locality: 360 };Also applies to: 351-351, 446-448
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts (2)
Pattern
apps/services/**/*: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (2)
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts (2)
31-31: LGTM: Import of 'path' moduleThe addition of the 'path' module import is appropriate for handling file paths, which is likely used for asset management in the PDF generation process.
312-315: LGTM: Improved method signatureThe addition of the Promise return type enhances type safety and clarity of the method's purpose.
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts
Outdated
Show resolved
Hide resolved
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts
Outdated
Show resolved
Hide resolved
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts
Outdated
Show resolved
Hide resolved
…dorsement-system/export-pdf-template-look
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts (3)
312-315: Approve method signature with a suggestionThe updated method signature with the explicit return type
Promise<Buffer>improves type safety and readability. However, consider replacing theanytype forendorsementListwith a more specific type, such asEndorsementList, to enhance type checking and code clarity.
344-507: Approve PDF content generation with suggestionsThe PDF content generation is well-structured and comprehensive, including proper pagination handling and good use of font styles. However, consider the following improvements:
- Add null checks or use optional chaining for properties that might be undefined, such as
endorsementList.title,endorsementList.description, etc.- Consider extracting repeated patterns (like setting font and fontSize) into helper functions to reduce code duplication.
- The
endorsementList.endorsements.forEachloop might benefit from usingfor...offor better performance with large datasets.Example of null checking:
.text(endorsementList.title ?? 'Untitled', 60, currentYPosition, { align: 'left', })
505-511: Approve PDF finalization with error handling suggestionThe addition of the footer image and the document finalization are implemented correctly. However, consider adding error handling to catch and properly manage any issues that might occur during PDF generation or buffer creation.
Example:
try { doc.end() return await getStream.buffer(doc) } catch (error) { this.logger.error('Error generating PDF buffer', { error, listId: endorsementList.id }) throw new Error('Failed to generate PDF buffer') }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts (2)
Pattern
apps/services/**/*: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (2)
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts (2)
31-31: LGTM: Import of 'path' moduleThe addition of the 'path' module import is appropriate for handling file paths, which will be useful for working with assets in the PDF generation process.
316-325: LGTM: PDF document initialization and event handlingThe initialization of the PDF document with a consistent margin and the setup of event listeners for 'data' and 'end' events are well-implemented. The success message logging in the 'end' event is a good practice for monitoring and debugging.
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts
Show resolved
Hide resolved
apps/services/endorsements/api/src/app/modules/endorsementList/endorsementList.service.ts
Show resolved
Hide resolved
* new look setup * s3 setup * updated readme * chore: nx format:write update dirty files * finalized layout * chore: nx format:write update dirty files * cleanup * cleanup * cleanup * cleanup * chore: nx format:write update dirty files * trigger --------- Co-authored-by: andes-it <[email protected]>
Improved layout for backend rendered pdf file
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation