Reduce the size of the images within the QR of the invoice #641 - #660
Conversation
WalkthroughA new environment variable, Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
util/index.ts (1)
17-18: Address import syntax inconsistency.The conversion to ES module imports is good, but the file still mixes ES modules with CommonJS requires (lines 13-15). This creates inconsistency in the codebase.
Consider converting all imports to ES modules for consistency:
-const fs = require('fs').promises; -const path = require('path'); -const { I18n } = require('@grammyjs/i18n'); +import { promises as fs } from 'fs'; +import path from 'path'; +import { I18n } from '@grammyjs/i18n';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.env-sample(1 hunks)util/index.ts(3 hunks)
🔇 Additional comments (1)
.env-sample (1)
85-86: LGTM! Well-documented configuration addition.The new environment variable is clearly documented and the default value of 0.2 (20% ratio) aligns perfectly with the PR objective to reduce image size within QR codes.
|
|
||
| // Validate ratio is a valid number between 0.1 and 0.5 | ||
| if (isNaN(imageToQrRatio) || imageToQrRatio < 0.1 || imageToQrRatio > 0.5) { | ||
| logger.warn(`Invalid IMAGE_TO_QR_RATIO value: ${rawRatio}, using default 0.2`); |
There was a problem hiding this comment.
Hi @Luquitasjeffrey You're using logger.warn(...), but since the project uses winston.config.syslog.levels, the correct method is logger.warning(...).
please fix that because as it is now, if the ratio is out of bounds, the QR with the image isn’t generated and this is show in the logs
[2025-06-04T11:38:29.984-06:00] error: logger_1.logger.warn is not a function TypeError: logger_1.logger.warn is not a function
at /home/catry/bot/dist/util/index.js:775:37
at step (/home/catry/bot/dist/util/index.js:33:23)
at Object.next (/home/catry/bot/dist/util/index.js:14:53)
at fulfilled (/home/catry/bot/dist/util/index.js:5:58)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
|
Hi @Luquitasjeffrey how are you progressing with this PR? Let me know if you need any help |
Catrya
left a comment
Catrya
left a comment
There was a problem hiding this comment.
tACK good job @Luquitasjeffrey
3 participants
To reduce the size of the images I added a new configuration environment variable that can set the size of an image relative to the size of the qr image, for instance if IMAGE_TO_QR_RATIO is set to 0.2 that means that imageWidth = 0.2*qrWidth
Summary by CodeRabbit
New Features
Chores