Merged
Conversation
|
Warning Rate limit exceeded@tarunnjoshi has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 14 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughFetches venue/address via Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant QrTrait as QrCodeable
participant EntitySvc as Event / EckEntity
participant GD as GD/Image
participant Storage as saveQrCode
Caller->>QrTrait: generateQrCode(entityId, customGroupName, ...)
alt customGroupName == Event_QR
QrTrait->>EntitySvc: load Event -> loc_block_id.address_id
EntitySvc-->>QrTrait: Address
else
QrTrait->>EntitySvc: EckEntity::get('Collection_Camp', FALSE) + fields
EntitySvc-->>QrTrait: Collection_Camp / subtype address fields
end
QrTrait->>GD: render QR (scale=12) -> base64
QrTrait->>GD: decode → imagecreatefromstring → resize (0.7×)
QrTrait->>GD: load uploads logo → resize (~0.5× QR) → preserve alpha
QrTrait->>GD: compose canvas (heading, centered QR, venue text) → output PNG
QrTrait->>Storage: saveQrCode(finalImage, {baseFileName, saveOptions})
Storage-->>QrTrait: success/failure
QrTrait-->>Caller: TRUE / FALSE
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Review notes / best-practice flags
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (2)
22-27: Explicitly disable base64 output to avoid extra decode and binary corruption riskSet QROptions::$outputBase64=false and work with raw PNG bytes directly. This removes the need to strip the data URI and base64_decode, and is faster. In v5 the default is a base64 data URI; this option flips it to raw binary. (github.com, newreleases.io)
Apply:
$options = new QROptions([ 'version' => 5, 'outputType' => QRCode::OUTPUT_IMAGE_PNG, 'eccLevel' => QRCode::ECC_L, 'scale' => 10, + 'outputBase64' => false, ]);
109-113: Catch Throwable, not just ExceptionGD/TypeErrors are Errors, not Exceptions. Catch Throwable to avoid fatals and log more context.
- catch (\Exception $e) { - \CRM_Core_Error::debug_log_message('Error generating QR code: ' . $e->getMessage()); + catch (\Throwable $e) { + \CRM_Core_Error::debug_log_message('Error generating QR code: ' . $e->getMessage());
🧹 Nitpick comments (4)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (4)
69-76: Avoid hardcoded caption; make it configurable or omitUse a supplied caption and default to empty to prevent unexpected user-facing text.
- $text = "Hello here is the qr code"; + $text = (string)($saveOptions['caption'] ?? ''); // Reserve space for text. $fontHeight = 30; - - $canvasHeight = $newLogoHeight + $qrHeight + $fontHeight + 20; + $extraTextHeight = $text !== '' ? $fontHeight : 0; + $canvasHeight = $newLogoHeight + $qrHeight + $extraTextHeight + 20;
85-92: Only render text when providedMinor conditional to avoid empty baseline.
- $black = imagecolorallocate($canvas, 0, 0, 0); - $x = 10; - $y = $newLogoHeight + $qrHeight + 20; - imagestring($canvas, 5, $x, $y, $text, $black); + if ($text !== '') { + $black = imagecolorallocate($canvas, 0, 0, 0); + $x = 10; + $y = $newLogoHeight + $qrHeight + 20; + imagestring($canvas, 5, $x, $y, $text, $black); + }
98-101: Guard resource cleanupOnly destroy if created.
- imagedestroy($qrImage); - imagedestroy($logoImage); - imagedestroy($resizedLogo); - imagedestroy($canvas); + if ($qrImage) { imagedestroy($qrImage); } + if ($logoImage) { imagedestroy($logoImage); } + if ($resizedLogo) { imagedestroy($resizedLogo); } + if ($canvas) { imagedestroy($canvas); }
41-41: Prefix global WP functions for clarity/perf
\wp_upload_dir()is clearer and avoids namespace lookups.- $upload_dir = wp_upload_dir(); + $upload_dir = \wp_upload_dir();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-11T09:53:14.458Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#284
File: wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php:520-520
Timestamp: 2024-10-11T09:53:14.458Z
Learning: In `wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php`, it's acceptable that the `reGenerateCollectionCampQr` method calls `generateQrCode` with only the `$collectionCampId` parameter, as QR codes are generated when the Dropping Center is authorized. We'll implement changes if any issues arise in the future.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
🔇 Additional comments (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (1)
52-67: LGTM on resize math and alpha handling
Comment on lines
+34
to
+38
| // --- Step 1: Create GD image from QR --- | ||
| $qrImage = imagecreatefromstring($qrcode); | ||
| $qrWidth = imagesx($qrImage); | ||
| $qrHeight = imagesy($qrImage); | ||
|
|
There was a problem hiding this comment.
Guard against GD image creation failures to prevent TypeError fatals
imagecreatefromstring() can return false; imagesx()/imagesy() would then throw. Add validation and a clear error.
- $qrImage = imagecreatefromstring($qrcode);
- $qrWidth = imagesx($qrImage);
- $qrHeight = imagesy($qrImage);
+ $qrImage = imagecreatefromstring($qrcode);
+ if (!$qrImage) {
+ throw new \RuntimeException('Failed to create QR image from binary');
+ }
+ $qrWidth = imagesx($qrImage);
+ $qrHeight = imagesy($qrImage);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // --- Step 1: Create GD image from QR --- | |
| $qrImage = imagecreatefromstring($qrcode); | |
| $qrWidth = imagesx($qrImage); | |
| $qrHeight = imagesy($qrImage); | |
| // --- Step 1: Create GD image from QR --- | |
| $qrImage = imagecreatefromstring($qrcode); | |
| if (!$qrImage) { | |
| throw new \RuntimeException('Failed to create QR image from binary'); | |
| } | |
| $qrWidth = imagesx($qrImage); | |
| $qrHeight = imagesy($qrImage); |
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php around
lines 34 to 38, imagecreatefromstring() can return false which will cause
imagesx()/imagesy() to throw TypeErrors; add a guard that checks if $qrImage ===
false (or !is_resource/!is_object depending on GD), and if so free any allocated
resources, log a clear error or throw a descriptive exception (e.g.
InvalidArgumentException with the raw QR data length or GD error), and
return/abort before calling imagesx/imagesy so subsequent code never receives a
boolean.
Comment on lines
+41
to
+48
| $upload_dir = wp_upload_dir(); | ||
| $logoUrl = $upload_dir['baseurl'] . '/2024/09/Goonj-logo-10June20-300x193-1.png'; | ||
| error_log('path: ' . print_r($logoUrl, TRUE)); | ||
|
|
||
| error_log('upload_dir: ' . print_r($upload_dir, TRUE)); | ||
| $logoData = file_get_contents($logoUrl); | ||
| $logoImage = imagecreatefromstring($logoData); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t hardcode logo path or fetch over HTTP; read from filesystem and handle errors
- Hardcoded year/month will break later.
- file_get_contents() over HTTP adds latency/fragility; use uploads basedir path first.
- Remove noisy error_log() calls.
- $upload_dir = wp_upload_dir();
- $logoUrl = $upload_dir['baseurl'] . '/2024/09/Goonj-logo-10June20-300x193-1.png';
- error_log('path: ' . print_r($logoUrl, TRUE));
-
- error_log('upload_dir: ' . print_r($upload_dir, TRUE));
- $logoData = file_get_contents($logoUrl);
- $logoImage = imagecreatefromstring($logoData);
+ $upload_dir = \wp_upload_dir();
+ // Prefer filesystem; allow passing a custom logo path via $saveOptions['logoPath'].
+ $logoPath = $saveOptions['logoPath'] ?? ($upload_dir['basedir'] . '/brand/Goonj-logo.png');
+ if (!\file_exists($logoPath)) {
+ // Fallback to URL if present; otherwise fail fast.
+ $logoUrl = $saveOptions['logoUrl'] ?? null;
+ if (!$logoUrl) {
+ throw new \RuntimeException("Logo not found at $logoPath and no logoUrl provided");
+ }
+ $logoData = \wp_remote_retrieve_body(\wp_remote_get($logoUrl, ['timeout' => 5]));
+ if (!$logoData) {
+ throw new \RuntimeException("Failed to download logo from $logoUrl");
+ }
+ $logoImage = imagecreatefromstring($logoData);
+ } else {
+ $logoImage = imagecreatefrompng($logoPath);
+ }
+ if (!$logoImage) {
+ throw new \RuntimeException('Failed to create logo image resource');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $upload_dir = wp_upload_dir(); | |
| $logoUrl = $upload_dir['baseurl'] . '/2024/09/Goonj-logo-10June20-300x193-1.png'; | |
| error_log('path: ' . print_r($logoUrl, TRUE)); | |
| error_log('upload_dir: ' . print_r($upload_dir, TRUE)); | |
| $logoData = file_get_contents($logoUrl); | |
| $logoImage = imagecreatefromstring($logoData); | |
| $upload_dir = \wp_upload_dir(); | |
| // Prefer filesystem; allow passing a custom logo path via $saveOptions['logoPath']. | |
| $logoPath = $saveOptions['logoPath'] ?? ($upload_dir['basedir'] . '/brand/Goonj-logo.png'); | |
| if (!\file_exists($logoPath)) { | |
| // Fallback to URL if present; otherwise fail fast. | |
| $logoUrl = $saveOptions['logoUrl'] ?? null; | |
| if (!$logoUrl) { | |
| throw new \RuntimeException("Logo not found at $logoPath and no logoUrl provided"); | |
| } | |
| $logoData = \wp_remote_retrieve_body(\wp_remote_get($logoUrl, ['timeout' => 5])); | |
| if (!$logoData) { | |
| throw new \RuntimeException("Failed to download logo from $logoUrl"); | |
| } | |
| $logoImage = imagecreatefromstring($logoData); | |
| } else { | |
| $logoImage = imagecreatefrompng($logoPath); | |
| } | |
| if (!$logoImage) { | |
| throw new \RuntimeException('Failed to create logo image resource'); | |
| } |
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php around
lines 41 to 48, avoid hardcoding the uploads URL/年月 and fetching the logo over
HTTP: build the path using wp_upload_dir()['basedir'] (not baseurl) and a
relative filename or configurable setting, check file_exists and is_readable
before reading, remove the noisy error_log() calls, use file_get_contents on the
local filesystem path (or imagecreatefrompng/gd functions) wrapped in try/if
checks and handle failures by returning/logging a clear error or falling back to
a default image.
Comment on lines
+103
to
+108
| $baseFileName = "qr_code_{$entityId}.png"; | ||
| $saveOptions['baseFileName'] = $baseFileName; | ||
| $saveOptions['entityId'] = $entityId; | ||
|
|
||
| self::saveQrCode($qrcode, $saveOptions); | ||
| self::saveQrCode($finalImage, $saveOptions); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Propagate save result instead of always returning TRUE
Return FALSE if attachment creation fails; optionally return the attachment on success.
- $baseFileName = "qr_code_{$entityId}.png";
+ $baseFileName = "qr_code_{$entityId}.png";
$saveOptions['baseFileName'] = $baseFileName;
$saveOptions['entityId'] = $entityId;
-
- self::saveQrCode($finalImage, $saveOptions);
+ $attachment = self::saveQrCode($finalImage, $saveOptions);
+ if ($attachment === false) {
+ return FALSE;
+ }
...
- return TRUE;
+ return TRUE; // or `return $attachment;` if callers expect the attachment dataIf callers rely on a truthy attachment array, switch the final return to $attachment.
Also applies to: 115-116
🏁 Script executed:
#!/bin/bash
# Locate QrCodeable.php
QR=$(fd QrCodeable.php)
echo "QrCodeable file: $QR"
# Show definition and usage of saveQrCode
rg -n -C3 "saveQrCode" "$QR"Length of output: 807
🏁 Script executed:
#!/bin/bash
sed -n '120,200p' wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.phpLength of output: 2615
Propagate save result instead of always returning TRUE
– In generateQrCode (lines 103–108), assign
$attachment = self::saveQrCode($finalImage, $saveOptions);
if ($attachment === FALSE) {
return FALSE;
}
return $attachment;instead of ignoring the call and returning TRUE.
– Apply the same pattern to the save call at lines 115–116.
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php around
lines 103–108 (and similarly at lines 115–116), the result of self::saveQrCode
is currently ignored and the method always returns TRUE; change both calls to
capture the return (e.g. $attachment = self::saveQrCode($finalImage,
$saveOptions)), check for a FALSE failure (if ($attachment === FALSE) return
FALSE;) and otherwise return the $attachment so callers receive the actual saved
attachment result.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (4)
29-33: Stop double-handling PNG bytes; make output deterministic (raw OR base64, not both).Current code strips a data URI and base64-decodes unconditionally. This only works if render() returns a base64 data URI; if it already returns raw PNG, decoding will corrupt the image. Make it explicit and remove the decode.
Option A (preferred): force raw bytes and drop decoding.
- // Generate QR. - $qrcode = (new QRCode($options))->render($data); - $qrcode = str_replace('data:image/png;base64,', '', $qrcode); - $qrcode = base64_decode($qrcode); + // Generate QR as raw PNG (no data URI, no base64). + $options = new QROptions([ + 'version' => 5, + 'outputType' => QRCode::OUTPUT_IMAGE_PNG, + 'eccLevel' => QRCode::ECC_L, + 'scale' => 10, + 'outputBase64' => false, + ]); + $qrcode = (new QRCode($options))->render($data);Option B: keep base64 flow but set it explicitly and retain the decode.
+ $options = new QROptions([ + // ... + 'outputBase64' => true, + ]); $qrcode = (new QRCode($options))->render($data); $qrcode = str_replace('data:image/png;base64,', '', $qrcode); $qrcode = base64_decode($qrcode);Pick one path; do not mix them.
33-35: Guard GD resource creation before using imagesx()/imagesy().imagecreatefromstring() can return false; calling imagesx/imagesy on it will fatal.
- $qrImage = imagecreatefromstring($qrcode); - $qrWidth = imagesx($qrImage); - $qrHeight = imagesy($qrImage); + $qrImage = imagecreatefromstring($qrcode); + if ($qrImage === false) { + throw new \RuntimeException('Failed to create QR image from binary'); + } + $qrWidth = imagesx($qrImage); + $qrHeight = imagesy($qrImage);
37-44: Don’t hardcode dated uploads path or fetch over HTTP; use filesystem with fallbacks.
- The hardcoded /2024/09 path will break later.
- Avoid HTTP fetch from baseurl; read from basedir and allow an override via $saveOptions.
- Also, call global WP functions with a leading backslash under a namespace.
- // Load logo (from uploads folder URL). - $upload_dir = wp_upload_dir(); - $logoUrl = $upload_dir['baseurl'] . '/2024/09/Goonj-logo-10June20-300x193-1.png'; - $logoData = file_get_contents($logoUrl); - $logoImage = imagecreatefromstring($logoData); + // Load logo from filesystem (configurable), fallback to URL if provided. + $upload_dir = \wp_upload_dir(); + $logoPath = $saveOptions['logoPath'] ?? ($upload_dir['basedir'] . '/brand/Goonj-logo.png'); + if (\file_exists($logoPath)) { + $logoImage = imagecreatefrompng($logoPath); + } elseif (!empty($saveOptions['logoUrl'])) { + $logoData = \wp_remote_retrieve_body(\wp_remote_get($saveOptions['logoUrl'], ['timeout' => 5])); + $logoImage = $logoData ? imagecreatefromstring($logoData) : false; + } else { + $logoImage = false; + } + if ($logoImage === false) { + throw new \RuntimeException('Failed to load logo image'); + } $logoWidth = imagesx($logoImage); $logoHeight = imagesy($logoImage);
102-106: Propagate the save result; don’t always return TRUE.Capture the attachment result and return it (or FALSE on failure). This prevents silent failures and gives callers the file metadata.
$saveOptions['baseFileName'] = $baseFileName; $saveOptions['entityId'] = $entityId; - self::saveQrCode($finalImage, $saveOptions); + $attachment = self::saveQrCode($finalImage, $saveOptions); + if ($attachment === false) { + return false; + } + return $attachment; @@ - return TRUE; + // Unreachable if returning above; if you prefer boolean API, return ($attachment !== false). + return TRUE;If callers expect a boolean today, we can keep the signature and just
return ($attachment !== false);. Please confirm the call sites.Also applies to: 113-113
🧹 Nitpick comments (2)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (2)
45-57: Preserve transparency when resizing the logo; add failure checks.Without pre-filling a fully transparent background, edges may appear with dark halos; also check allocation succeeded.
$resizedLogo = imagecreatetruecolor($newLogoWidth, $newLogoHeight); - imagealphablending($resizedLogo, FALSE); - imagesavealpha($resizedLogo, TRUE); + if ($resizedLogo === false) { + throw new \RuntimeException('Failed to allocate resized logo canvas'); + } + imagealphablending($resizedLogo, false); + imagesavealpha($resizedLogo, true); + $transparent = imagecolorallocatealpha($resizedLogo, 0, 0, 0, 127); + imagefill($resizedLogo, 0, 0, $transparent);
63-71: Avoid magic numbers for text width; center using GD font metrics.Using 9 px per char is brittle and breaks with different fonts/strings. Use imagefontwidth()/imagefontheight().
- $canvasWidth = $qrWidth + 100; - $canvasHeight = $newLogoHeight + $qrHeight + 160; + $font = 5; + $fw = \imagefontwidth($font); + $fh = \imagefontheight($font); + $canvasWidth = $qrWidth + 100; + $canvasHeight = $newLogoHeight + 20 /*logo gap*/ + + $fh /*top text*/ + + 40 /*gap*/ + + $qrHeight + + 30 /*gap*/ + + $fh /*bottom text*/ + + 10; @@ - $topY = $logoY + $newLogoHeight + 20; - $topX = (int) (($canvasWidth - (strlen($topText) * 9)) / 2); - imagestring($canvas, 5, $topX, $topY, $topText, $black); + $topY = $logoY + $newLogoHeight + 20; + $topX = (int)(($canvasWidth - ($fw * (function_exists('mb_strlen') ? mb_strlen($topText) : strlen($topText)))) / 2); + imagestring($canvas, $font, $topX, $topY, $topText, $black); @@ - $bottomY = $qrY + $qrHeight + 30; - $bottomX = (int) (($canvasWidth - (strlen($bottomText) * 9)) / 2); - imagestring($canvas, 5, $bottomX, $bottomY, $bottomText, $black); + $bottomY = $qrY + $qrHeight + 30; + $bottomX = (int)(($canvasWidth - ($fw * (function_exists('mb_strlen') ? mb_strlen($bottomText) : strlen($bottomText)))) / 2); + imagestring($canvas, $font, $bottomX, $bottomY, $bottomText, $black);Optional: switch to imagettftext() for better typography.
Also applies to: 77-81, 87-91
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-11T09:53:14.458Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#284
File: wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php:520-520
Timestamp: 2024-10-11T09:53:14.458Z
Learning: In `wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php`, it's acceptable that the `reGenerateCollectionCampQr` method calls `generateQrCode` with only the `$collectionCampId` parameter, as QR codes are generated when the Dropping Center is authorized. We'll implement changes if any issues arise in the future.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
📚 Learning: 2024-10-09T15:24:42.859Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#352
File: wp-content/civi-extensions/goonjcustom/Civi/QrCodeService.php:51-53
Timestamp: 2024-10-09T15:24:42.859Z
Learning: In `QrCodeService::generateQrCode`, when using `QRCode::OUTPUT_IMAGE_PNG`, the `render` method returns a data URI with base64-encoded image data. Therefore, it's necessary to remove the `data:image/png;base64,` prefix and base64-decode the data to obtain the raw binary image.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
🔇 Additional comments (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (1)
97-100: Resource cleanup looks good.
Comment on lines
+59
to
+62
| // Texts. | ||
| $topText = "Scan to Record Your Contribution"; | ||
| $bottomText = "Venue: Gaur Saundaryam Society, Greater Noida"; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hard-coded, user-facing text; make it dynamic (and translatable if needed).
Venue and CTA should come from data/options, not hardcoded to a specific society.
- $topText = "Scan to Record Your Contribution";
- $bottomText = "Venue: Gaur Saundaryam Society, Greater Noida";
+ $topText = $saveOptions['topText'] ?? 'Scan to Record Your Contribution';
+ $bottomText = $saveOptions['bottomText'] ?? ($saveOptions['venue'] ?? '');If i18n applies, wrap defaults with the project’s translation helper.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Texts. | |
| $topText = "Scan to Record Your Contribution"; | |
| $bottomText = "Venue: Gaur Saundaryam Society, Greater Noida"; | |
| // Texts. | |
| - $topText = "Scan to Record Your Contribution"; | |
| // Allow overriding via saveOptions; fallback to default strings (wrap in __() for i18n if needed). | |
| $topText = $saveOptions['topText'] ?? 'Scan to Record Your Contribution'; | |
| $bottomText = $saveOptions['bottomText'] ?? ($saveOptions['venue'] ?? ''); |
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php around
lines 59-62, the topText and bottomText are hard-coded; change them to pull
values from the relevant contribution/options data or extension settings (e.g.,
QR label/venue fields) and only fall back to defaults when those are empty. Also
wrap the fallback default strings with the project’s translation helper (use the
CiviCRM translation helper) so they are translatable. Validate and sanitize the
retrieved values before use and ensure tests/defaults cover both populated and
empty-settings cases.
Comment on lines
+92
to
+96
| // Save final. | ||
| ob_start(); | ||
| imagepng($canvas); | ||
| $finalImage = ob_get_clean(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate PNG buffer before saving.
If imagepng/ob_get_clean fails, we’ll try to save an empty file and still return TRUE later.
ob_start();
- imagepng($canvas);
- $finalImage = ob_get_clean();
+ $ok = imagepng($canvas);
+ $finalImage = ob_get_clean();
+ if (!$ok || $finalImage === '' || $finalImage === false) {
+ throw new \RuntimeException('Failed to render composite PNG');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Save final. | |
| ob_start(); | |
| imagepng($canvas); | |
| $finalImage = ob_get_clean(); | |
| // Save final. | |
| ob_start(); | |
| $ok = imagepng($canvas); | |
| $finalImage = ob_get_clean(); | |
| if (!$ok || $finalImage === '' || $finalImage === false) { | |
| throw new \RuntimeException('Failed to render composite PNG'); | |
| } |
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php around
lines 92-96, the PNG data produced by imagepng is taken from the output buffer
without validating it; if imagepng or ob_get_clean fails we may write an empty
file and still return true. Fix by capturing the buffer with
ob_start/ob_get_contents and ob_end_clean, check the return value of imagepng
(it returns true/false) and ensure the buffer is a non-empty string before
calling file_put_contents; if either imagepng failed or the buffer is empty, do
not write the file and return/throw an error (or return false) so the caller
knows the save failed.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (6)
23-28: Use raw PNG output; drop data-URI stripping and base64 decodeSet
outputBase64 => falseand use the renderer’s raw bytes. Removes unnecessary transform and avoids accidental double-decode.$options = new QROptions([ - 'version' => 5, - 'outputType' => QRCode::OUTPUT_IMAGE_PNG, - 'eccLevel' => QRCode::ECC_L, + 'version' => 0, // let library pick best version for data + 'outputType' => QRCode::OUTPUT_IMAGE_PNG, + 'outputBase64' => false, + 'eccLevel' => QRCode::ECC_M, 'scale' => 10, ]); @@ - $qrcode = (new QRCode($options))->render($data); - $qrcode = str_replace('data:image/png;base64,', '', $qrcode); - $qrcode = base64_decode($qrcode); + $qrcode = (new QRCode($options))->render($data); // raw PNG bytesRun to confirm the library supports
outputBase64in your version:#!/bin/bash # Show installed chillerlan/php-qrcode version and whether QROptions has outputBase64 fd composer.lock -t f -x jq -r '.packages[]?|select(.name=="chillerlan/php-qrcode")|.version' {} 2>/dev/null rg -n --no-heading 'class\s+QROptions|outputBase64' vendor 2>/dev/null | sed -n '1,50p'Also applies to: 37-39
66-69: Hard-coded user-facing text; make dynamic (and translatable if applicable)Let callers pass text/venue; default sensibly.
- $topText = "Scan to Record Your Contribution"; - $bottomText = "Venue: $collectionCampAddress"; + $topText = $saveOptions['topText'] ?? \CRM_Core_OptionGroup::getLabel('goonj_qr', 'top_text') ?? 'Scan to Record Your Contribution'; + $bottomText = $saveOptions['bottomText'] ?? ($saveOptions['venue'] ?? ($collectionCampAddress ? "Venue: $collectionCampAddress" : ''));
40-42: Guard GD decode; avoid TypeError on imagesx/imagesy when decode fails
imagecreatefromstring()can return false; check before measuring.- $qrImage = imagecreatefromstring($qrcode); - $qrWidth = imagesx($qrImage); - $qrHeight = imagesy($qrImage); + $qrImage = imagecreatefromstring($qrcode); + if (!$qrImage) { + throw new \RuntimeException('Failed to create QR image from binary'); + } + $qrWidth = imagesx($qrImage); + $qrHeight = imagesy($qrImage);
44-51: Don’t hardcode uploads year/month or fetch logo over HTTP; use filesystem with fallbackPrefer
basedirpath (fast, reliable), make it configurable, and handle failures explicitly.- // Load logo (from uploads folder URL). - $upload_dir = wp_upload_dir(); - $logoUrl = $upload_dir['baseurl'] . '/2024/09/Goonj-logo-10June20-300x193-1.png'; - $logoData = file_get_contents($logoUrl); - $logoImage = imagecreatefromstring($logoData); + // Load logo (prefer filesystem; allow overrides). + $upload_dir = \wp_upload_dir(); + $logoPath = $saveOptions['logoPath'] ?? ($upload_dir['basedir'] . '/brand/Goonj-logo.png'); + if (\file_exists($logoPath) && \is_readable($logoPath)) { + $logoImage = \imagecreatefrompng($logoPath); + } else { + $logoUrl = $saveOptions['logoUrl'] ?? null; + if (!$logoUrl) { + throw new \RuntimeException("Logo not found at $logoPath and no logoUrl provided"); + } + $resp = \wp_remote_get($logoUrl, ['timeout' => 5]); + $logoData = \wp_remote_retrieve_body($resp); + if (!$logoData) { + throw new \RuntimeException("Failed to download logo from $logoUrl"); + } + $logoImage = \imagecreatefromstring($logoData); + } + if (!$logoImage) { + throw new \RuntimeException('Failed to create logo image resource'); + }
99-103: Validate the PNG buffer before savingAvoid writing empty/invalid files if GD or buffering fails.
// Save final. ob_start(); - imagepng($canvas); - $finalImage = ob_get_clean(); + $ok = \imagepng($canvas); + $finalImage = ob_get_clean(); + if (!$ok || $finalImage === '' || $finalImage === false) { + throw new \RuntimeException('Failed to render composite PNG'); + }
109-113: Return the actual attachment (or FALSE) instead of always TRUEPropagate
saveQrCode()result so callers can inspect failures and metadata.- self::saveQrCode($finalImage, $saveOptions); + $attachment = self::saveQrCode($finalImage, $saveOptions); + if ($attachment === FALSE) { + return FALSE; + } + return $attachment; @@ - return TRUE; + return TRUE; // unreachable after early return above; remove if returning $attachmentIf existing callers expect boolean, keep boolean but at least return FALSE on failure paths.
Also applies to: 120-120
🧹 Nitpick comments (3)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (3)
52-64: Bounds-check resized logo; prevent 0/oversized dimensionsHandle tiny/huge logos gracefully and keep margins around the QR.
- // Resize logo (smaller). - $scaleFactor = 0.5; - $newLogoWidth = (int) ($qrWidth * $scaleFactor); - $newLogoHeight = (int) ($logoHeight * ($newLogoWidth / $logoWidth)); + // Resize logo (keep within 25% of QR width, min 32px, leave 40px margin). + $maxLogoWidth = max(32, min((int)($qrWidth * 0.25), max(32, $qrWidth - 40))); + $newLogoWidth = (int)$maxLogoWidth; + $newLogoHeight = (int) max(16, ($logoWidth > 0 ? ($logoHeight * ($newLogoWidth / $logoWidth)) : 0));
104-107: Ensure resources are destroyed in a finally blockIf an exception fires before these lines, resources leak for this request. Wrap cleanup in
finally.Example:
try { // ... } catch (\Exception $e) { // ... } finally { foreach ([$qrImage ?? null, $logoImage ?? null, $resizedLogo ?? null, $canvas ?? null] as $im) { if (\is_resource($im) || (is_object($im) && $im instanceof \GdImage)) { \imagedestroy($im); } } }
45-46: Prefix global functions to skip namespace fallbackMinor perf/readability win; apply consistently (
\wp_upload_dir,\file_put_contents, etc.).- $upload_dir = wp_upload_dir(); + $upload_dir = \wp_upload_dir(); @@ - $numBytes = file_put_contents($tempFilePath, $qrcode); + $numBytes = \file_put_contents($tempFilePath, $qrcode);Also applies to: 135-136
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-11T09:53:14.458Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#284
File: wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php:520-520
Timestamp: 2024-10-11T09:53:14.458Z
Learning: In `wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php`, it's acceptable that the `reGenerateCollectionCampQr` method calls `generateQrCode` with only the `$collectionCampId` parameter, as QR codes are generated when the Dropping Center is authorized. We'll implement changes if any issues arise in the future.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
📚 Learning: 2024-10-09T15:24:42.859Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#352
File: wp-content/civi-extensions/goonjcustom/Civi/QrCodeService.php:51-53
Timestamp: 2024-10-09T15:24:42.859Z
Learning: In `QrCodeService::generateQrCode`, when using `QRCode::OUTPUT_IMAGE_PNG`, the `render` method returns a data URI with base64-encoded image data. Therefore, it's necessary to remove the `data:image/png;base64,` prefix and base64-decode the data to obtain the raw binary image.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
Comment on lines
+30
to
+36
| $collectionCamp = EckEntity::get('Collection_Camp', FALSE) | ||
| ->addSelect('Collection_Camp_Intent_Details.Location_Area_of_camp') | ||
| ->addWhere('id', '=', $entityId) | ||
| ->execute()->first(); | ||
|
|
||
| // Remove the base64 header and decode the image data. | ||
| $collectionCampAddress = $collectionCamp['Collection_Camp_Intent_Details.Location_Area_of_camp']; | ||
| // Generate QR. |
There was a problem hiding this comment.
Null-check the camp lookup to avoid runtime fatal on missing entity
first() may return null; indexing into it will throw. Bail out early with a clear log.
- $collectionCamp = EckEntity::get('Collection_Camp', FALSE)
- ->addSelect('Collection_Camp_Intent_Details.Location_Area_of_camp')
- ->addWhere('id', '=', $entityId)
- ->execute()->first();
-
- $collectionCampAddress = $collectionCamp['Collection_Camp_Intent_Details.Location_Area_of_camp'];
+ $collectionCamp = EckEntity::get('Collection_Camp', FALSE)
+ ->addSelect('Collection_Camp_Intent_Details.Location_Area_of_camp')
+ ->addWhere('id', '=', $entityId)
+ ->execute()
+ ->first();
+ if (!$collectionCamp) {
+ \CRM_Core_Error::debug_log_message("Collection Camp not found for id {$entityId}");
+ return FALSE;
+ }
+ $collectionCampAddress = $collectionCamp['Collection_Camp_Intent_Details.Location_Area_of_camp'] ?? '';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $collectionCamp = EckEntity::get('Collection_Camp', FALSE) | |
| ->addSelect('Collection_Camp_Intent_Details.Location_Area_of_camp') | |
| ->addWhere('id', '=', $entityId) | |
| ->execute()->first(); | |
| // Remove the base64 header and decode the image data. | |
| $collectionCampAddress = $collectionCamp['Collection_Camp_Intent_Details.Location_Area_of_camp']; | |
| // Generate QR. | |
| $collectionCamp = EckEntity::get('Collection_Camp', FALSE) | |
| ->addSelect('Collection_Camp_Intent_Details.Location_Area_of_camp') | |
| ->addWhere('id', '=', $entityId) | |
| ->execute() | |
| ->first(); | |
| if (!$collectionCamp) { | |
| \CRM_Core_Error::debug_log_message("Collection Camp not found for id {$entityId}"); | |
| return FALSE; | |
| } | |
| $collectionCampAddress = $collectionCamp['Collection_Camp_Intent_Details.Location_Area_of_camp'] ?? ''; | |
| // Generate QR. |
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php around
lines 30 to 36, the result of ->first() may be null and the subsequent array
access on $collectionCamp will cause a fatal; add a null check after ->first()
and if null, log a clear message (including $entityId) and return/throw early to
avoid proceeding, otherwise continue to access the address field.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (8)
30-35: Fix potential null pointer exception when collection camp is not foundThe
first()method can return null if no collection camp is found for the given ID, which will cause a fatal error when trying to access array keys on line 35.Based on previous review feedback, this issue was already identified. Add a null check after
->first()and return early with proper error logging if the collection camp is not found.$collectionCamp = EckEntity::get('Collection_Camp', FALSE) ->addSelect('Collection_Camp_Intent_Details.Location_Area_of_camp') ->addWhere('id', '=', $entityId) - ->execute()->first(); + ->execute() + ->first(); + if (!$collectionCamp) { + \CRM_Core_Error::debug_log_message("Collection Camp not found for id {$entityId}"); + return FALSE; + } - $collectionCampAddress = $collectionCamp['Collection_Camp_Intent_Details.Location_Area_of_camp']; + $collectionCampAddress = $collectionCamp['Collection_Camp_Intent_Details.Location_Area_of_camp'] ?? '';
37-42: Remove data URI processing since outputBase64 is not set to falseThe code is still stripping the data URI prefix and base64-decoding the QR code output, but this will corrupt the PNG data when
outputBase64is not explicitly set to false.- // Generate QR. - $qrcode = (new QRCode($options))->render($data); - $qrcode = str_replace('data:image/png;base64,', '', $qrcode); - $qrcode = base64_decode($qrcode); + // Generate QR as raw PNG binary. + $options = new QROptions([ + 'version' => 5, + 'outputType' => QRCode::OUTPUT_IMAGE_PNG, + 'outputBase64' => false, + 'eccLevel' => QRCode::ECC_L, + 'scale' => 10, + ]); + $qrcode = (new QRCode($options))->render($data); $qrImage = imagecreatefromstring($qrcode);
40-42: Add validation for image creation to prevent fatal errors
imagecreatefromstring()can return false, which will causeimagesx()andimagesy()to throw TypeError exceptions.$qrImage = imagecreatefromstring($qrcode); + if (!$qrImage) { + throw new \RuntimeException('Failed to create QR image from binary data'); + } $qrWidth = imagesx($qrImage); $qrHeight = imagesy($qrImage);
44-50: Replace hardcoded logo path and HTTP fetch with filesystem approachThe hardcoded path with year/month will break in future months, and fetching over HTTP adds unnecessary latency and potential failure points.
- // Load logo (from uploads folder URL). - $upload_dir = wp_upload_dir(); - $logoUrl = $upload_dir['baseurl'] . '/2024/09/Goonj-logo-10June20-300x193-1.png'; - $logoData = file_get_contents($logoUrl); - $logoImage = imagecreatefromstring($logoData); + // Load logo from filesystem + $upload_dir = \wp_upload_dir(); + $logoPath = $saveOptions['logoPath'] ?? ($upload_dir['basedir'] . '/brand/Goonj-logo.png'); + + if (!\file_exists($logoPath)) { + $logoUrl = $saveOptions['logoUrl'] ?? null; + if (!$logoUrl) { + throw new \RuntimeException("Logo not found at $logoPath and no logoUrl provided"); + } + $logoResponse = \wp_remote_get($logoUrl, ['timeout' => 5]); + if (\is_wp_error($logoResponse)) { + throw new \RuntimeException("Failed to download logo from $logoUrl: " . $logoResponse->get_error_message()); + } + $logoData = \wp_remote_retrieve_body($logoResponse); + $logoImage = imagecreatefromstring($logoData); + } else { + $logoImage = imagecreatefrompng($logoPath); + } + + if (!$logoImage) { + throw new \RuntimeException('Failed to create logo image resource'); + }
67-69: Make text content configurable instead of hardcodedThe text content should be configurable through parameters rather than hardcoded, especially the venue information which may vary.
- // Texts. - // force a line break. - $topText = "Scan to Record Your\nContribution"; - $bottomText = "Venue: $collectionCampAddress"; + // Texts - make configurable + $topText = $saveOptions['topText'] ?? "Scan to Record Your\nContribution"; + $bottomText = $saveOptions['bottomText'] ?? "Venue: $collectionCampAddress";
111-112: Fix inconsistent text centering approachThe bottom text still uses the brittle
strlen() * 9approach while the top text uses proper TTF measurement, creating inconsistent text positioning.- $bottomY = $qrY + $qrHeight + 30; - $bottomX = (int) (($canvasWidth - (strlen($bottomText) * 9)) / 2); - imagestring($canvas, 5, $bottomX, $bottomY, $bottomText, $black); + $bottomY = $qrY + $qrHeight + 30; + $bbox = imagettfbbox($fontSize, 0, $fontPath, $bottomText); + $textWidth = abs($bbox[2] - $bbox[0]); + $bottomX = (int) (($canvasWidth - $textWidth) / 2); + imagettftext($canvas, $fontSize, 0, $bottomX, $bottomY, $black, $fontPath, $bottomText);
115-117: Add validation for PNG rendering to prevent saving empty filesThe code doesn't validate whether
imagepng()succeeded or if the output buffer contains valid data before attempting to save.ob_start(); - imagepng($canvas); - $finalImage = ob_get_clean(); + $ok = imagepng($canvas); + $finalImage = ob_get_clean(); + if (!$ok || $finalImage === '' || $finalImage === false) { + throw new \RuntimeException('Failed to render composite PNG'); + }
127-127: Propagate save result instead of ignoring return valueThe code ignores the return value from
saveQrCode()and always returns TRUE, masking potential save failures from the caller.- self::saveQrCode($finalImage, $saveOptions); + $attachment = self::saveQrCode($finalImage, $saveOptions); + if ($attachment === FALSE) { + return FALSE; + }And update the final return to propagate the actual result:
- return TRUE; + return $attachment;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-11T09:53:14.458Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#284
File: wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php:520-520
Timestamp: 2024-10-11T09:53:14.458Z
Learning: In `wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php`, it's acceptable that the `reGenerateCollectionCampQr` method calls `generateQrCode` with only the `$collectionCampId` parameter, as QR codes are generated when the Dropping Center is authorized. We'll implement changes if any issues arise in the future.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
📚 Learning: 2024-10-09T15:24:42.859Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#352
File: wp-content/civi-extensions/goonjcustom/Civi/QrCodeService.php:51-53
Timestamp: 2024-10-09T15:24:42.859Z
Learning: In `QrCodeService::generateQrCode`, when using `QRCode::OUTPUT_IMAGE_PNG`, the `render` method returns a data URI with base64-encoded image data. Therefore, it's necessary to remove the `data:image/png;base64,` prefix and base64-decode the data to obtain the raw binary image.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
Comment on lines
+86
to
+87
| $fontPath = dirname(__DIR__, 2) . '/fonts/DejaVuSans-Bold.ttf'; | ||
| $fontSize = 28; |
There was a problem hiding this comment.
Verify font file exists before using TTF functions
The code assumes the TTF font file exists at the hardcoded path, which could cause fatal errors if the file is missing.
// --- Step 2: Place heading text below logo using TTF font
$fontPath = dirname(__DIR__, 2) . '/fonts/DejaVuSans-Bold.ttf';
+ if (!file_exists($fontPath)) {
+ throw new \RuntimeException("TTF font file not found at: $fontPath");
+ }
$fontSize = 28;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $fontPath = dirname(__DIR__, 2) . '/fonts/DejaVuSans-Bold.ttf'; | |
| $fontSize = 28; | |
| // --- Step 2: Place heading text below logo using TTF font | |
| $fontPath = dirname(__DIR__, 2) . '/fonts/DejaVuSans-Bold.ttf'; | |
| if (!file_exists($fontPath)) { | |
| throw new \RuntimeException("TTF font file not found at: $fontPath"); | |
| } | |
| $fontSize = 28; |
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php around
lines 86-87, the code blindly assumes the TTF at dirname(__DIR__, 2) .
'/fonts/DejaVuSans-Bold.ttf' exists; add a pre-check using
file_exists/is_readable for that path and if it’s missing fall back to a safe
alternative (use a bundled default font path, use a non-TTF renderer, or skip
TTF-specific functions) and log or throw a controlled exception so the app
doesn’t fatally error; ensure any code that calls TTF functions only runs when
the font check passes and adjust fontSize or behavior appropriately in the
fallback branch.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (1)
187-197: Propagate save result; update callers
InCivi/Traits/QrCodeable.php(lines 187–197), capture the return value ofself::saveQrCode($finalImage, $saveOptions)and return the created attachment (orFALSEon failure) instead of unconditionally returningTRUE. Then update allgenerateQrCode()call sites (e.g. inInstitutionCollectionCampService.php,InstitutionGoonjActivitiesService.php,GoonjInitiatedEventsService.php, etc.) to assign the return value and handle failures appropriately.
♻️ Duplicate comments (5)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (5)
38-45: Stop stripping data URI and base64-decoding; use raw PNG bytes from render()// Generate QR. $qrcode = (new QRCode($options))->render($data); - $qrcode = str_replace('data:image/png;base64,', '', $qrcode); - $qrcode = base64_decode($qrcode); $qrImage = imagecreatefromstring($qrcode);
31-37: Null-check the EckEntity fetch to avoid fatal on missing record
->first()may return null; array access on$collectionCamp[...]will error. Bail out cleanly.- $collectionCamp = EckEntity::get('Collection_Camp', FALSE) + $collectionCamp = EckEntity::get('Collection_Camp', FALSE) ->addSelect('Collection_Camp_Intent_Details.Location_Area_of_camp') ->addWhere('id', '=', $entityId) - ->execute()->first(); + ->execute()->first(); - $collectionCampAddress = $collectionCamp['Collection_Camp_Intent_Details.Location_Area_of_camp']; + if (!$collectionCamp) { + \CRM_Core_Error::debug_log_message("Collection Camp not found for id {$entityId}"); + return FALSE; + } + $collectionCampAddress = $collectionCamp['Collection_Camp_Intent_Details.Location_Area_of_camp'] ?? '';
42-45: Guard GD resource creation to prevent TypeError fatals
imagecreatefromstring()can return false; validate beforeimagesx()/imagesy().- $qrImage = imagecreatefromstring($qrcode); - $qrWidth = imagesx($qrImage); - $qrHeight = imagesy($qrImage); + $qrImage = imagecreatefromstring($qrcode); + if (!$qrImage) { + throw new \RuntimeException('Failed to create QR image from binary'); + } + $qrWidth = imagesx($qrImage); + $qrHeight = imagesy($qrImage);
176-180: Validate PNG buffer before savingAvoid writing empty/corrupt files if
imagepng()or buffering fails.// Save final. ob_start(); - imagepng($canvas); - $finalImage = ob_get_clean(); + $ok = imagepng($canvas); + $finalImage = ob_get_clean(); + if (!$ok || $finalImage === '' || $finalImage === false) { + throw new \RuntimeException('Failed to render composite PNG'); + }
62-69: Don’t fetch the logo over HTTP; read from filesystem, add fallbacks and checksHardcoded uploads Y/M and HTTP fetch are brittle; prefer
basedirand validate GD resource.- $upload_dir = wp_upload_dir(); - $logoUrl = $upload_dir['baseurl'] . '/2024/09/Goonj-logo-10June20-300x193-1.png'; - $logoData = file_get_contents($logoUrl); - $logoImage = imagecreatefromstring($logoData); + $upload_dir = \wp_upload_dir(); + $logoPath = $saveOptions['logoPath'] ?? ($upload_dir['basedir'] . '/brand/Goonj-logo.png'); + if (\file_exists($logoPath)) { + $logoImage = imagecreatefrompng($logoPath); + } else { + $logoUrl = $saveOptions['logoUrl'] ?? null; + if (!$logoUrl) { + throw new \RuntimeException("Logo not found at $logoPath and no logoUrl provided"); + } + $resp = \wp_remote_get($logoUrl, ['timeout' => 5]); + $logoData = \wp_remote_retrieve_body($resp); + $logoImage = $logoData ? imagecreatefromstring($logoData) : false; + } + if (!$logoImage) { + throw new \RuntimeException('Failed to create logo image resource'); + } $logoWidth = imagesx($logoImage); $logoHeight = imagesy($logoImage);
🧹 Nitpick comments (5)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (5)
23-29: Explicitly disable base64 output; consider stronger ECCSet
outputBase64 => falseto ensurerender()returns raw PNG; considerECC_Qfor resilience.$options = new QROptions([ 'version' => 5, 'outputType' => QRCode::OUTPUT_IMAGE_PNG, - 'eccLevel' => QRCode::ECC_L, + 'eccLevel' => QRCode::ECC_Q, + 'outputBase64' => false, // High for good quality. 'scale' => 12, ]);
70-83: Use resampled scaling for better logo quality
imagecopyresizedis jaggy; useimagecopyresampled.- imagecopyresized( + imagecopyresampled( $resizedLogo, $logoImage, 0, 0, 0, 0, $newLogoWidth, $newLogoHeight, $logoWidth, $logoHeight );
84-88: Make user-facing text dynamic (and translatable)Avoid hardcoded copy; read from
$saveOptions, defaulting to translatable strings.- $topText = "Scan to Record Your\nContribution"; - $venueLabel = "Venue: "; - $venueValue = $collectionCampAddress; + $topText = $saveOptions['topText'] ?? \ts("Scan to Record Your\nContribution"); + $venueLabel = $saveOptions['venueLabel'] ?? \ts('Venue: '); + $venueValue = $saveOptions['venueValue'] ?? $collectionCampAddress;
89-96: Canvas sizing may clip long text; size using measured TTF widthsCompute text widths first and use max of logo, heading, QR, and venue wrap width. If reordering is heavy, at minimum use
$newQrWidthinstead of$qrWidth.- // Canvas: logo + top text + QR + bottom text. - $canvasWidth = $qrWidth + 100; - $canvasHeight = $newLogoHeight + $qrHeight + 220; + // Canvas: logo + top text + QR + bottom text. + $canvasWidth = $newQrWidth + 100; + $canvasHeight = $newLogoHeight + $newQrHeight + 220;Optional (recommended): pre-measure
$topTextand the wrapped venue to derive$canvasWidth = max($newQrWidth + 100, $maxTextWidth + 40, $newLogoWidth + 40).
126-154: Remove unused$currentWidth; tighten word-wrappingStatic analysis flags
$currentWidthas unused; also ensure first line wraps cleanly.- $lines = []; - $currentLine = $venueLabel; - $currentWidth = $labelWidth; + $lines = []; + $currentLine = $venueLabel;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-11T09:53:14.458Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#284
File: wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php:520-520
Timestamp: 2024-10-11T09:53:14.458Z
Learning: In `wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php`, it's acceptable that the `reGenerateCollectionCampQr` method calls `generateQrCode` with only the `$collectionCampId` parameter, as QR codes are generated when the Dropping Center is authorized. We'll implement changes if any issues arise in the future.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
📚 Learning: 2024-10-09T15:24:42.859Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#352
File: wp-content/civi-extensions/goonjcustom/Civi/QrCodeService.php:51-53
Timestamp: 2024-10-09T15:24:42.859Z
Learning: In `QrCodeService::generateQrCode`, when using `QRCode::OUTPUT_IMAGE_PNG`, the `render` method returns a data URI with base64-encoded image data. Therefore, it's necessary to remove the `data:image/png;base64,` prefix and base64-decode the data to obtain the raw binary image.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
🪛 PHPMD (2.15.0)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
137-137: Avoid unused local variables such as '$currentWidth'. (Unused Code Rules)
(UnusedLocalVariable)
Comment on lines
+98
to
+101
| // Fonts. | ||
| $fontPath = dirname(__DIR__, 2) . '/fonts/DejaVuSans-Bold.ttf'; | ||
| $fontPathRegular = dirname(__DIR__, 2) . '/fonts/DejaVuSans.ttf'; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify TTF files exist before using imagettf APIs*
Missing font files will trigger warnings and broken rendering.
// Fonts.
- $fontPath = dirname(__DIR__, 2) . '/fonts/DejaVuSans-Bold.ttf';
- $fontPathRegular = dirname(__DIR__, 2) . '/fonts/DejaVuSans.ttf';
+ $fontPath = dirname(__DIR__, 2) . '/fonts/DejaVuSans-Bold.ttf';
+ $fontPathRegular = dirname(__DIR__, 2) . '/fonts/DejaVuSans.ttf';
+ if (!\file_exists($fontPath) || !\file_exists($fontPathRegular)) {
+ throw new \RuntimeException("TTF font file(s) not found");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Fonts. | |
| $fontPath = dirname(__DIR__, 2) . '/fonts/DejaVuSans-Bold.ttf'; | |
| $fontPathRegular = dirname(__DIR__, 2) . '/fonts/DejaVuSans.ttf'; | |
| // Fonts. | |
| $fontPath = dirname(__DIR__, 2) . '/fonts/DejaVuSans-Bold.ttf'; | |
| $fontPathRegular = dirname(__DIR__, 2) . '/fonts/DejaVuSans.ttf'; | |
| if (!\file_exists($fontPath) || !\file_exists($fontPathRegular)) { | |
| throw new \RuntimeException("TTF font file(s) not found"); | |
| } |
Comment on lines
+121
to
+125
| // --- Step 3: QR code | ||
| $qrY = $topY + 50; | ||
| $qrX = (int) (($canvasWidth - $newQrWidth) / 2); | ||
| imagecopy($canvas, $resizedQr, $qrX, $qrY, 0, 0, $newQrWidth, $newQrHeight); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid QR overlap with the heading; position QR after measured text block
Use measured text height instead of a fixed 50px gap.
- $fontSize = 35;
+ $fontSize = 35;
$topY = $logoY + $newLogoHeight + 50;
$lines = explode("\n", $topText);
$lineHeight = $fontSize + 8;
foreach ($lines as $i => $line) {
$bbox = imagettfbbox($fontSize, 0, $fontPath, $line);
$textWidth = abs($bbox[2] - $bbox[0]);
$x = (int) (($canvasWidth - $textWidth) / 2);
$y = $topY + ($i * $lineHeight);
imagettftext($canvas, $fontSize, 0, $x, $y, $black, $fontPath, $line);
}
// --- Step 3: QR code
- $qrY = $topY + 50;
+ $textBlockHeight = (count($lines) ?: 1) * $lineHeight;
+ $qrY = $topY + $textBlockHeight + 20;
$qrX = (int) (($canvasWidth - $newQrWidth) / 2);
imagecopy($canvas, $resizedQr, $qrX, $qrY, 0, 0, $newQrWidth, $newQrHeight);Also applies to: 107-119
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php around
lines 121-125 (and similarly adjust lines 107-119), the QR is placed using a
fixed 50px vertical offset which can overlap the heading; replace the hardcoded
value with the measured height of the rendered text block plus a small padding.
Use imagettfbbox (or your existing text measurement logic) to compute the total
text block height from topY (or the heading start) and set $qrY = $topY +
$textBlockHeight + $padding; keep the $qrX centering logic unchanged; apply the
same change to the other occurrence (lines 107-119) so both placements use
measured height rather than 50px.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (10)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (10)
23-29: Stop base64-stripping; render() already returns raw PNG when not base64.This corrupts the image. Explicitly disable base64 and use the raw bytes as-is.
$options = new QROptions([ 'version' => 5, 'outputType' => QRCode::OUTPUT_IMAGE_PNG, 'eccLevel' => QRCode::ECC_L, - // High for good quality. + // High for good quality. 'scale' => 12, + // Ensure raw PNG bytes (not data URI). + 'outputBase64' => false, ]); - // Generate QR. - $qrcode = (new QRCode($options))->render($data); - $qrcode = str_replace('data:image/png;base64,', '', $qrcode); - $qrcode = base64_decode($qrcode); + // Generate QR as raw PNG binary. + $qrcode = (new QRCode($options))->render($data);Also applies to: 38-44
84-88: Externalize user-facing strings (and make translatable).Hardcoding heading and label is brittle; pull from options and default.
- $topText = "Scan to Record Your\nContribution"; - $venueLabel = "Venue - "; - $venueValue = $collectionCampAddress; + $topText = $saveOptions['topText'] ?? "Scan to Record Your\nContribution"; + $venueLabel = $saveOptions['venueLabel'] ?? 'Venue - '; + $venueValue = $saveOptions['venue'] ?? $collectionCampAddress;
89-97: Canvas sizing and vertical spacing should use measured text metrics (avoid clipping/overlap).Current fixed paddings can overlap the QR and clip long text. Use measured heights/widths.
- // Canvas: logo + top text + QR + bottom text. - $canvasWidth = $qrWidth + 100; - $canvasHeight = $newLogoHeight + $qrHeight + 220; + // Precompute heading block height. + $fontSize = 47; + $lines = explode("\n", $topText); + $lineHeight = $fontSize + 10; + $textBlockHeight = (count($lines) ?: 1) * $lineHeight; + // Canvas: accommodate logo + heading + QR + venue text area. + $canvasWidth = max($qrWidth + 100, 800); // widen as needed + $canvasHeight = $newLogoHeight + $textBlockHeight + $newQrHeight + 240; // --- Step 2: Heading text - $fontSize = 47; - $topY = $logoY + $newLogoHeight + 50; - $lines = explode("\n", $topText); - $lineHeight = $fontSize + 10; + $topY = $logoY + $newLogoHeight + 40; // --- Step 3: QR code - $qrY = $topY + 60; + $qrY = $topY + $textBlockHeight + 20;You can further tighten by computing max text width via imagettfbbox and growing $canvasWidth accordingly.
Also applies to: 107-120, 121-124
136-138: Fix first-line wrap measurement; drop unused variable.The first line mixes bold label + regular value but width is measured with a single font, causing mis-wraps. Also $currentWidth is unused.
- $currentWidth = $labelWidth; + // (removed) $currentWidth was unused $words = explode(" ", $venueValue); foreach ($words as $word) { $testLine = ($currentLine === $venueLabel ? $currentLine : $currentLine . " ") . $word; - $bboxTest = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $testLine); - $testWidth = abs($bboxTest[2] - $bboxTest[0]); + if ($currentLine === $venueLabel) { + // Measure bold label + regular value width correctly. + $valuePart = trim(str_replace($venueLabel, '', $testLine)); + $bboxVal = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $valuePart); + $testWidth = $labelWidth + abs($bboxVal[2] - $bboxVal[0]); + } else { + $bboxTest = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $testLine); + $testWidth = abs($bboxTest[2] - $bboxTest[0]); + }Also applies to: 139-146
187-197: Propagate the save result instead of always returning TRUE.Return the attachment (or FALSE) so callers can react to failures.
- self::saveQrCode($finalImage, $saveOptions); + $attachment = self::saveQrCode($finalImage, $saveOptions); + if ($attachment === FALSE) { + return FALSE; + } + return $attachment; - return TRUE;Also applies to: 190-190
31-37: Null-check the camp lookup to avoid fatal on missing entity.first() can return null; dereferencing will crash. Also cap to 1 row.
- $collectionCamp = EckEntity::get('Collection_Camp', FALSE) + $collectionCamp = EckEntity::get('Collection_Camp', FALSE) ->addSelect('Collection_Camp_Intent_Details.Location_Area_of_camp') ->addWhere('id', '=', $entityId) - ->execute()->first(); + ->setLimit(1) + ->execute() + ->first(); + - $collectionCampAddress = $collectionCamp['Collection_Camp_Intent_Details.Location_Area_of_camp']; + if (!$collectionCamp) { + \CRM_Core_Error::debug_log_message("Collection Camp not found for id {$entityId}"); + return FALSE; + } + $collectionCampAddress = $collectionCamp['Collection_Camp_Intent_Details.Location_Area_of_camp'] ?? '';
42-44: Guard against GD image creation failures.imagecreatefromstring() may return false; imagesx/y would then throw.
- $qrImage = imagecreatefromstring($qrcode); - $qrWidth = imagesx($qrImage); - $qrHeight = imagesy($qrImage); + $qrImage = imagecreatefromstring($qrcode); + if (!$qrImage) { + throw new \RuntimeException('Failed to create QR image from binary'); + } + $qrWidth = imagesx($qrImage); + $qrHeight = imagesy($qrImage);
62-69: Don’t hardcode logo path or fetch over HTTP; prefer filesystem with fallbacks.Hardcoded YYYY/MM and network fetch are brittle. Use uploads basedir and allow overrides.
- $upload_dir = wp_upload_dir(); - $logoUrl = $upload_dir['baseurl'] . '/2024/09/Goonj-logo-10June20-300x193-1.png'; - $logoData = file_get_contents($logoUrl); - $logoImage = imagecreatefromstring($logoData); + $upload_dir = \wp_upload_dir(); + $logoPath = $saveOptions['logoPath'] ?? ($upload_dir['basedir'] . '/brand/Goonj-logo.png'); + if (\file_exists($logoPath)) { + $logoImage = imagecreatefrompng($logoPath); + } else { + $logoUrl = $saveOptions['logoUrl'] ?? null; + if (!$logoUrl) { + throw new \RuntimeException("Logo not found at {$logoPath} and no logoUrl provided"); + } + $response = \wp_remote_get($logoUrl, ['timeout' => 5]); + $logoData = \wp_remote_retrieve_body($response); + if (!$logoData) { + throw new \RuntimeException("Failed to download logo from $logoUrl"); + } + $logoImage = imagecreatefromstring($logoData); + } + if (!$logoImage) { + throw new \RuntimeException('Failed to create logo image resource'); + }
98-101: Verify TTF font files exist before using imagettf.*Avoid warnings/bad renders if fonts are missing.
$fontPath = dirname(__DIR__, 2) . '/fonts/AbhayaLibre-Bold.ttf'; $fontPathRegular = dirname(__DIR__, 2) . '/fonts/AbhayaLibre-Regular.ttf'; + if (!\file_exists($fontPath) || !\file_exists($fontPathRegular)) { + throw new \RuntimeException('TTF font file(s) not found'); + }
176-180: Validate PNG buffer before saving.Avoid saving empty/corrupt files.
ob_start(); - imagepng($canvas); - $finalImage = ob_get_clean(); + $ok = imagepng($canvas); + $finalImage = ob_get_clean(); + if (!$ok || $finalImage === '' || $finalImage === false) { + throw new \RuntimeException('Failed to render composite PNG'); + }
🧹 Nitpick comments (3)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (3)
77-82: Use imagecopyresampled for better logo quality.imagecopyresized is low quality for downscales.
- imagecopyresized( + imagecopyresampled( $resizedLogo, $logoImage, 0, 0, 0, 0, $newLogoWidth, $newLogoHeight, $logoWidth, $logoHeight );
203-217: Use strict check for file_put_contents result.0 bytes shouldn’t be treated the same as FALSE; prefer strict comparison.
- $numBytes = file_put_contents($tempFilePath, $qrcode); - - if (!$numBytes) { + $numBytes = file_put_contents($tempFilePath, $qrcode); + if ($numBytes === false) { \CRM_Core_Error::debug_log_message('Failed to write QR code to temporary file for entity ID ' . $entityId); return FALSE; }
63-64: Namespace global WordPress functions for clarity/perf.Prefix with backslash to avoid namespace resolution overhead.
- $upload_dir = wp_upload_dir(); + $upload_dir = \wp_upload_dir();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (5)
wp-content/civi-extensions/goonjcustom/fonts/AbhayaLibre-Bold.ttfis excluded by!**/*.ttfwp-content/civi-extensions/goonjcustom/fonts/AbhayaLibre-ExtraBold.ttfis excluded by!**/*.ttfwp-content/civi-extensions/goonjcustom/fonts/AbhayaLibre-Medium.ttfis excluded by!**/*.ttfwp-content/civi-extensions/goonjcustom/fonts/AbhayaLibre-Regular.ttfis excluded by!**/*.ttfwp-content/civi-extensions/goonjcustom/fonts/AbhayaLibre-SemiBold.ttfis excluded by!**/*.ttf
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-11T09:53:14.458Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#284
File: wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php:520-520
Timestamp: 2024-10-11T09:53:14.458Z
Learning: In `wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php`, it's acceptable that the `reGenerateCollectionCampQr` method calls `generateQrCode` with only the `$collectionCampId` parameter, as QR codes are generated when the Dropping Center is authorized. We'll implement changes if any issues arise in the future.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
📚 Learning: 2024-10-09T15:24:42.859Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#352
File: wp-content/civi-extensions/goonjcustom/Civi/QrCodeService.php:51-53
Timestamp: 2024-10-09T15:24:42.859Z
Learning: In `QrCodeService::generateQrCode`, when using `QRCode::OUTPUT_IMAGE_PNG`, the `render` method returns a data URI with base64-encoded image data. Therefore, it's necessary to remove the `data:image/png;base64,` prefix and base64-decode the data to obtain the raw binary image.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
🪛 PHPMD (2.15.0)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
137-137: Avoid unused local variables such as '$currentWidth'. (Unused Code Rules)
(UnusedLocalVariable)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (1)
212-219: Propagate save result; don’t always return TRUE
If saving the attachment fails, the function still returns TRUE today.- self::saveQrCode($finalImage, $saveOptions); + $saved = self::saveQrCode($finalImage, $saveOptions); + if ($saved === FALSE) { + return FALSE; + } @@ - return TRUE; + return TRUE; // Optionally: return $saved to expose the attachment
♻️ Duplicate comments (7)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (7)
106-110: Make user-facing strings dynamic (and translatable if needed)
Avoid hardcoding; allow overrides via$saveOptions.- $topText = "Scan to Record Your\nContribution"; - $venueLabel = "Venue - "; - $venueValue = $address; + $topText = $saveOptions['topText'] ?? __('Scan to Record Your', 'goonj') . "\n" . __('Contribution', 'goonj'); + $venueLabel = $saveOptions['venueLabel'] ?? __('Venue - ', 'goonj'); + $venueValue = $saveOptions['venue'] ?? $address;
31-35: Null-check the camp lookup to avoid fatals on missing entity
->first()can return null; subsequent array access will throw.- $campData = EckEntity::get('Collection_Camp', FALSE) + $campData = EckEntity::get('Collection_Camp', FALSE) ->addSelect('subtype:name', 'Collection_Camp_Intent_Details.Location_Area_of_camp', 'Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_', 'Goonj_Activities.Where_do_you_wish_to_organise_the_activity_', 'Institution_Collection_Camp_Intent.Collection_Camp_Address', 'Institution_Dropping_Center_Intent.Dropping_Center_Address', 'Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_') ->addWhere('id', '=', $entityId) - ->execute()->first(); + ->execute() + ->first(); + if (!$campData) { + \CRM_Core_Error::debug_log_message("Collection Camp not found for id {$entityId}"); + return FALSE; + }
64-66: Guard against GD image creation failures (QR)
imagecreatefromstring()can return false;imagesx/y()will then fatal.- $qrImage = imagecreatefromstring($qrcode); - $qrWidth = imagesx($qrImage); - $qrHeight = imagesy($qrImage); + $qrImage = imagecreatefromstring($qrcode); + if (!$qrImage) { + throw new \RuntimeException('Failed to create QR image from binary'); + } + $qrWidth = imagesx($qrImage); + $qrHeight = imagesy($qrImage);
84-91: Avoid hardcoded HTTP logo path; prefer filesystem + robust error handling
Year/month in URL will rot; HTTP adds latency and failures. Use uploads basedir or configurable path; validate and fallback.- $upload_dir = wp_upload_dir(); - $logoUrl = $upload_dir['baseurl'] . '/2024/09/Goonj-logo-10June20-300x193-1.png'; - $logoData = file_get_contents($logoUrl); - $logoImage = imagecreatefromstring($logoData); + $upload_dir = \wp_upload_dir(); + $logoPath = $saveOptions['logoPath'] ?? ($upload_dir['basedir'] . '/brand/Goonj-logo.png'); + if (\file_exists($logoPath)) { + $logoImage = imagecreatefrompng($logoPath); + } else { + $logoUrl = $saveOptions['logoUrl'] ?? null; + if (!$logoUrl) { + throw new \RuntimeException("Logo not found at $logoPath and no logoUrl provided"); + } + $resp = \wp_remote_get($logoUrl, ['timeout' => 5]); + $logoData = \wp_remote_retrieve_body($resp); + if (!$logoData) { + throw new \RuntimeException("Failed to download logo from $logoUrl"); + } + $logoImage = imagecreatefromstring($logoData); + } + if (!$logoImage) { + throw new \RuntimeException('Failed to create logo image resource'); + } $logoWidth = imagesx($logoImage); $logoHeight = imagesy($logoImage);
120-123: Verify TTF font files exist before calling imagettf APIs*
Prevents warnings/fatals in misconfigured environments.// Fonts. - $fontPath = dirname(__DIR__, 2) . '/fonts/AbhayaLibre-Bold.ttf'; - $fontPathRegular = dirname(__DIR__, 2) . '/fonts/AbhayaLibre-Regular.ttf'; + $fontPath = dirname(__DIR__, 2) . '/fonts/AbhayaLibre-Bold.ttf'; + $fontPathRegular = dirname(__DIR__, 2) . '/fonts/AbhayaLibre-Regular.ttf'; + if (!\file_exists($fontPath) || !\file_exists($fontPathRegular)) { + throw new \RuntimeException('TTF font file(s) not found'); + }
111-119: Canvas sizing and spacing don’t account for measured TTF text; risk of clipping/overlap
Compute text widths first; size canvas accordingly and place QR after measured text height.- // Canvas: logo + top text + QR + bottom text. - $canvasWidth = $qrWidth + 100; - $canvasHeight = $newLogoHeight + $qrHeight + 220; - $canvas = imagecreatetruecolor($canvasWidth, $canvasHeight); + // Measure heading block first + $fontSize = 47; + $lines = explode("\n", $topText); + $lineHeight = $fontSize + 10; + $maxTopW = 0; + foreach ($lines as $line) { + $bbox = imagettfbbox($fontSize, 0, $fontPath, $line); + $maxTopW = max($maxTopW, abs($bbox[2] - $bbox[0])); + } + // Canvas: logo + heading + QR + venue + $canvasWidth = max($newQrWidth + 100, $newLogoWidth + 40, $maxTopW + 40); + $canvasHeight = $newLogoHeight + (count($lines) * $lineHeight) + $newQrHeight + 240; + $canvas = imagecreatetruecolor($canvasWidth, $canvasHeight); @@ - $fontSize = 47; - $topY = $logoY + $newLogoHeight + 60; - $lines = explode("\n", $topText); - $lineHeight = $fontSize + 10; + $topY = $logoY + $newLogoHeight + 60; @@ - $qrY = $topY + 70; + $textBlockHeight = max(1, count($lines)) * $lineHeight; + $qrY = $topY + $textBlockHeight + 20;Also applies to: 129-141, 143-147
198-203: Validate PNG buffer before saving
Avoid writing empty/corrupt files.- ob_start(); - imagepng($canvas); - $finalImage = ob_get_clean(); + ob_start(); + $ok = imagepng($canvas); + $finalImage = ob_get_clean(); + if (!$ok || $finalImage === '' || $finalImage === false) { + throw new \RuntimeException('Failed to render composite PNG'); + }
🧹 Nitpick comments (6)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (6)
38-58: Replace long if/elseif with a map; also guard missing address fieldImproves readability and avoids undefined index when a field is absent.
- if ($campStatus == 'Collection_Camp') { - $address = $campData['Collection_Camp_Intent_Details.Location_Area_of_camp']; - } - elseif ($campStatus == 'Dropping_Center') { - $address = $campData['Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_']; - } - elseif ($campStatus == 'Goonj_Activities') { - $address = $campData['Goonj_Activities.Where_do_you_wish_to_organise_the_activity_']; - } - elseif ($campStatus == 'Institution_Collection_Camp') { - $address = $campData['Institution_Collection_Camp_Intent.Collection_Camp_Address']; - } - elseif ($campStatus == 'Institution_Dropping_Center') { - $address = $campData['Institution_Dropping_Center_Intent.Dropping_Center_Address']; - } - elseif ($campStatus == 'Institution_Goonj_Activities') { - $address = $campData['Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_']; - } - else { - throw new \Exception('Invalid entity type for QR code generation.'); - } + $addrFieldBySubtype = [ + 'Collection_Camp' => 'Collection_Camp_Intent_Details.Location_Area_of_camp', + 'Dropping_Center' => 'Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_', + 'Goonj_Activities' => 'Goonj_Activities.Where_do_you_wish_to_organise_the_activity_', + 'Institution_Collection_Camp' => 'Institution_Collection_Camp_Intent.Collection_Camp_Address', + 'Institution_Dropping_Center' => 'Institution_Dropping_Center_Intent.Dropping_Center_Address', + 'Institution_Goonj_Activities' => 'Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_', + ]; + if (!isset($addrFieldBySubtype[$campStatus])) { + throw new \Exception('Invalid entity type for QR code generation.'); + } + $address = $campData[$addrFieldBySubtype[$campStatus]] ?? ''; + if ($address === '') { + \CRM_Core_Error::debug_log_message("Empty address for {$campStatus} id {$entityId}"); + }
23-29: Make QR output type explicit; don’t rely on library defaultsPer team learnings (PR #352), render() returns a data URI only when base64 mode is on. Be explicit: either request raw PNG and drop decoding, or keep base64 but set the option.
Option A (preferred: raw PNG, simpler/faster):
$options = new QROptions([ 'version' => 5, 'outputType' => QRCode::OUTPUT_IMAGE_PNG, 'eccLevel' => QRCode::ECC_L, + 'outputBase64' => false, // High for good quality. 'scale' => 12, ]); - $qrcode = (new QRCode($options))->render($data); - $qrcode = str_replace('data:image/png;base64,', '', $qrcode); - $qrcode = base64_decode($qrcode); + $qrcode = (new QRCode($options))->render($data);Option B (keep base64 pipeline, but be explicit):
$options = new QROptions([ 'version' => 5, 'outputType' => QRCode::OUTPUT_IMAGE_PNG, 'eccLevel' => QRCode::ECC_L, + 'outputBase64' => true,Also applies to: 60-66
99-104: Use resampling for higher-quality logo resize
imagecopyresampledyields better quality thanimagecopyresized.- imagecopyresized( + imagecopyresampled( $resizedLogo, $logoImage, 0, 0, 0, 0, $newLogoWidth, $newLogoHeight, $logoWidth, $logoHeight );
268-276: Prefix global functions in namespaces
Minor perf/readability: call globalcivicrm_api3()explicitly.- $result = civicrm_api3('Attachment', 'create', $params); + $result = \civicrm_api3('Attachment', 'create', $params);
225-239: Clean up temp file on failure paths
Prevent orphaned files when write/attach fails.- $numBytes = file_put_contents($tempFilePath, $qrcode); + $numBytes = file_put_contents($tempFilePath, $qrcode); if (!$numBytes) { \CRM_Core_Error::debug_log_message('Failed to write QR code to temporary file for entity ID ' . $entityId); return FALSE; } @@ - if (!empty($result['is_error'])) { + if (!empty($result['is_error'])) { \CRM_Core_Error::debug_log_message('Failed to create attachment for entity ID ' . $entityId); + @\unlink($tempFilePath); return FALSE; }Also applies to: 268-273
23-29: Optional: increase ECC level for robustness
With compositing and scaling, ECC_M tends to scan more reliably than ECC_L.- 'eccLevel' => QRCode::ECC_L, + 'eccLevel' => QRCode::ECC_M,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-11T09:53:14.458Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#284
File: wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php:520-520
Timestamp: 2024-10-11T09:53:14.458Z
Learning: In `wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php`, it's acceptable that the `reGenerateCollectionCampQr` method calls `generateQrCode` with only the `$collectionCampId` parameter, as QR codes are generated when the Dropping Center is authorized. We'll implement changes if any issues arise in the future.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
📚 Learning: 2024-10-09T15:24:42.859Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#352
File: wp-content/civi-extensions/goonjcustom/Civi/QrCodeService.php:51-53
Timestamp: 2024-10-09T15:24:42.859Z
Learning: In `QrCodeService::generateQrCode`, when using `QRCode::OUTPUT_IMAGE_PNG`, the `render` method returns a data URI with base64-encoded image data. Therefore, it's necessary to remove the `data:image/png;base64,` prefix and base64-decode the data to obtain the raw binary image.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
🪛 PHPMD (2.15.0)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
159-159: Avoid unused local variables such as '$currentWidth'. (Unused Code Rules)
(UnusedLocalVariable)
🔇 Additional comments (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (1)
73-83: Good: alpha-safe resampling pipeline for QR
Alpha handling + resampling is correct and readable.
Comment on lines
+148
to
+196
| // --- Step 4: Venue text with wrapping | ||
| $bottomFontSize = 25; | ||
| $maxWidth = $canvasWidth - 80; | ||
| $bottomY = $qrY + $newQrHeight + 40; | ||
|
|
||
| // Start with bold "Venue:". | ||
| $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel); | ||
| $labelWidth = abs($bboxLabel[2] - $bboxLabel[0]); | ||
|
|
||
| $lines = []; | ||
| $currentLine = $venueLabel; | ||
| $currentWidth = $labelWidth; | ||
|
|
||
| $words = explode(" ", $venueValue); | ||
| foreach ($words as $word) { | ||
| $testLine = ($currentLine === $venueLabel ? $currentLine : $currentLine . " ") . $word; | ||
| $bboxTest = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $testLine); | ||
| $testWidth = abs($bboxTest[2] - $bboxTest[0]); | ||
|
|
||
| if ($testWidth > $maxWidth) { | ||
| $lines[] = $currentLine; | ||
| $currentLine = $word; | ||
| } | ||
| else { | ||
| $currentLine = $testLine; | ||
| } | ||
| } | ||
| $lines[] = $currentLine; | ||
|
|
||
| $lineHeight = $bottomFontSize + 8; | ||
| foreach ($lines as $i => $line) { | ||
| $bbox = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $line); | ||
| $lineWidth = abs($bbox[2] - $bbox[0]); | ||
| $x = (int) (($canvasWidth - $lineWidth) / 2); | ||
| $y = $bottomY + ($i * $lineHeight); | ||
|
|
||
| if ($i === 0) { | ||
| imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel); | ||
| $valuePart = trim(str_replace($venueLabel, "", $line)); | ||
| if ($valuePart !== "") { | ||
| $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel); | ||
| $offset = abs($bboxLabel[2] - $bboxLabel[0]); | ||
| imagettftext($canvas, $bottomFontSize, 0, $x + $offset, $y, $black, $fontPathRegular, $valuePart); | ||
| } | ||
| } | ||
| else { | ||
| imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $line); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix venue wrapping and centering: measure bold label separately; remove unused var
Current wrap measures the first line with regular font while drawing label in bold, causing off-center placement. $currentWidth is unused (PHPMD).
- $bottomFontSize = 25;
- $maxWidth = $canvasWidth - 80;
- $bottomY = $qrY + $newQrHeight + 40;
+ $bottomFontSize = 25;
+ $maxWidth = $canvasWidth - 80;
+ $bottomY = $qrY + $newQrHeight + 40;
@@
- $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel);
- $labelWidth = abs($bboxLabel[2] - $bboxLabel[0]);
-
- $lines = [];
- $currentLine = $venueLabel;
- $currentWidth = $labelWidth;
-
- $words = explode(" ", $venueValue);
- foreach ($words as $word) {
- $testLine = ($currentLine === $venueLabel ? $currentLine : $currentLine . " ") . $word;
- $bboxTest = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $testLine);
- $testWidth = abs($bboxTest[2] - $bboxTest[0]);
-
- if ($testWidth > $maxWidth) {
- $lines[] = $currentLine;
- $currentLine = $word;
- }
- else {
- $currentLine = $testLine;
- }
- }
- $lines[] = $currentLine;
+ $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel);
+ $labelWidth = abs($bboxLabel[2] - $bboxLabel[0]);
+ // Wrap only the value; first line has reduced width due to label
+ $words = explode(' ', trim($venueValue));
+ $valueLines = [];
+ $current = '';
+ foreach ($words as $word) {
+ $test = $current === '' ? $word : $current . ' ' . $word;
+ $bbox = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $test);
+ $testW = abs($bbox[2] - $bbox[0]);
+ $allowed = empty($valueLines) ? ($maxWidth - $labelWidth) : $maxWidth;
+ if ($testW > $allowed && $current !== '') {
+ $valueLines[] = $current;
+ $current = $word;
+ } else {
+ $current = $test;
+ }
+ }
+ if ($current !== '') {
+ $valueLines[] = $current;
+ }
@@
- foreach ($lines as $i => $line) {
- $bbox = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $line);
- $lineWidth = abs($bbox[2] - $bbox[0]);
- $x = (int) (($canvasWidth - $lineWidth) / 2);
- $y = $bottomY + ($i * $lineHeight);
-
- if ($i === 0) {
- imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel);
- $valuePart = trim(str_replace($venueLabel, "", $line));
- if ($valuePart !== "") {
- $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel);
- $offset = abs($bboxLabel[2] - $bboxLabel[0]);
- imagettftext($canvas, $bottomFontSize, 0, $x + $offset, $y, $black, $fontPathRegular, $valuePart);
- }
- }
- else {
- imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $line);
- }
- }
+ foreach ($valueLines as $i => $valLine) {
+ $bboxVal = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $valLine);
+ $valW = abs($bboxVal[2] - $bboxVal[0]);
+ $lineW = ($i === 0) ? ($labelWidth + $valW) : $valW;
+ $x = (int)(($canvasWidth - $lineW) / 2);
+ $y = $bottomY + ($i * $lineHeight);
+ if ($i === 0) {
+ imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel);
+ imagettftext($canvas, $bottomFontSize, 0, $x + $labelWidth, $y, $black, $fontPathRegular, $valLine);
+ } else {
+ imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $valLine);
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // --- Step 4: Venue text with wrapping | |
| $bottomFontSize = 25; | |
| $maxWidth = $canvasWidth - 80; | |
| $bottomY = $qrY + $newQrHeight + 40; | |
| // Start with bold "Venue:". | |
| $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel); | |
| $labelWidth = abs($bboxLabel[2] - $bboxLabel[0]); | |
| $lines = []; | |
| $currentLine = $venueLabel; | |
| $currentWidth = $labelWidth; | |
| $words = explode(" ", $venueValue); | |
| foreach ($words as $word) { | |
| $testLine = ($currentLine === $venueLabel ? $currentLine : $currentLine . " ") . $word; | |
| $bboxTest = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $testLine); | |
| $testWidth = abs($bboxTest[2] - $bboxTest[0]); | |
| if ($testWidth > $maxWidth) { | |
| $lines[] = $currentLine; | |
| $currentLine = $word; | |
| } | |
| else { | |
| $currentLine = $testLine; | |
| } | |
| } | |
| $lines[] = $currentLine; | |
| $lineHeight = $bottomFontSize + 8; | |
| foreach ($lines as $i => $line) { | |
| $bbox = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $line); | |
| $lineWidth = abs($bbox[2] - $bbox[0]); | |
| $x = (int) (($canvasWidth - $lineWidth) / 2); | |
| $y = $bottomY + ($i * $lineHeight); | |
| if ($i === 0) { | |
| imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel); | |
| $valuePart = trim(str_replace($venueLabel, "", $line)); | |
| if ($valuePart !== "") { | |
| $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel); | |
| $offset = abs($bboxLabel[2] - $bboxLabel[0]); | |
| imagettftext($canvas, $bottomFontSize, 0, $x + $offset, $y, $black, $fontPathRegular, $valuePart); | |
| } | |
| } | |
| else { | |
| imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $line); | |
| } | |
| } | |
| // --- Step 4: Venue text with wrapping | |
| $bottomFontSize = 25; | |
| $maxWidth = $canvasWidth - 80; | |
| $bottomY = $qrY + $newQrHeight + 40; | |
| // Measure bold "Venue:" label | |
| $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel); | |
| $labelWidth = abs($bboxLabel[2] - $bboxLabel[0]); | |
| // Wrap only the venue value; the first line gets less width because of the label | |
| $words = explode(' ', trim($venueValue)); | |
| $valueLines = []; | |
| $current = ''; | |
| foreach ($words as $word) { | |
| $test = $current === '' ? $word : $current . ' ' . $word; | |
| $bbox = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $test); | |
| $testW = abs($bbox[2] - $bbox[0]); | |
| $allowed = empty($valueLines) ? ($maxWidth - $labelWidth) : $maxWidth; | |
| if ($testW > $allowed && $current !== '') { | |
| $valueLines[] = $current; | |
| $current = $word; | |
| } else { | |
| $current = $test; | |
| } | |
| } | |
| if ($current !== '') { | |
| $valueLines[] = $current; | |
| } | |
| // Render each line, centering the first line by combining label+value width | |
| $lineHeight = $bottomFontSize + 8; | |
| foreach ($valueLines as $i => $valLine) { | |
| $bboxVal = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $valLine); | |
| $valW = abs($bboxVal[2] - $bboxVal[0]); | |
| $lineW = ($i === 0) ? ($labelWidth + $valW) : $valW; | |
| $x = (int)(($canvasWidth - $lineW) / 2); | |
| $y = $bottomY + ($i * $lineHeight); | |
| if ($i === 0) { | |
| imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel); | |
| imagettftext($canvas, $bottomFontSize, 0, $x + $labelWidth, $y, $black, $fontPathRegular, $valLine); | |
| } else { | |
| imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $valLine); | |
| } | |
| } |
🧰 Tools
🪛 PHPMD (2.15.0)
159-159: Avoid unused local variables such as '$currentWidth'. (Unused Code Rules)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php around
lines 148 to 196, the venue wrapping currently measures the first line with the
regular font while the label "Venue:" is drawn in bold and the variable
$currentWidth is unused; to fix: stop including the bold label in width
calculations for wrapped lines — keep $currentLine initially empty or only the
venue value, measure the bold label separately with imagettfbbox using $fontPath
to get its width, remove the unused $currentWidth variable, and when centering
the first rendered line compute x by measuring the text portions with their
actual fonts (label measured with $fontPath and value measured with
$fontPathRegular) so the combined drawn text is centered correctly; ensure
during the word-wrapping loop you measure test widths using $fontPathRegular for
the value-only test strings and when outputting the first line draw the bold
label at x and the remaining value at x + labelWidth.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (11)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (11)
23-28: Stop base64-stripping; render raw PNG instead.Set outputBase64=false and remove the decode. Current decode path can corrupt data when render() already returns raw bytes.
$options = new QROptions([ 'version' => 5, 'outputType' => QRCode::OUTPUT_IMAGE_PNG, - 'eccLevel' => QRCode::ECC_L, + 'eccLevel' => QRCode::ECC_L, + 'outputBase64' => false, 'scale' => 12, ]); @@ - $qrcode = (new QRCode($options))->render($data); - $qrcode = str_replace('data:image/png;base64,', '', $qrcode); - $qrcode = base64_decode($qrcode); + $qrcode = (new QRCode($options))->render($data);Also applies to: 66-70
112-116: Hard-coded, user-facing text; make configurable.Source from $saveOptions (and i18n if applicable).
- $topText = "Scan to Record Your\nContribution"; - $venueLabel = "Venue - "; - $venueValue = $address; + $topText = $saveOptions['topText'] ?? "Scan to Record Your\nContribution"; + $venueLabel = $saveOptions['venueLabel'] ?? "Venue - "; + $venueValue = $saveOptions['venueValue'] ?? $address;
149-153: Avoid QR overlapping the heading; position using measured text height.Replace fixed 70px gap with actual heading block height.
- $qrY = $topY + 70; + $headingBlockHeight = (count($lines) ?: 1) * $lineHeight; + $qrY = $topY + $headingBlockHeight + 20;
37-43: Null-check EckEntity::first() result before indexing.first() may return null; subsequent array access will fatal. Bail out early with a clear log.
$campData = EckEntity::get('Collection_Camp', FALSE) ->addSelect('subtype:name', 'Collection_Camp_Intent_Details.Location_Area_of_camp', 'Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_', 'Goonj_Activities.Where_do_you_wish_to_organise_the_activity_', 'Institution_Collection_Camp_Intent.Collection_Camp_Address', 'Institution_Dropping_Center_Intent.Dropping_Center_Address', 'Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_') ->addWhere('id', '=', $entityId) - ->execute()->first(); + ->execute()->first(); + + if (!$campData) { + \CRM_Core_Error::debug_log_message("Collection Camp not found for id {$entityId}"); + return FALSE; + }
70-73: Guard GD creation failures to prevent TypeErrors.imagecreatefromstring() can return false; check before imagesx()/imagesy().
- $qrImage = imagecreatefromstring($qrcode); - $qrWidth = imagesx($qrImage); - $qrHeight = imagesy($qrImage); + $qrImage = imagecreatefromstring($qrcode); + if (!$qrImage) { + throw new \RuntimeException('Failed to create QR image from binary'); + } + $qrWidth = imagesx($qrImage); + $qrHeight = imagesy($qrImage);
90-97: Don’t fetch logo over HTTP or hardcode dated paths; prefer filesystem with fallback.Use uploads basedir + configurable path; add timeouts and error handling.
- $upload_dir = wp_upload_dir(); - $logoUrl = $upload_dir['baseurl'] . '/2024/09/Goonj-logo-10June20-300x193-1.png'; - $logoData = file_get_contents($logoUrl); - $logoImage = imagecreatefromstring($logoData); + $upload_dir = \wp_upload_dir(); + $logoPath = $saveOptions['logoPath'] ?? ($upload_dir['basedir'] . '/brand/Goonj-logo.png'); + if (\file_exists($logoPath) && \is_readable($logoPath)) { + $logoImage = \imagecreatefrompng($logoPath); + } else { + $logoUrl = $saveOptions['logoUrl'] ?? ''; + if (!$logoUrl) { + throw new \RuntimeException("Logo not found at $logoPath and no logoUrl provided"); + } + $resp = \wp_remote_get($logoUrl, ['timeout' => 5]); + $logoData = \wp_remote_retrieve_body($resp); + $logoImage = $logoData ? \imagecreatefromstring($logoData) : false; + } + if (!$logoImage) { + throw new \RuntimeException('Failed to create logo image'); + }
126-129: Verify TTF font files exist before imagettf calls.*Avoid warnings and broken rendering when fonts are missing.
- $fontPath = dirname(__DIR__, 2) . '/fonts/AbhayaLibre-Bold.ttf'; - $fontPathRegular = dirname(__DIR__, 2) . '/fonts/AbhayaLibre-Regular.ttf'; + $fontPath = dirname(__DIR__, 2) . '/fonts/AbhayaLibre-Bold.ttf'; + $fontPathRegular = dirname(__DIR__, 2) . '/fonts/AbhayaLibre-Regular.ttf'; + if (!\file_exists($fontPath) || !\file_exists($fontPathRegular)) { + throw new \RuntimeException('TTF font file(s) not found'); + }
117-121: Canvas sizing should be based on measured TTF text, not fixed widths.Current width/height risk clipping long headings/venues. Measure first, then allocate canvas.
- // Canvas: logo + top text + QR + bottom text. - $canvasWidth = $qrWidth + 100; - $canvasHeight = $newLogoHeight + $qrHeight + 220; - $canvas = imagecreatetruecolor($canvasWidth, $canvasHeight); + // Canvas: logo + heading + QR + venue; compute dynamic dimensions. + $headingFontSize = 47; + $headingLines = explode("\n", $topText); + $maxTextWidth = 0; + foreach ($headingLines as $hl) { + $bbox = \imagettfbbox($headingFontSize, 0, $fontPath, $hl); + $maxTextWidth = max($maxTextWidth, abs($bbox[2] - $bbox[0])); + } + $canvasWidth = max($newLogoWidth + 40, $newQrWidth + 100, $maxTextWidth + 40); + $canvasHeight = $newLogoHeight + (count($headingLines) * ($headingFontSize + 10)) + $newQrHeight + 200; + $canvas = \imagecreatetruecolor($canvasWidth, $canvasHeight);Additional change (outside this hunk): move the “Fonts.” block (Lines 126–129) above this section so $fontPath is available for measurement.
163-202: Fix venue wrapping/centering and remove unused $currentWidth (PHPMD).Measure bold label separately; wrap only value; center combined first line.
- $lines = []; - $currentLine = $venueLabel; - $currentWidth = $labelWidth; - - $words = explode(" ", $venueValue); - foreach ($words as $word) { - $testLine = ($currentLine === $venueLabel ? $currentLine : $currentLine . " ") . $word; - $bboxTest = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $testLine); - $testWidth = abs($bboxTest[2] - $bboxTest[0]); - - if ($testWidth > $maxWidth) { - $lines[] = $currentLine; - $currentLine = $word; - } - else { - $currentLine = $testLine; - } - } - $lines[] = $currentLine; + $words = explode(' ', trim($venueValue)); + $valueLines = []; + $current = ''; + foreach ($words as $word) { + $test = $current === '' ? $word : $current . ' ' . $word; + $bbox = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $test); + $testW = abs($bbox[2] - $bbox[0]); + $allowed = empty($valueLines) ? ($maxWidth - $labelWidth) : $maxWidth; + if ($testW > $allowed && $current !== '') { + $valueLines[] = $current; + $current = $word; + } else { + $current = $test; + } + } + if ($current !== '') { + $valueLines[] = $current; + } @@ - foreach ($lines as $i => $line) { - $bbox = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $line); - $lineWidth = abs($bbox[2] - $bbox[0]); - $x = (int) (($canvasWidth - $lineWidth) / 2); - $y = $bottomY + ($i * $lineHeight); - - if ($i === 0) { - imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel); - $valuePart = trim(str_replace($venueLabel, "", $line)); - if ($valuePart !== "") { - $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel); - $offset = abs($bboxLabel[2] - $bboxLabel[0]); - imagettftext($canvas, $bottomFontSize, 0, $x + $offset, $y, $black, $fontPathRegular, $valuePart); - } - } - else { - imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $line); - } - } + foreach ($valueLines as $i => $valLine) { + $bboxVal = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $valLine); + $valW = abs($bboxVal[2] - $bboxVal[0]); + $lineW = ($i === 0) ? ($labelWidth + $valW) : $valW; + $x = (int)(($canvasWidth - $lineW) / 2); + $y = $bottomY + ($i * $lineHeight); + if ($i === 0) { + imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel); + imagettftext($canvas, $bottomFontSize, 0, $x + $labelWidth, $y, $black, $fontPathRegular, $valLine); + } else { + imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $valLine); + } + }
204-208: Validate PNG buffer before saving.If imagepng/ob_get_clean fails, you’ll write an empty file and still report success.
- ob_start(); - imagepng($canvas); - $finalImage = ob_get_clean(); + ob_start(); + $ok = \imagepng($canvas); + $finalImage = ob_get_clean(); + if (!$ok || $finalImage === '' || $finalImage === false) { + throw new \RuntimeException('Failed to render composite PNG'); + }
215-219: Propagate save result instead of always returning TRUE.Return the attachment or FALSE; don’t swallow failures.
- self::saveQrCode($finalImage, $saveOptions); + $attachment = self::saveQrCode($finalImage, $saveOptions); + if ($attachment === FALSE) { + return FALSE; + } + return $attachment; @@ - return TRUE; + // return TRUE; // handled above by returning $attachmentAlso applies to: 225-225
🧹 Nitpick comments (3)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (3)
105-110: Use resampling for better logo quality.imagecopyresampled() yields smoother results than imagecopyresized().
- imagecopyresized( + imagecopyresampled( $resizedLogo, $logoImage, 0, 0, 0, 0, $newLogoWidth, $newLogoHeight, $logoWidth, $logoHeight );
23-28: Optional: increase ECC level for better scan resilience.ECC_L is minimal; consider ECC_M for real-world usage.
- 'eccLevel' => QRCode::ECC_L, + 'eccLevel' => QRCode::ECC_M,
204-214: Optional: ensure GD resources are freed on exceptions.Move imagedestroy(...) calls to a finally block to avoid leaks on early errors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
wp-content/civi-extensions/goonjcustom/fonts/Fontspring-DEMO-proximanova-bold.otfis excluded by!**/*.otfwp-content/civi-extensions/goonjcustom/fonts/Fontspring-DEMO-proximanova-regular.otfis excluded by!**/*.otf
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-11T09:53:14.458Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#284
File: wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php:520-520
Timestamp: 2024-10-11T09:53:14.458Z
Learning: In `wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php`, it's acceptable that the `reGenerateCollectionCampQr` method calls `generateQrCode` with only the `$collectionCampId` parameter, as QR codes are generated when the Dropping Center is authorized. We'll implement changes if any issues arise in the future.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
📚 Learning: 2024-10-09T15:24:42.859Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#352
File: wp-content/civi-extensions/goonjcustom/Civi/QrCodeService.php:51-53
Timestamp: 2024-10-09T15:24:42.859Z
Learning: In `QrCodeService::generateQrCode`, when using `QRCode::OUTPUT_IMAGE_PNG`, the `render` method returns a data URI with base64-encoded image data. Therefore, it's necessary to remove the `data:image/png;base64,` prefix and base64-decode the data to obtain the raw binary image.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
🪛 PHPMD (2.15.0)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
165-165: Avoid unused local variables such as '$currentWidth'. (Unused Code Rules)
(UnusedLocalVariable)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (9)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (9)
127-131: Canvas sizing ignores TTF text dimensions; may clip long textMeasure text first (heading and venue) and size canvas accordingly. Also verify fonts exist.
Apply:
- // Canvas: logo + top text + QR + bottom text. - $canvasWidth = $qrWidth + 100; - $canvasHeight = $newLogoHeight + $qrHeight + 220; - $canvas = imagecreatetruecolor($canvasWidth, $canvasHeight); + // Fonts. + $fontPath = dirname(__DIR__, 2) . '/fonts/AbhayaLibre-Bold.ttf'; + $fontPathRegular = dirname(__DIR__, 2) . '/fonts/AbhayaLibre-Regular.ttf'; + if (!\file_exists($fontPath) || !\file_exists($fontPathRegular)) { + throw new \RuntimeException('TTF font file(s) not found'); + } + + // Measure text to derive canvas size. + $headingSize = 47; + $headingLines = explode("\n", $topText); + $maxHeadingW = 0; + foreach ($headingLines as $line) { + $bbox = imagettfbbox($headingSize, 0, $fontPath, $line); + $maxHeadingW = max($maxHeadingW, abs($bbox[2] - $bbox[0])); + } + $venueSize = 25; + $bboxLabel = imagettfbbox($venueSize, 0, $fontPath, $venueLabel); + $labelW = abs($bboxLabel[2] - $bboxLabel[0]); + $bboxVal = imagettfbbox($venueSize, 0, $fontPathRegular, (string)$address); + $valW = abs($bboxVal[2] - $bboxVal[0]); + $maxTextW = max($maxHeadingW, $labelW + $valW); + + // Canvas: accommodate widest element. + $canvasWidth = max($newQrWidth + 100, $newLogoWidth + 40, $maxTextW + 80); + $canvasHeight = $newLogoHeight + (count($headingLines) * ($headingSize + 10)) + $newQrHeight + ($venueSize + 8) * 2 + 200; + $canvas = imagecreatetruecolor($canvasWidth, $canvasHeight);Also applies to: 136-139
145-157: Place QR after measured heading to avoid overlapUse measured heading height instead of fixed 70px gap.
Apply:
- $fontSize = 47; - $topY = $logoY + $newLogoHeight + 60; - $lines = explode("\n", $topText); - $lineHeight = $fontSize + 10; + $fontSize = 47; + $topY = $logoY + $newLogoHeight + 60; + $lines = explode("\n", $topText); + $lineHeight = $fontSize + 10; @@ - // --- Step 3: QR code - $qrY = $topY + 70; + // --- Step 3: QR code + $textBlockHeight = (count($lines) ?: 1) * $lineHeight; + $qrY = $topY + $textBlockHeight + 20; $qrX = (int) (($canvasWidth - $newQrWidth) / 2);Also applies to: 159-163
169-212: Fix venue wrapping/centering; remove unused varMeasure bold label separately; wrap only the value; drop
$currentWidth.Apply:
- $lines = []; - $currentLine = $venueLabel; - $currentWidth = $labelWidth; - - $words = explode(" ", $venueValue); - foreach ($words as $word) { - $testLine = ($currentLine === $venueLabel ? $currentLine : $currentLine . " ") . $word; - $bboxTest = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $testLine); - $testWidth = abs($bboxTest[2] - $bboxTest[0]); - - if ($testWidth > $maxWidth) { - $lines[] = $currentLine; - $currentLine = $word; - } - else { - $currentLine = $testLine; - } - } - $lines[] = $currentLine; + $words = explode(' ', trim((string)$venueValue)); + $valueLines = []; + $current = ''; + foreach ($words as $word) { + $test = $current === '' ? $word : $current . ' ' . $word; + $bboxTest = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $test); + $testWidth = abs($bboxTest[2] - $bboxTest[0]); + $allowed = empty($valueLines) ? ($maxWidth - $labelWidth) : $maxWidth; + if ($testWidth > $allowed && $current !== '') { + $valueLines[] = $current; + $current = $word; + } else { + $current = $test; + } + } + if ($current !== '') { + $valueLines[] = $current; + } @@ - foreach ($lines as $i => $line) { - $bbox = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $line); - $lineWidth = abs($bbox[2] - $bbox[0]); - $x = (int) (($canvasWidth - $lineWidth) / 2); - $y = $bottomY + ($i * $lineHeight); - - if ($i === 0) { - imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel); - $valuePart = trim(str_replace($venueLabel, "", $line)); - if ($valuePart !== "") { - $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel); - $offset = abs($bboxLabel[2] - $bboxLabel[0]); - imagettftext($canvas, $bottomFontSize, 0, $x + $offset, $y, $black, $fontPathRegular, $valuePart); - } - } - else { - imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $line); - } - } + foreach ($valueLines as $i => $valLine) { + $bboxVal = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $valLine); + $valW = abs($bboxVal[2] - $bboxVal[0]); + $lineW = ($i === 0) ? ($labelWidth + $valW) : $valW; + $x = (int)(($canvasWidth - $lineW) / 2); + $y = $bottomY + ($i * $lineHeight); + if ($i === 0) { + imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel); + imagettftext($canvas, $bottomFontSize, 0, $x + $labelWidth, $y, $black, $fontPathRegular, $valLine); + } else { + imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $valLine); + } + }
122-126: Make user-facing text configurableHardcoded heading/label/venue reduces reuse; read from options with sensible defaults.
Apply:
- $topText = "Scan to Record Your\nContribution"; - $venueLabel = "Venue - "; - $venueValue = $address; + $topText = $saveOptions['topText'] ?? "Scan to Record Your\nContribution"; + $venueLabel = $saveOptions['venueLabel'] ?? "Venue - "; + $venueValue = $saveOptions['venue'] ?? ($address ?? '');
225-235: Propagate save result; don’t always return TRUEReturn FALSE on save failure; optionally return attachment.
Apply:
- self::saveQrCode($finalImage, $saveOptions); + $attachment = self::saveQrCode($finalImage, $saveOptions); + if ($attachment === FALSE) { + return FALSE; + } @@ - return TRUE; + return TRUE; // or `return $attachment;` if callers expect the attachmentAlso applies to: 228-228
45-74: Null-check camp data and guard missing keys
$campDatamay be null; individual keys may be absent. Prevent undefined index/fatals.Apply:
- $campData = EckEntity::get('Collection_Camp', FALSE) + $campData = EckEntity::get('Collection_Camp', FALSE) ->addSelect('subtype:name', 'Collection_Camp_Intent_Details.Location_Area_of_camp', 'Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_', 'Goonj_Activities.Where_do_you_wish_to_organise_the_activity_', 'Institution_Collection_Camp_Intent.Collection_Camp_Address', 'Institution_Dropping_Center_Intent.Dropping_Center_Address', 'Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_') ->addWhere('id', '=', $entityId) ->execute()->first(); - - $campStatus = $campData['subtype:name']; + if (!$campData) { + \CRM_Core_Error::debug_log_message("Collection Camp not found for id {$entityId}"); + return FALSE; + } + $campStatus = $campData['subtype:name'] ?? null; + if (!$campStatus) { + \CRM_Core_Error::debug_log_message("Missing subtype for camp id {$entityId}"); + return FALSE; + } @@ - if ($campStatus == 'Collection_Camp') { - $address = $campData['Collection_Camp_Intent_Details.Location_Area_of_camp']; + if ($campStatus == 'Collection_Camp') { + $address = $campData['Collection_Camp_Intent_Details.Location_Area_of_camp'] ?? ''; } elseif ($campStatus == 'Dropping_Center') { - $address = $campData['Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_']; + $address = $campData['Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_'] ?? ''; } elseif ($campStatus == 'Goonj_Activities') { - $address = $campData['Goonj_Activities.Where_do_you_wish_to_organise_the_activity_']; + $address = $campData['Goonj_Activities.Where_do_you_wish_to_organise_the_activity_'] ?? ''; } elseif ($campStatus == 'Institution_Collection_Camp') { - $address = $campData['Institution_Collection_Camp_Intent.Collection_Camp_Address']; + $address = $campData['Institution_Collection_Camp_Intent.Collection_Camp_Address'] ?? ''; } elseif ($campStatus == 'Institution_Dropping_Center') { - $address = $campData['Institution_Dropping_Center_Intent.Dropping_Center_Address']; + $address = $campData['Institution_Dropping_Center_Intent.Dropping_Center_Address'] ?? ''; } elseif ($campStatus == 'Institution_Goonj_Activities') { - $address = $campData['Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_']; + $address = $campData['Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_'] ?? ''; }
80-83: Check QR image creation result
imagecreatefromstring()can return false; guard before imagesx/y.Apply:
- $qrImage = imagecreatefromstring($qrcode); - $qrWidth = imagesx($qrImage); - $qrHeight = imagesy($qrImage); + $qrImage = imagecreatefromstring($qrcode); + if (!$qrImage) { + throw new \RuntimeException('Failed to create QR image from binary'); + } + $qrWidth = imagesx($qrImage); + $qrHeight = imagesy($qrImage);
100-121: Don’t hardcode HTTP logo path; use filesystem and validateCurrent code fetches over HTTP with a dated path and no error checks; brittle and slower.
Apply:
- $upload_dir = wp_upload_dir(); - $logoUrl = $upload_dir['baseurl'] . '/2024/09/Goonj-logo-10June20-300x193-1.png'; - $logoData = file_get_contents($logoUrl); - $logoImage = imagecreatefromstring($logoData); + $upload_dir = \wp_upload_dir(); + $logoPath = $saveOptions['logoPath'] ?? ($upload_dir['basedir'] . '/brand/Goonj-logo.png'); + if (\file_exists($logoPath)) { + $logoImage = imagecreatefrompng($logoPath); + } else { + $logoUrl = $saveOptions['logoUrl'] ?? null; + if (!$logoUrl) { + throw new \RuntimeException("Logo not found at $logoPath and no logoUrl provided"); + } + $resp = \wp_remote_get($logoUrl, ['timeout' => 5]); + $logoData = \wp_remote_retrieve_body($resp); + if (!$logoData) { + throw new \RuntimeException("Failed to download logo from $logoUrl"); + } + $logoImage = imagecreatefromstring($logoData); + } + if (!$logoImage) { + throw new \RuntimeException('Failed to create logo image resource'); + }
214-218: Validate PNG buffer before savingAvoid writing empty buffer on failure.
Apply:
// Save final. ob_start(); - imagepng($canvas); - $finalImage = ob_get_clean(); + $ok = imagepng($canvas); + $finalImage = ob_get_clean(); + if (!$ok || $finalImage === '' || $finalImage === false) { + throw new \RuntimeException('Failed to render composite PNG'); + }
🧹 Nitpick comments (2)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (2)
25-30: Avoid base64 round-trip; render raw PNGSet outputBase64=false and remove stripping/decoding to cut CPU/memory.
Apply:
$options = new QROptions([ 'version' => 5, 'outputType' => QRCode::OUTPUT_IMAGE_PNG, 'eccLevel' => QRCode::ECC_L, - 'scale' => 12, + 'scale' => 12, + 'outputBase64' => false, ]); @@ - $qrcode = str_replace('data:image/png;base64,', '', $qrcode); - $qrcode = base64_decode($qrcode); + // $qrcode is raw PNG bytes when outputBase64=falseAlso applies to: 78-79
115-120: Use resampling for logo quality
imagecopyresizedproduces jaggies; preferimagecopyresampled.Apply:
- imagecopyresized( + imagecopyresampled( $resizedLogo, $logoImage, 0, 0, 0, 0, $newLogoWidth, $newLogoHeight, $logoWidth, $logoHeight );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-11T09:53:14.458Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#284
File: wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php:520-520
Timestamp: 2024-10-11T09:53:14.458Z
Learning: In `wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php`, it's acceptable that the `reGenerateCollectionCampQr` method calls `generateQrCode` with only the `$collectionCampId` parameter, as QR codes are generated when the Dropping Center is authorized. We'll implement changes if any issues arise in the future.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
📚 Learning: 2024-10-09T15:24:42.859Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#352
File: wp-content/civi-extensions/goonjcustom/Civi/QrCodeService.php:51-53
Timestamp: 2024-10-09T15:24:42.859Z
Learning: In `QrCodeService::generateQrCode`, when using `QRCode::OUTPUT_IMAGE_PNG`, the `render` method returns a data URI with base64-encoded image data. Therefore, it's necessary to remove the `data:image/png;base64,` prefix and base64-decode the data to obtain the raw binary image.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
🪛 PHPMD (2.15.0)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
175-175: Avoid unused local variables such as '$currentWidth'. (Unused Code Rules)
(UnusedLocalVariable)
Comment on lines
+32
to
+43
| if (!empty($saveOptions['customGroupName']) && $saveOptions['customGroupName'] === 'Event_QR') { | ||
| $event = Event::get(TRUE) | ||
| ->addSelect('loc_block_id.address_id') | ||
| ->addWhere('id', '=', $entityId) | ||
| ->execute()->first(); | ||
|
|
||
| $addresses = Address::get(FALSE) | ||
| ->addWhere('id', '=', $event['loc_block_id.address_id']) | ||
| ->setLimit(1) | ||
| ->execute()->first(); | ||
| $address = \CRM_Utils_Address::format($addresses); | ||
| } |
There was a problem hiding this comment.
Guard Event/address lookups to avoid fatals
->first() may return null; accessing array offsets would fatal. Also handle missing loc_block/address rows.
Apply:
- $event = Event::get(TRUE)
+ $event = Event::get(TRUE)
->addSelect('loc_block_id.address_id')
->addWhere('id', '=', $entityId)
- ->execute()->first();
+ ->execute()->first();
+ if (!$event || empty($event['loc_block_id.address_id'])) {
+ \CRM_Core_Error::debug_log_message("Event or address not found for id {$entityId}");
+ return FALSE;
+ }
-
- $addresses = Address::get(FALSE)
+ $addresses = Address::get(FALSE)
->addWhere('id', '=', $event['loc_block_id.address_id'])
->setLimit(1)
- ->execute()->first();
- $address = \CRM_Utils_Address::format($addresses);
+ ->execute()->first();
+ if (!$addresses) {
+ \CRM_Core_Error::debug_log_message("Address not found for event id {$entityId}");
+ return FALSE;
+ }
+ $address = \CRM_Utils_Address::format($addresses);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!empty($saveOptions['customGroupName']) && $saveOptions['customGroupName'] === 'Event_QR') { | |
| $event = Event::get(TRUE) | |
| ->addSelect('loc_block_id.address_id') | |
| ->addWhere('id', '=', $entityId) | |
| ->execute()->first(); | |
| $addresses = Address::get(FALSE) | |
| ->addWhere('id', '=', $event['loc_block_id.address_id']) | |
| ->setLimit(1) | |
| ->execute()->first(); | |
| $address = \CRM_Utils_Address::format($addresses); | |
| } | |
| if (!empty($saveOptions['customGroupName']) && $saveOptions['customGroupName'] === 'Event_QR') { | |
| $event = Event::get(TRUE) | |
| ->addSelect('loc_block_id.address_id') | |
| ->addWhere('id', '=', $entityId) | |
| ->execute()->first(); | |
| if (!$event || empty($event['loc_block_id.address_id'])) { | |
| \CRM_Core_Error::debug_log_message("Event or address not found for id {$entityId}"); | |
| return FALSE; | |
| } | |
| $addresses = Address::get(FALSE) | |
| ->addWhere('id', '=', $event['loc_block_id.address_id']) | |
| ->setLimit(1) | |
| ->execute()->first(); | |
| if (!$addresses) { | |
| \CRM_Core_Error::debug_log_message("Address not found for event id {$entityId}"); | |
| return FALSE; | |
| } | |
| $address = \CRM_Utils_Address::format($addresses); | |
| } |
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php around
lines 32 to 43, the code assumes Event::get(...)->execute()->first() and
Address::get(...)->execute()->first() always return arrays and directly indexes
them which can cause fatal errors; update the code to guard both lookups: check
the event result is non-null/array before accessing 'loc_block_id.address_id',
verify that the retrieved address row exists before passing it to
CRM_Utils_Address::format (or provide a sensible default/empty string if
missing), and bail out or skip QR/address logic when required IDs/rows are
absent to avoid notices/fatals.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (12)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (12)
25-30: Render raw PNG: set outputBase64=false and remove manual base64 handlingExplicitly disable base64 output to avoid brittle string ops and potential binary mangling; let render() return raw PNG bytes and drop the decode logic. Also prefer auto version and higher ECC for scan reliability.
$options = new QROptions([ - 'version' => 5, + 'version' => 0, // auto 'outputType' => QRCode::OUTPUT_IMAGE_PNG, - 'eccLevel' => QRCode::ECC_L, + 'eccLevel' => QRCode::ECC_M, 'scale' => 12, + 'outputBase64' => false, ]); @@ - $qrcode = str_replace('data:image/png;base64,', '', $qrcode); - $qrcode = base64_decode($qrcode);Also applies to: 76-80
122-126: Make heading and venue text configurable (and sanitize address)Avoid hardcoded strings; pull from $saveOptions and strip tags from $address.
- $topText = "Scan to Record Your\nContribution"; - $venueLabel = "Venue: "; - $venueValue = $address; + $topText = $saveOptions['topText'] ?? "Scan to Record Your\nContribution"; + $venueLabel = $saveOptions['venueLabel'] ?? "Venue: "; + $venueValue = isset($address) ? \wp_strip_all_tags((string)$address) : '';
127-135: Canvas sizing should consider resized QR and measured TTF textUsing $qrWidth and fixed paddings risks clipping/overwide canvases. Compute widths from imagettfbbox and use $newQrWidth.
- $canvasWidth = $qrWidth + 100; - $canvasHeight = $newLogoHeight + $qrHeight + 220; + // After measuring text (see font block), recompute to fit content. + $canvasWidth = $newQrWidth + 100; // will be max'd against text widths later + $canvasHeight = $newLogoHeight + $newQrHeight + 220;Optionally: pre-measure top/bottom text with imagettfbbox and set canvasWidth = max(newQrWidth+100, maxTextWidth+40, newLogoWidth+40).
159-163: Avoid hardcoded QR Y-offset; place after measured text blockCompute text block height to prevent overlaps across font/line changes.
- $qrY = $topY + 70; + $textBlockHeight = (count($lines) ?: 1) * $lineHeight; + $qrY = $topY + $textBlockHeight + 20;
225-235: Propagate save result instead of always returning TRUEReturn the attachment (or FALSE) so callers can react to failures.
$saveOptions['baseFileName'] = $baseFileName; $saveOptions['entityId'] = $entityId; - self::saveQrCode($finalImage, $saveOptions); + $attachment = self::saveQrCode($finalImage, $saveOptions); + if ($attachment === FALSE) { + return FALSE; + } @@ - return TRUE; + return $attachment;
32-43: Null-guard Event and Address fetches to prevent fatals->first() may return null; indexing into it will fatal. Guard both lookups and bail with a clear log.
$event = Event::get(TRUE) ->addSelect('loc_block_id.address_id') ->addWhere('id', '=', $entityId) - ->execute()->first(); + ->execute()->first(); + if (!$event || empty($event['loc_block_id.address_id'])) { + \CRM_Core_Error::debug_log_message("Event or address not found for id {$entityId}"); + return FALSE; + } $addresses = Address::get(FALSE) ->addWhere('id', '=', $event['loc_block_id.address_id']) ->setLimit(1) - ->execute()->first(); - $address = \CRM_Utils_Address::format($addresses); + ->execute()->first(); + if (!$addresses) { + \CRM_Core_Error::debug_log_message("Address not found for event id {$entityId}"); + return FALSE; + } + $address = \CRM_Utils_Address::format($addresses);
45-74: Guard Collection_Camp lookup and missing keys$campData can be null and individual fields can be absent; current code will fatal or raise notices. Add null checks and use null-coalescing on reads.
- $campData = EckEntity::get('Collection_Camp', FALSE) + $campData = EckEntity::get('Collection_Camp', FALSE) ->addSelect('subtype:name', 'Collection_Camp_Intent_Details.Location_Area_of_camp', 'Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_', 'Goonj_Activities.Where_do_you_wish_to_organise_the_activity_', 'Institution_Collection_Camp_Intent.Collection_Camp_Address', 'Institution_Dropping_Center_Intent.Dropping_Center_Address', 'Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_') ->addWhere('id', '=', $entityId) ->execute()->first(); + if (!$campData) { + \CRM_Core_Error::debug_log_message("Collection Camp not found for id {$entityId}"); + return FALSE; + } - $campStatus = $campData['subtype:name']; + $campStatus = $campData['subtype:name'] ?? null; + if (!$campStatus) { + \CRM_Core_Error::debug_log_message("Missing subtype for Collection Camp id {$entityId}"); + return FALSE; + } @@ - $address = $campData['Collection_Camp_Intent_Details.Location_Area_of_camp']; + $address = $campData['Collection_Camp_Intent_Details.Location_Area_of_camp'] ?? ''; @@ - $address = $campData['Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_']; + $address = $campData['Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_'] ?? ''; @@ - $address = $campData['Goonj_Activities.Where_do_you_wish_to_organise_the_activity_']; + $address = $campData['Goonj_Activities.Where_do_you_wish_to_organise_the_activity_'] ?? ''; @@ - $address = $campData['Institution_Collection_Camp_Intent.Collection_Camp_Address']; + $address = $campData['Institution_Collection_Camp_Intent.Collection_Camp_Address'] ?? ''; @@ - $address = $campData['Institution_Dropping_Center_Intent.Dropping_Center_Address']; + $address = $campData['Institution_Dropping_Center_Intent.Dropping_Center_Address'] ?? ''; @@ - $address = $campData['Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_']; + $address = $campData['Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_'] ?? '';
80-82: Guard GD image creation for QR to avoid TypeError on imagesx/yimagecreatefromstring() can return false; check before measuring.
- $qrImage = imagecreatefromstring($qrcode); - $qrWidth = imagesx($qrImage); - $qrHeight = imagesy($qrImage); + $qrImage = imagecreatefromstring($qrcode); + if (!$qrImage) { + throw new \RuntimeException('Failed to create QR image from binary'); + } + $qrWidth = imagesx($qrImage); + $qrHeight = imagesy($qrImage);
100-107: Don’t hardcode dated logo URL or fetch via HTTP; prefer filesystem with fallbacksUse uploads basedir and allow overrides via $saveOptions; add failure checks.
- $upload_dir = wp_upload_dir(); - $logoUrl = $upload_dir['baseurl'] . '/2024/09/Goonj-logo-10June20-300x193-1.png'; - $logoData = file_get_contents($logoUrl); - $logoImage = imagecreatefromstring($logoData); + $upload_dir = \wp_upload_dir(); + $logoPath = $saveOptions['logoPath'] ?? ($upload_dir['basedir'] . '/brand/Goonj-logo.png'); + if (\file_exists($logoPath)) { + $logoImage = \imagecreatefrompng($logoPath); + } else { + $logoUrl = $saveOptions['logoUrl'] ?? null; + if (!$logoUrl) { + throw new \RuntimeException("Logo not found at $logoPath and no logoUrl provided"); + } + $resp = \wp_remote_get($logoUrl, ['timeout' => 5]); + $logoData = \wp_remote_retrieve_body($resp); + if (!$logoData) { + throw new \RuntimeException("Failed to download logo from $logoUrl"); + } + $logoImage = \imagecreatefromstring($logoData); + } + if (!$logoImage) { + throw new \RuntimeException('Failed to create logo image resource'); + }
136-139: Verify font files exist before calling imagettf APIs*Missing OTFs will emit warnings and break rendering; add explicit checks.
$fontPath = dirname(__DIR__, 2) . '/fonts/Fontspring-proximanova-bold.otf'; $fontPathRegular = dirname(__DIR__, 2) . '/fonts/Fontspring-proximanova-regular.otf'; + if (!\file_exists($fontPath) || !\file_exists($fontPathRegular)) { + throw new \RuntimeException('TTF/OTF font file(s) not found'); + }
164-212: Fix venue wrapping/centering; remove unused var and measure bold label separatelyCurrent wrap mixes fonts during measurement and leaves $currentWidth unused. Center the combined first line (bold label + regular value) and wrap value-only lines.
$bottomFontSize = 25; $maxWidth = $canvasWidth - 80; $bottomY = $qrY + $newQrHeight + 40; @@ - $lines = []; - $currentLine = $venueLabel; - $currentWidth = $labelWidth; - - $words = explode(" ", $venueValue); - foreach ($words as $word) { - $testLine = ($currentLine === $venueLabel ? $currentLine : $currentLine . " ") . $word; - $bboxTest = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $testLine); - $testWidth = abs($bboxTest[2] - $bboxTest[0]); - - if ($testWidth > $maxWidth) { - $lines[] = $currentLine; - $currentLine = $word; - } - else { - $currentLine = $testLine; - } - } - $lines[] = $currentLine; + $words = explode(' ', trim($venueValue)); + $valueLines = []; + $current = ''; + foreach ($words as $word) { + $test = $current === '' ? $word : $current . ' ' . $word; + $bbox = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $test); + $testW = abs($bbox[2] - $bbox[0]); + $allowed = empty($valueLines) ? ($maxWidth - $labelWidth) : $maxWidth; + if ($testW > $allowed && $current !== '') { + $valueLines[] = $current; + $current = $word; + } else { + $current = $test; + } + } + if ($current !== '') { + $valueLines[] = $current; + } @@ - foreach ($lines as $i => $line) { - $bbox = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $line); - $lineWidth = abs($bbox[2] - $bbox[0]); - $x = (int) (($canvasWidth - $lineWidth) / 2); - $y = $bottomY + ($i * $lineHeight); - - if ($i === 0) { - imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel); - $valuePart = trim(str_replace($venueLabel, "", $line)); - if ($valuePart !== "") { - $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel); - $offset = abs($bboxLabel[2] - $bboxLabel[0]); - imagettftext($canvas, $bottomFontSize, 0, $x + $offset, $y, $black, $fontPathRegular, $valuePart); - } - } - else { - imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $line); - } - } + foreach ($valueLines as $i => $valLine) { + $bboxVal = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $valLine); + $valW = abs($bboxVal[2] - $bboxVal[0]); + $lineW = ($i === 0) ? ($labelWidth + $valW) : $valW; + $x = (int)(($canvasWidth - $lineW) / 2); + $y = $bottomY + ($i * $lineHeight); + if ($i === 0) { + imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel); + imagettftext($canvas, $bottomFontSize, 0, $x + $labelWidth, $y, $black, $fontPathRegular, $valLine); + } else { + imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $valLine); + } + }
214-218: Validate PNG buffer before savingCheck imagepng return and ensure buffer is non-empty to avoid writing corrupt files.
ob_start(); - imagepng($canvas); - $finalImage = ob_get_clean(); + $ok = imagepng($canvas); + $finalImage = ob_get_clean(); + if (!$ok || $finalImage === '' || $finalImage === false) { + throw new \RuntimeException('Failed to render composite PNG'); + }
🧹 Nitpick comments (3)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (3)
115-120: Use imagecopyresampled for logo too (better quality)Resampling avoids jagged edges on scaling.
- imagecopyresized( + imagecopyresampled( $resizedLogo, $logoImage, 0, 0, 0, 0, $newLogoWidth, $newLogoHeight, $logoWidth, $logoHeight );
284-289: Prefix global civicrm_api3 for clarity and minor perfAvoid namespaced function lookup overhead.
- $result = civicrm_api3('Attachment', 'create', $params); + $result = \civicrm_api3('Attachment', 'create', $params);
25-30: Optional: bump ECC and let version auto-size for scan robustnessECC_M/H improves resilience; version=0 lets library pick minimal size.
No code sample beyond the earlier QROptions diff needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
wp-content/civi-extensions/goonjcustom/fonts/Fontspring-proximanova-bold.otfis excluded by!**/*.otfwp-content/civi-extensions/goonjcustom/fonts/Fontspring-proximanova-regular.otfis excluded by!**/*.otf
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-11T09:53:14.458Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#284
File: wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php:520-520
Timestamp: 2024-10-11T09:53:14.458Z
Learning: In `wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php`, it's acceptable that the `reGenerateCollectionCampQr` method calls `generateQrCode` with only the `$collectionCampId` parameter, as QR codes are generated when the Dropping Center is authorized. We'll implement changes if any issues arise in the future.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
📚 Learning: 2024-10-09T15:24:42.859Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#352
File: wp-content/civi-extensions/goonjcustom/Civi/QrCodeService.php:51-53
Timestamp: 2024-10-09T15:24:42.859Z
Learning: In `QrCodeService::generateQrCode`, when using `QRCode::OUTPUT_IMAGE_PNG`, the `render` method returns a data URI with base64-encoded image data. Therefore, it's necessary to remove the `data:image/png;base64,` prefix and base64-decode the data to obtain the raw binary image.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
🪛 PHPMD (2.15.0)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
175-175: Avoid unused local variables such as '$currentWidth'. (Unused Code Rules)
(UnusedLocalVariable)
🔇 Additional comments (2)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (2)
89-98: Alpha-safe resample for QR looks goodPreserving alpha and using imagecopyresampled is correct for QR quality.
146-157: Heading rendering is accurateMulti-line TTF with bbox centering looks good.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (10)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (10)
25-31: Stop base64-stripping; render raw PNG and validate GD resourceWith raw PNG output, stripping/decoding corrupts data. Also guard
imagecreatefromstring.$options = new QROptions([ 'version' => 5, 'outputType' => QRCode::OUTPUT_IMAGE_PNG, 'eccLevel' => QRCode::ECC_L, - 'scale' => 12, + 'scale' => 12, + // Ensure render() returns raw PNG, not a data URI. + 'outputBase64' => false, ]); @@ - $qrcode = str_replace('data:image/png;base64,', '', $qrcode); - $qrcode = base64_decode($qrcode); - $qrImage = imagecreatefromstring($qrcode); + // $qrcode already is raw PNG bytes. + $qrImage = imagecreatefromstring($qrcode); + if (!$qrImage) { + throw new \RuntimeException('Failed to create QR image from binary'); + } $qrWidth = imagesx($qrImage); $qrHeight = imagesy($qrImage);Also applies to: 76-83
122-126: Hard-coded user-facing text; make configurable (and translatable if needed)Allow callers to pass headline and venue label/value via
$saveOptions.- // Texts. - $topText = "Scan to Record Your\nContribution"; - $venueLabel = "Venue: "; - $venueValue = $address; + // Texts. + $topText = $saveOptions['topText'] ?? "Scan to Record Your\nContribution"; + $venueLabel = $saveOptions['venueLabel'] ?? "Venue: "; + $venueValue = $saveOptions['venue'] ?? ($address ?? '');
225-235: Propagate save result (attachment) instead of always TRUEReturn FALSE when save fails; otherwise return the created attachment for downstream use.
$baseFileName = "qr_code_{$entityId}.png"; $saveOptions['baseFileName'] = $baseFileName; $saveOptions['entityId'] = $entityId; - self::saveQrCode($finalImage, $saveOptions); + $attachment = self::saveQrCode($finalImage, $saveOptions); + if ($attachment === FALSE) { + return FALSE; + } + return $attachment; - } + } catch (\Exception $e) { \CRM_Core_Error::debug_log_message('Error generating QR code: ' . $e->getMessage()); return FALSE; } - return TRUE; + // Unreachable after early returns above. + return TRUE;
100-107: Don’t fetch logo over HTTP; use filesystem, validate, and resampleAvoid hardcoded month path and network fetch. Prefer uploads basedir and handle errors. Also use
imagecopyresampledfor quality.- // Load logo. - $upload_dir = wp_upload_dir(); - $logoUrl = $upload_dir['baseurl'] . '/2024/09/Goonj-logo-10June20-300x193-1.png'; - $logoData = file_get_contents($logoUrl); - $logoImage = imagecreatefromstring($logoData); + // Load logo (filesystem preferred; allow override). + $upload_dir = \wp_upload_dir(); + $logoPath = $saveOptions['logoPath'] ?? ($upload_dir['basedir'] . '/brand/Goonj-logo.png'); + if (\file_exists($logoPath)) { + $logoImage = imagecreatefrompng($logoPath); + } else { + $logoUrl = $saveOptions['logoUrl'] ?? null; + if (!$logoUrl) { + throw new \RuntimeException("Logo not found at $logoPath and no logoUrl provided"); + } + $resp = \wp_remote_get($logoUrl, ['timeout' => 5]); + $logoData = \wp_remote_retrieve_body($resp); + $logoImage = $logoData ? imagecreatefromstring($logoData) : false; + } + if (!$logoImage) { + throw new \RuntimeException('Failed to create logo image resource'); + } @@ - imagecopyresized( + imagecopyresampled( $resizedLogo, $logoImage, 0, 0, 0, 0, $newLogoWidth, $newLogoHeight, $logoWidth, $logoHeight );Also applies to: 115-120
127-131: Fix canvas sizing and QR placement (use resized dims and measured text height)Avoid clipping/overlap; base width/height on resized QR and heading block height.
- // Canvas: logo + top text + QR + bottom text. - $canvasWidth = $qrWidth + 100; - $canvasHeight = $newLogoHeight + $qrHeight + 220; + // Canvas: logo + heading + QR + venue. + $textBlockHeight = max(1, count($lines)) * ($lineHeight); + $canvasWidth = max($newQrWidth + 100, $newLogoWidth + 60); + $canvasHeight = $newLogoHeight + 40 + $textBlockHeight + 20 + $newQrHeight + 180; @@ - $qrY = $topY + 70; + $qrY = $topY + $textBlockHeight + 20; $qrX = (int) (($canvasWidth - $newQrWidth) / 2);Also applies to: 159-163
136-139: Verify font files exist; fail fast with clear error or fallbackAvoid warnings from missing OTF/TTF files.
// Fonts. $fontPath = dirname(__DIR__, 2) . '/fonts/Fontspring-proximanova-bold.otf'; $fontPathRegular = dirname(__DIR__, 2) . '/fonts/Fontspring-proximanova-regular.otf'; + if (!\file_exists($fontPath) || !\file_exists($fontPathRegular)) { + throw new \RuntimeException('Required font file(s) not found'); + }
164-212: Fix venue wrapping/centering; remove unused varMeasure bold label separately; wrap and center value correctly; drop
$currentWidth.// --- Step 4: Venue text with wrapping $bottomFontSize = 25; $maxWidth = $canvasWidth - 80; $bottomY = $qrY + $newQrHeight + 40; // Start with bold "Venue:". $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel); $labelWidth = abs($bboxLabel[2] - $bboxLabel[0]); - $lines = []; - $currentLine = $venueLabel; - $currentWidth = $labelWidth; - - $words = explode(" ", $venueValue); - foreach ($words as $word) { - $testLine = ($currentLine === $venueLabel ? $currentLine : $currentLine . " ") . $word; - $bboxTest = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $testLine); - $testWidth = abs($bboxTest[2] - $bboxTest[0]); - - if ($testWidth > $maxWidth) { - $lines[] = $currentLine; - $currentLine = $word; - } - else { - $currentLine = $testLine; - } - } - $lines[] = $currentLine; + $words = explode(' ', trim($venueValue ?? '')); + $valueLines = []; + $current = ''; + foreach ($words as $word) { + $test = $current === '' ? $word : $current . ' ' . $word; + $bbox = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $test); + $testW = abs($bbox[2] - $bbox[0]); + $allowed = empty($valueLines) ? ($maxWidth - $labelWidth) : $maxWidth; + if ($testW > $allowed && $current !== '') { + $valueLines[] = $current; + $current = $word; + } else { + $current = $test; + } + } + if ($current !== '') { + $valueLines[] = $current; + } $lineHeight = $bottomFontSize + 8; - foreach ($lines as $i => $line) { - $bbox = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $line); - $lineWidth = abs($bbox[2] - $bbox[0]); - $x = (int) (($canvasWidth - $lineWidth) / 2); - $y = $bottomY + ($i * $lineHeight); - - if ($i === 0) { - imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel); - $valuePart = trim(str_replace($venueLabel, "", $line)); - if ($valuePart !== "") { - $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel); - $offset = abs($bboxLabel[2] - $bboxLabel[0]); - imagettftext($canvas, $bottomFontSize, 0, $x + $offset, $y, $black, $fontPathRegular, $valuePart); - } - } - else { - imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $line); - } - } + foreach ($valueLines as $i => $valLine) { + $bboxVal = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $valLine); + $valW = abs($bboxVal[2] - $bboxVal[0]); + $lineW = ($i === 0) ? ($labelWidth + $valW) : $valW; + $x = (int) (($canvasWidth - $lineW) / 2); + $y = $bottomY + ($i * $lineHeight); + if ($i === 0) { + imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel); + imagettftext($canvas, $bottomFontSize, 0, $x + $labelWidth, $y, $black, $fontPathRegular, $valLine); + } else { + imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $valLine); + } + }
214-218: Validate PNG buffer before saving to avoid empty writesCheck
imagepng()result and buffer content.// Save final. ob_start(); - imagepng($canvas); - $finalImage = ob_get_clean(); + $ok = imagepng($canvas); + $finalImage = ob_get_clean(); + if (!$ok || $finalImage === '' || $finalImage === false) { + throw new \RuntimeException('Failed to render composite PNG'); + }
32-43: Guard Event/address lookups to avoid fatals; bail out cleanlyBoth
->first()calls can return null. Accessing offsets would fatal. Return FALSE with logs when missing.if (!empty($saveOptions['customGroupName']) && $saveOptions['customGroupName'] === 'Event_QR') { - $event = Event::get(FALSE) + $event = Event::get(FALSE) ->addSelect('loc_block_id.address_id') ->addWhere('id', '=', $entityId) - ->execute()->first(); + ->execute()->first(); + if (!$event || empty($event['loc_block_id.address_id'])) { + \CRM_Core_Error::debug_log_message("Event or address not found for id {$entityId}"); + return FALSE; + } - $addresses = Address::get(FALSE) + $addresses = Address::get(FALSE) ->addWhere('id', '=', $event['loc_block_id.address_id']) ->setLimit(1) - ->execute()->first(); - $address = \CRM_Utils_Address::format($addresses); + ->execute()->first(); + if (!$addresses) { + \CRM_Core_Error::debug_log_message("Address row missing for event id {$entityId}"); + return FALSE; + } + $address = \CRM_Utils_Address::format($addresses); }
45-74: Null-check camp fetch and guard missing keys
->first()may be null; individual keys may be absent. Prevent notices and undefined$address.- if (!empty($saveOptions['customGroupName']) && $saveOptions['customGroupName'] != 'Event_QR') { + if (!empty($saveOptions['customGroupName']) && $saveOptions['customGroupName'] != 'Event_QR') { $campData = EckEntity::get('Collection_Camp', FALSE) ->addSelect('subtype:name', 'Collection_Camp_Intent_Details.Location_Area_of_camp', 'Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_', 'Goonj_Activities.Where_do_you_wish_to_organise_the_activity_', 'Institution_Collection_Camp_Intent.Collection_Camp_Address', 'Institution_Dropping_Center_Intent.Dropping_Center_Address', 'Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_') ->addWhere('id', '=', $entityId) ->execute()->first(); + if (!$campData) { + \CRM_Core_Error::debug_log_message("Collection_Camp not found for id {$entityId}"); + return FALSE; + } - $campStatus = $campData['subtype:name']; + $campStatus = $campData['subtype:name'] ?? null; + if (!$campStatus) { + \CRM_Core_Error::debug_log_message("Missing subtype for Collection_Camp id {$entityId}"); + return FALSE; + } if ($campStatus == 'Collection_Camp') { - $address = $campData['Collection_Camp_Intent_Details.Location_Area_of_camp']; + $address = $campData['Collection_Camp_Intent_Details.Location_Area_of_camp'] ?? ''; } elseif ($campStatus == 'Dropping_Center') { - $address = $campData['Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_']; + $address = $campData['Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_'] ?? ''; } elseif ($campStatus == 'Goonj_Activities') { - $address = $campData['Goonj_Activities.Where_do_you_wish_to_organise_the_activity_']; + $address = $campData['Goonj_Activities.Where_do_you_wish_to_organise_the_activity_'] ?? ''; } elseif ($campStatus == 'Institution_Collection_Camp') { - $address = $campData['Institution_Collection_Camp_Intent.Collection_Camp_Address']; + $address = $campData['Institution_Collection_Camp_Intent.Collection_Camp_Address'] ?? ''; } elseif ($campStatus == 'Institution_Dropping_Center') { - $address = $campData['Institution_Dropping_Center_Intent.Dropping_Center_Address']; + $address = $campData['Institution_Dropping_Center_Intent.Dropping_Center_Address'] ?? ''; } elseif ($campStatus == 'Institution_Goonj_Activities') { - $address = $campData['Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_']; + $address = $campData['Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_'] ?? ''; } else { throw new \Exception('Invalid entity type for QR code generation.'); } }
🧹 Nitpick comments (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (1)
25-31: Consider higher ECC for field scanning reliabilityECC_L is the lowest. ECC_M/Q/H are more robust against logo overlays and print defects.
- 'eccLevel' => QRCode::ECC_L, + 'eccLevel' => QRCode::ECC_Q,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-11T09:53:14.458Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#284
File: wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php:520-520
Timestamp: 2024-10-11T09:53:14.458Z
Learning: In `wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php`, it's acceptable that the `reGenerateCollectionCampQr` method calls `generateQrCode` with only the `$collectionCampId` parameter, as QR codes are generated when the Dropping Center is authorized. We'll implement changes if any issues arise in the future.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
📚 Learning: 2024-10-09T15:24:42.859Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#352
File: wp-content/civi-extensions/goonjcustom/Civi/QrCodeService.php:51-53
Timestamp: 2024-10-09T15:24:42.859Z
Learning: In `QrCodeService::generateQrCode`, when using `QRCode::OUTPUT_IMAGE_PNG`, the `render` method returns a data URI with base64-encoded image data. Therefore, it's necessary to remove the `data:image/png;base64,` prefix and base64-decode the data to obtain the raw binary image.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
🪛 PHPMD (2.15.0)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
175-175: Avoid unused local variables such as '$currentWidth'. (Unused Code Rules)
(UnusedLocalVariable)
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (1)
25-31: Render raw PNG; drop base64 handlingSet outputBase64=false and stop stripping/decoding. Prevents corrupted data and simplifies IO.
@@ - $options = new QROptions([ + $options = new QROptions([ 'version' => 5, 'outputType' => QRCode::OUTPUT_IMAGE_PNG, 'eccLevel' => QRCode::ECC_L, - 'scale' => 12, + 'scale' => 12, + 'outputBase64' => false, ]); @@ - $qrcode = (new QRCode($options))->render($data); - $qrcode = str_replace('data:image/png;base64,', '', $qrcode); - $qrcode = base64_decode($qrcode); + $qrcode = (new QRCode($options))->render($data); @@ - // Remove the base64 header and decode the image data. - $qrcode = str_replace('data:image/png;base64,', '', $qrcode); - $qrcode = base64_decode($qrcode); + // Render already returns raw PNG bytes when outputBase64=false. + // $qrcode is raw PNG binary.Also applies to: 78-80, 306-311
♻️ Duplicate comments (8)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (8)
122-126: Make heading/venue text dynamic (and translatable)Don’t hardcode user-facing strings; pull from options with safe defaults.
- $topText = "Scan to Record Your\nContribution"; - $venueLabel = "Venue: "; - $venueValue = $address; + $topText = $saveOptions['topText'] ?? "Scan to Record Your\nContribution"; + $venueLabel = $saveOptions['venueLabel'] ?? "Venue: "; + $venueValue = $saveOptions['venue'] ?? ($address ?? '');
159-163: Position QR after measured heading; avoid overlapUse measured text height instead of a fixed 70px gap.
- $qrY = $topY + 70; + $textBlockHeight = (count($lines) ?: 1) * ($lineHeight); + $qrY = $topY + $textBlockHeight + 20;
228-235: Propagate save result instead of always returning TRUEReturn the attachment (or FALSE) so callers can act on failures.
- self::saveQrCode($finalImage, $saveOptions); + $attachment = self::saveQrCode($finalImage, $saveOptions); + if ($attachment === FALSE) { + return FALSE; + } + return $attachment; @@ - return TRUE; + return TRUE; // Unreachable if returning above; keep if you prefer boolean API.
80-83: Guard GD image creation to prevent TypeErrorimagecreatefromstring() may return false; check before imagesx/y.
- $qrImage = imagecreatefromstring($qrcode); - $qrWidth = imagesx($qrImage); - $qrHeight = imagesy($qrImage); + $qrImage = imagecreatefromstring($qrcode); + if (!$qrImage) { + throw new \RuntimeException('Failed to create QR image from binary'); + } + $qrWidth = imagesx($qrImage); + $qrHeight = imagesy($qrImage);
100-107: Don’t hardcode logo URL; prefer filesystem and handle errorsAvoid HTTP fetch, year/month coupling, and missing-file fatals.
- $upload_dir = wp_upload_dir(); - $logoUrl = $upload_dir['baseurl'] . '/2024/09/Goonj-logo-10June20-300x193-1.png'; - $logoData = file_get_contents($logoUrl); - $logoImage = imagecreatefromstring($logoData); + $upload_dir = \wp_upload_dir(); + $logoPath = $saveOptions['logoPath'] ?? ($upload_dir['basedir'] . '/brand/Goonj-logo.png'); + if (\file_exists($logoPath)) { + $logoImage = imagecreatefrompng($logoPath); + } + else { + $logoUrl = $saveOptions['logoUrl'] ?? null; + if (!$logoUrl) { + throw new \RuntimeException("Logo not found at $logoPath and no logoUrl provided"); + } + $resp = \wp_remote_get($logoUrl, ['timeout' => 5]); + $logoData = \wp_remote_retrieve_body($resp); + if (!$logoData) { + throw new \RuntimeException("Failed to download logo from $logoUrl"); + } + $logoImage = imagecreatefromstring($logoData); + } + if (!$logoImage) { + throw new \RuntimeException('Failed to create logo image resource'); + }
136-139: Verify TTF files exist before renderingAvoid warnings and broken text when fonts are missing.
$fontPath = dirname(__DIR__, 2) . '/fonts/Fontspring-proximanova-bold.otf'; $fontPathRegular = dirname(__DIR__, 2) . '/fonts/Fontspring-proximanova-regular.otf'; + if (!\file_exists($fontPath) || !\file_exists($fontPathRegular)) { + throw new \RuntimeException('TTF font file(s) not found'); + }
214-218: Validate PNG buffer before savingProtect against empty/failed renders.
ob_start(); - imagepng($canvas); - $finalImage = ob_get_clean(); + $ok = imagepng($canvas); + $finalImage = ob_get_clean(); + if (!$ok || $finalImage === '' || $finalImage === false) { + throw new \RuntimeException('Failed to render composite PNG'); + }
32-43: Guard Event/address lookups to avoid fatalsfirst() can return null; bail out cleanly if missing.
$event = Event::get(FALSE) ->addSelect('loc_block_id.address_id') ->addWhere('id', '=', $entityId) - ->execute()->first(); + ->execute()->first(); + if (!$event || empty($event['loc_block_id.address_id'])) { + \CRM_Core_Error::debug_log_message("Event/address missing for id {$entityId}"); + return FALSE; + } $addresses = Address::get(FALSE) ->addWhere('id', '=', $event['loc_block_id.address_id']) ->setLimit(1) - ->execute()->first(); + ->execute()->first(); + if (!$addresses) { + \CRM_Core_Error::debug_log_message("Address row not found for id {$entityId}"); + return FALSE; + } $address = \CRM_Utils_Address::format($addresses);
🧹 Nitpick comments (7)
wp-content/civi-extensions/goonjcustom/Civi/DroppingCenterService.php (1)
298-304: Check poster QR save result and log on failureHandle failures from generateQrCodeForPoster to avoid silent drops.
- self::generateQrCodeForPoster($data, $id, $saveOptionsForPoster); + $poster = self::generateQrCodeForPoster($data, $id, $saveOptionsForPoster); + if ($poster === FALSE) { + \Civi::log()->warning("Poster QR save failed for Dropping Center {$id}"); + }wp-content/civi-extensions/goonjcustom/Civi/InstitutionDroppingCenterService.php (1)
616-623: Guard poster QR generation resultSame handling as main QR; improves debuggability.
- self::generateQrCodeForPoster($data, $id, $saveOptionsForPoster); + $poster = self::generateQrCodeForPoster($data, $id, $saveOptionsForPoster); + if ($poster === FALSE) { + \Civi::log()->warning("Poster QR save failed for Institution Dropping Center {$id}"); + }wp-content/civi-extensions/goonjcustom/Civi/InstitutionCollectionCampService.php (1)
536-542: Guard poster QR generation resultMirror main QR handling.
- self::generateQrCodeForPoster($data, $id, $saveOptionsForPoster); + $poster = self::generateQrCodeForPoster($data, $id, $saveOptionsForPoster); + if ($poster === FALSE) { + \Civi::log()->warning("Poster QR save failed for Institution Collection Camp {$id}"); + }wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (1)
533-540: Standardize call order across servicesOther services call poster first, then main; this one calls main first. Pick one order for consistency (prefer poster then main).
- self::generateQrCode($data, $id, $saveOptions); - self::generateQrCodeForPoster($data, $id, $saveOptionsForPoster); + self::generateQrCodeForPoster($data, $id, $saveOptionsForPoster); + self::generateQrCode($data, $id, $saveOptions);wp-content/civi-extensions/goonjcustom/Civi/GoonjInitiatedEventsService.php (1)
584-589: Don’t skip poster generation when the main Event QR already existsEarly return is based only on
Event_QR.QR_Code. If legacy events already have the main QR but lack the poster field, the poster won’t be generated. Consider fetchingEvent_QR.QR_Code_For_Posteras well and only return when both exist.wp-content/civi-extensions/goonjcustom/Civi/InstitutionGoonjActivitiesService.php (1)
493-499: Poster QR path looks good; consider idempotency and constants
- LGTM to generate poster first, then main QR.
- Optional: check whether
Collection_Camp_QR_Code.QR_Code_For_Posteralready exists to avoid redundant writes and churn.- Extract
'Collection_Camp_QR_Code'and'QR_Code_For_Poster'as shared constants (or centralize in QrCodeable) to remove duplication across services.wp-content/civi-extensions/goonjcustom/Civi/GoonjActivitiesService.php (1)
606-612: Consistent poster QR generation; minor polish
- Implementation matches the Collection Camp schema; poster first, then main QR.
- As above, consider an existence check to keep writes idempotent and lift group/field strings into shared constants.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php(1 hunks)wp-content/civi-extensions/goonjcustom/Civi/DroppingCenterService.php(1 hunks)wp-content/civi-extensions/goonjcustom/Civi/GoonjActivitiesService.php(1 hunks)wp-content/civi-extensions/goonjcustom/Civi/GoonjInitiatedEventsService.php(1 hunks)wp-content/civi-extensions/goonjcustom/Civi/InstitutionCollectionCampService.php(1 hunks)wp-content/civi-extensions/goonjcustom/Civi/InstitutionDroppingCenterService.php(1 hunks)wp-content/civi-extensions/goonjcustom/Civi/InstitutionGoonjActivitiesService.php(1 hunks)wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-11T09:53:14.458Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#284
File: wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php:520-520
Timestamp: 2024-10-11T09:53:14.458Z
Learning: In `wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php`, it's acceptable that the `reGenerateCollectionCampQr` method calls `generateQrCode` with only the `$collectionCampId` parameter, as QR codes are generated when the Dropping Center is authorized. We'll implement changes if any issues arise in the future.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/InstitutionGoonjActivitiesService.phpwp-content/civi-extensions/goonjcustom/Civi/InstitutionDroppingCenterService.phpwp-content/civi-extensions/goonjcustom/Civi/DroppingCenterService.phpwp-content/civi-extensions/goonjcustom/Civi/GoonjActivitiesService.phpwp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.phpwp-content/civi-extensions/goonjcustom/Civi/InstitutionCollectionCampService.phpwp-content/civi-extensions/goonjcustom/Civi/GoonjInitiatedEventsService.phpwp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
📚 Learning: 2024-10-09T15:24:42.859Z
Learnt from: belwalshubham
PR: ColoredCow/goonj#352
File: wp-content/civi-extensions/goonjcustom/Civi/QrCodeService.php:51-53
Timestamp: 2024-10-09T15:24:42.859Z
Learning: In `QrCodeService::generateQrCode`, when using `QRCode::OUTPUT_IMAGE_PNG`, the `render` method returns a data URI with base64-encoded image data. Therefore, it's necessary to remove the `data:image/png;base64,` prefix and base64-decode the data to obtain the raw binary image.
Applied to files:
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
🧬 Code graph analysis (7)
wp-content/civi-extensions/goonjcustom/Civi/InstitutionGoonjActivitiesService.php (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (1)
generateQrCodeForPoster(297-325)
wp-content/civi-extensions/goonjcustom/Civi/InstitutionDroppingCenterService.php (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (1)
generateQrCodeForPoster(297-325)
wp-content/civi-extensions/goonjcustom/Civi/DroppingCenterService.php (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (1)
generateQrCodeForPoster(297-325)
wp-content/civi-extensions/goonjcustom/Civi/GoonjActivitiesService.php (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (1)
generateQrCodeForPoster(297-325)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (2)
generateQrCode(23-236)generateQrCodeForPoster(297-325)
wp-content/civi-extensions/goonjcustom/Civi/InstitutionCollectionCampService.php (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (1)
generateQrCodeForPoster(297-325)
wp-content/civi-extensions/goonjcustom/Civi/GoonjInitiatedEventsService.php (1)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php (1)
generateQrCodeForPoster(297-325)
🪛 PHPMD (2.15.0)
wp-content/civi-extensions/goonjcustom/Civi/GoonjInitiatedEventsService.php
589-589: Avoid unused local variables such as '$data'. (Unused Code Rules)
(UnusedLocalVariable)
589-589: Avoid unused local variables such as '$id'. (Unused Code Rules)
(UnusedLocalVariable)
wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php
175-175: Avoid unused local variables such as '$currentWidth'. (Unused Code Rules)
(UnusedLocalVariable)
Comment on lines
+584
to
+587
| $saveOptionsForPoster = [ | ||
| 'customGroupName' => 'Collection_Camp_QR_Code', | ||
| 'customFieldName' => 'QR_Code_For_Poster', | ||
| ]; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Poster QR saved under Collection_Camp_QR_Code for an Event — likely wrong custom group
The standard QR for events uses customGroupName 'Event_QR'. Saving the poster QR into 'Collection_Camp_QR_Code' will either fail to resolve the field or write to the wrong entity. Use the Event group for consistency unless there’s a schema difference you intend.
Suggested fix:
- $saveOptionsForPoster = [
- 'customGroupName' => 'Collection_Camp_QR_Code',
- 'customFieldName' => 'QR_Code_For_Poster',
- ];
+ $saveOptionsForPoster = [
+ 'customGroupName' => 'Event_QR',
+ 'customFieldName' => 'QR_Code_For_Poster',
+ ];Verification script:
🏁 Script executed:
#!/bin/bash
# Verify where QR_Code_For_Poster is defined and used.
rg -n -C2 -S "QR_Code_For_Poster|Event_QR|Collection_Camp_QR_Code" wp-content/civi-extensions
# Check other services for the expected group/field pairing for Event vs Collection_Camp.
rg -n -C2 -S "customGroupName' => 'Event_QR'|customGroupName' => 'Collection_Camp_QR_Code'" wp-content/civi-extensionsLength of output: 19392
Use Event_QR as the custom group for event poster QR
In GoonjInitiatedEventsService.php (lines 584–587) $saveOptionsForPoster uses 'Collection_Camp_QR_Code', which routes poster QR saving through the camp branch. Change to 'Event_QR' so the poster QR is saved under the event group.
- $saveOptionsForPoster = [
- 'customGroupName' => 'Collection_Camp_QR_Code',
- 'customFieldName' => 'QR_Code_For_Poster',
- ];
+ $saveOptionsForPoster = [
+ 'customGroupName' => 'Event_QR',
+ 'customFieldName' => 'QR_Code_For_Poster',
+ ];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $saveOptionsForPoster = [ | |
| 'customGroupName' => 'Collection_Camp_QR_Code', | |
| 'customFieldName' => 'QR_Code_For_Poster', | |
| ]; | |
| $saveOptionsForPoster = [ | |
| 'customGroupName' => 'Event_QR', | |
| 'customFieldName' => 'QR_Code_For_Poster', | |
| ]; |
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/GoonjInitiatedEventsService.php
around lines 584 to 587, the $saveOptionsForPoster array sets 'customGroupName'
to 'Collection_Camp_QR_Code' which incorrectly routes poster QR saves to the
camp group; change the 'customGroupName' value to 'Event_QR' so poster QR
entries are saved under the Event_QR custom group (leave the 'customFieldName'
as 'QR_Code_For_Poster').
| 'customFieldName' => 'QR_Code_For_Poster', | ||
| ]; | ||
|
|
||
| self::generateQrCodeForPoster($data, $id, $saveOptionsForPoster); |
There was a problem hiding this comment.
Undefined variables passed to generateQrCodeForPoster() — runtime fatal
$data and $id are not defined in this scope. You likely meant $qrCodeData and $eventId.
Apply:
- self::generateQrCodeForPoster($data, $id, $saveOptionsForPoster);
+ self::generateQrCodeForPoster($qrCodeData, $eventId, $saveOptionsForPoster);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| self::generateQrCodeForPoster($data, $id, $saveOptionsForPoster); | |
| self::generateQrCodeForPoster($qrCodeData, $eventId, $saveOptionsForPoster); |
🧰 Tools
🪛 PHPMD (2.15.0)
589-589: Avoid unused local variables such as '$data'. (Unused Code Rules)
(UnusedLocalVariable)
589-589: Avoid unused local variables such as '$id'. (Unused Code Rules)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/GoonjInitiatedEventsService.php
around line 589, the call uses undefined variables $data and $id; replace them
with the correct variables $qrCodeData and $eventId (i.e., call
self::generateQrCodeForPoster($qrCodeData, $eventId, $saveOptionsForPoster));
ensure those variables are in scope and populated before the call, or if
differently named earlier, use the existing variable names consistently and run
static analysis to confirm no other mismatches.
Comment on lines
+45
to
+74
| if (!empty($saveOptions['customGroupName']) && $saveOptions['customGroupName'] != 'Event_QR') { | ||
| $campData = EckEntity::get('Collection_Camp', FALSE) | ||
| ->addSelect('subtype:name', 'Collection_Camp_Intent_Details.Location_Area_of_camp', 'Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_', 'Goonj_Activities.Where_do_you_wish_to_organise_the_activity_', 'Institution_Collection_Camp_Intent.Collection_Camp_Address', 'Institution_Dropping_Center_Intent.Dropping_Center_Address', 'Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_') | ||
| ->addWhere('id', '=', $entityId) | ||
| ->execute()->first(); | ||
|
|
||
| $campStatus = $campData['subtype:name']; | ||
|
|
||
| if ($campStatus == 'Collection_Camp') { | ||
| $address = $campData['Collection_Camp_Intent_Details.Location_Area_of_camp']; | ||
| } | ||
| elseif ($campStatus == 'Dropping_Center') { | ||
| $address = $campData['Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_']; | ||
| } | ||
| elseif ($campStatus == 'Goonj_Activities') { | ||
| $address = $campData['Goonj_Activities.Where_do_you_wish_to_organise_the_activity_']; | ||
| } | ||
| elseif ($campStatus == 'Institution_Collection_Camp') { | ||
| $address = $campData['Institution_Collection_Camp_Intent.Collection_Camp_Address']; | ||
| } | ||
| elseif ($campStatus == 'Institution_Dropping_Center') { | ||
| $address = $campData['Institution_Dropping_Center_Intent.Dropping_Center_Address']; | ||
| } | ||
| elseif ($campStatus == 'Institution_Goonj_Activities') { | ||
| $address = $campData['Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_']; | ||
| } | ||
| else { | ||
| throw new \Exception('Invalid entity type for QR code generation.'); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Null-check camp data and use safe indexing
Avoid undefined index notices and handle unknown subtype.
- $campData = EckEntity::get('Collection_Camp', FALSE)
+ $campData = EckEntity::get('Collection_Camp', FALSE)
->addSelect('subtype:name', 'Collection_Camp_Intent_Details.Location_Area_of_camp', 'Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_', 'Goonj_Activities.Where_do_you_wish_to_organise_the_activity_', 'Institution_Collection_Camp_Intent.Collection_Camp_Address', 'Institution_Dropping_Center_Intent.Dropping_Center_Address', 'Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_')
->addWhere('id', '=', $entityId)
->execute()->first();
+ if (!$campData) {
+ \CRM_Core_Error::debug_log_message("Camp not found for id {$entityId}");
+ return FALSE;
+ }
- $campStatus = $campData['subtype:name'];
+ $campStatus = $campData['subtype:name'] ?? null;
+ if (!$campStatus) {
+ throw new \RuntimeException('Missing subtype for QR generation');
+ }
@@
- $address = $campData['Collection_Camp_Intent_Details.Location_Area_of_camp'];
+ $address = $campData['Collection_Camp_Intent_Details.Location_Area_of_camp'] ?? '';
@@
- $address = $campData['Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_'];
+ $address = $campData['Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_'] ?? '';
@@
- $address = $campData['Goonj_Activities.Where_do_you_wish_to_organise_the_activity_'];
+ $address = $campData['Goonj_Activities.Where_do_you_wish_to_organise_the_activity_'] ?? '';
@@
- $address = $campData['Institution_Collection_Camp_Intent.Collection_Camp_Address'];
+ $address = $campData['Institution_Collection_Camp_Intent.Collection_Camp_Address'] ?? '';
@@
- $address = $campData['Institution_Dropping_Center_Intent.Dropping_Center_Address'];
+ $address = $campData['Institution_Dropping_Center_Intent.Dropping_Center_Address'] ?? '';
@@
- $address = $campData['Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_'];
+ $address = $campData['Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_'] ?? '';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!empty($saveOptions['customGroupName']) && $saveOptions['customGroupName'] != 'Event_QR') { | |
| $campData = EckEntity::get('Collection_Camp', FALSE) | |
| ->addSelect('subtype:name', 'Collection_Camp_Intent_Details.Location_Area_of_camp', 'Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_', 'Goonj_Activities.Where_do_you_wish_to_organise_the_activity_', 'Institution_Collection_Camp_Intent.Collection_Camp_Address', 'Institution_Dropping_Center_Intent.Dropping_Center_Address', 'Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_') | |
| ->addWhere('id', '=', $entityId) | |
| ->execute()->first(); | |
| $campStatus = $campData['subtype:name']; | |
| if ($campStatus == 'Collection_Camp') { | |
| $address = $campData['Collection_Camp_Intent_Details.Location_Area_of_camp']; | |
| } | |
| elseif ($campStatus == 'Dropping_Center') { | |
| $address = $campData['Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_']; | |
| } | |
| elseif ($campStatus == 'Goonj_Activities') { | |
| $address = $campData['Goonj_Activities.Where_do_you_wish_to_organise_the_activity_']; | |
| } | |
| elseif ($campStatus == 'Institution_Collection_Camp') { | |
| $address = $campData['Institution_Collection_Camp_Intent.Collection_Camp_Address']; | |
| } | |
| elseif ($campStatus == 'Institution_Dropping_Center') { | |
| $address = $campData['Institution_Dropping_Center_Intent.Dropping_Center_Address']; | |
| } | |
| elseif ($campStatus == 'Institution_Goonj_Activities') { | |
| $address = $campData['Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_']; | |
| } | |
| else { | |
| throw new \Exception('Invalid entity type for QR code generation.'); | |
| } | |
| } | |
| if (!empty($saveOptions['customGroupName']) && $saveOptions['customGroupName'] != 'Event_QR') { | |
| $campData = EckEntity::get('Collection_Camp', FALSE) | |
| ->addSelect( | |
| 'subtype:name', | |
| 'Collection_Camp_Intent_Details.Location_Area_of_camp', | |
| 'Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_', | |
| 'Goonj_Activities.Where_do_you_wish_to_organise_the_activity_', | |
| 'Institution_Collection_Camp_Intent.Collection_Camp_Address', | |
| 'Institution_Dropping_Center_Intent.Dropping_Center_Address', | |
| 'Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_' | |
| ) | |
| ->addWhere('id', '=', $entityId) | |
| ->execute() | |
| ->first(); | |
| if (!$campData) { | |
| \CRM_Core_Error::debug_log_message("Camp not found for id {$entityId}"); | |
| return FALSE; | |
| } | |
| $campStatus = $campData['subtype:name'] ?? null; | |
| if (!$campStatus) { | |
| throw new \RuntimeException('Missing subtype for QR generation'); | |
| } | |
| if ($campStatus == 'Collection_Camp') { | |
| $address = $campData['Collection_Camp_Intent_Details.Location_Area_of_camp'] ?? ''; | |
| } | |
| elseif ($campStatus == 'Dropping_Center') { | |
| $address = $campData['Dropping_Centre.Where_do_you_wish_to_open_dropping_center_Address_'] ?? ''; | |
| } | |
| elseif ($campStatus == 'Goonj_Activities') { | |
| $address = $campData['Goonj_Activities.Where_do_you_wish_to_organise_the_activity_'] ?? ''; | |
| } | |
| elseif ($campStatus == 'Institution_Collection_Camp') { | |
| $address = $campData['Institution_Collection_Camp_Intent.Collection_Camp_Address'] ?? ''; | |
| } | |
| elseif ($campStatus == 'Institution_Dropping_Center') { | |
| $address = $campData['Institution_Dropping_Center_Intent.Dropping_Center_Address'] ?? ''; | |
| } | |
| elseif ($campStatus == 'Institution_Goonj_Activities') { | |
| $address = $campData['Institution_Goonj_Activities.Where_do_you_wish_to_organise_the_activity_'] ?? ''; | |
| } | |
| else { | |
| throw new \Exception('Invalid entity type for QR code generation.'); | |
| } | |
| } |
Comment on lines
+164
to
+176
| // --- Step 4: Venue text with wrapping | ||
| $bottomFontSize = 25; | ||
| $maxWidth = $canvasWidth - 80; | ||
| $bottomY = $qrY + $newQrHeight + 40; | ||
|
|
||
| // Start with bold "Venue:". | ||
| $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel); | ||
| $labelWidth = abs($bboxLabel[2] - $bboxLabel[0]); | ||
|
|
||
| $lines = []; | ||
| $currentLine = $venueLabel; | ||
| $currentWidth = $labelWidth; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix venue wrapping/centering and remove unused variable
Measure bold label separately, wrap only the value, and center combined line. Drop $currentWidth (PHPMD).
- $lines = [];
- $currentLine = $venueLabel;
- $currentWidth = $labelWidth;
-
- $words = explode(" ", $venueValue);
- foreach ($words as $word) {
- $testLine = ($currentLine === $venueLabel ? $currentLine : $currentLine . " ") . $word;
- $bboxTest = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $testLine);
- $testWidth = abs($bboxTest[2] - $bboxTest[0]);
-
- if ($testWidth > $maxWidth) {
- $lines[] = $currentLine;
- $currentLine = $word;
- }
- else {
- $currentLine = $testLine;
- }
- }
- $lines[] = $currentLine;
+ $words = explode(' ', trim($venueValue));
+ $valueLines = [];
+ $current = '';
+ foreach ($words as $word) {
+ $test = $current === '' ? $word : $current . ' ' . $word;
+ $bbox = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $test);
+ $testW = abs($bbox[2] - $bbox[0]);
+ $allowed = empty($valueLines) ? ($maxWidth - $labelWidth) : $maxWidth;
+ if ($testW > $allowed && $current !== '') {
+ $valueLines[] = $current;
+ $current = $word;
+ } else {
+ $current = $test;
+ }
+ }
+ if ($current !== '') {
+ $valueLines[] = $current;
+ }
@@
- foreach ($lines as $i => $line) {
- $bbox = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $line);
- $lineWidth = abs($bbox[2] - $bbox[0]);
- $x = (int) (($canvasWidth - $lineWidth) / 2);
- $y = $bottomY + ($i * $lineHeight);
-
- if ($i === 0) {
- imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel);
- $valuePart = trim(str_replace($venueLabel, "", $line));
- if ($valuePart !== "") {
- $bboxLabel = imagettfbbox($bottomFontSize, 0, $fontPath, $venueLabel);
- $offset = abs($bboxLabel[2] - $bboxLabel[0]);
- imagettftext($canvas, $bottomFontSize, 0, $x + $offset, $y, $black, $fontPathRegular, $valuePart);
- }
- }
- else {
- imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $line);
- }
- }
+ foreach ($valueLines as $i => $valLine) {
+ $bboxVal = imagettfbbox($bottomFontSize, 0, $fontPathRegular, $valLine);
+ $valW = abs($bboxVal[2] - $bboxVal[0]);
+ $lineW = ($i === 0) ? ($labelWidth + $valW) : $valW;
+ $x = (int) (($canvasWidth - $lineW) / 2);
+ $y = $bottomY + ($i * $lineHeight);
+ if ($i === 0) {
+ imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPath, $venueLabel);
+ imagettftext($canvas, $bottomFontSize, 0, $x + $labelWidth, $y, $black, $fontPathRegular, $valLine);
+ } else {
+ imagettftext($canvas, $bottomFontSize, 0, $x, $y, $black, $fontPathRegular, $valLine);
+ }
+ }Also applies to: 193-212
🧰 Tools
🪛 PHPMD (2.15.0)
175-175: Avoid unused local variables such as '$currentWidth'. (Unused Code Rules)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php around
lines 164 to 176, the venue text wrapping and centering logic is incorrect and
an unused variable $currentWidth is present; to fix, compute the bounding box
and width for the bold "Venue:" label alone, then perform word-wrapping only on
the venue value using the remaining width (maxWidth minus labelWidth), create
combined lines by prepending the label to the first wrapped line only, center
the combined text block horizontally by calculating total combined line widths
per line, and remove the unused $currentWidth variable; apply the same changes
to the corresponding block at lines 193-212.
Comment on lines
+297
to
+305
| public static function generateQrCodeForPoster($data, $entityId, $saveOptions) { | ||
| try { | ||
| $options = new QROptions([ | ||
| 'version' => 5, | ||
| 'outputType' => QRCode::OUTPUT_IMAGE_PNG, | ||
| 'eccLevel' => QRCode::ECC_L, | ||
| 'scale' => 10, | ||
| ]); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unify poster path with main QR flow: raw PNG + return attachment
Mirror the raw-bytes rendering and return value.
$options = new QROptions([
'version' => 5,
'outputType' => QRCode::OUTPUT_IMAGE_PNG,
'eccLevel' => QRCode::ECC_L,
- 'scale' => 10,
+ 'scale' => 10,
+ 'outputBase64' => false,
]);
- $qrcode = (new QRCode($options))->render($data);
-
- // Remove the base64 header and decode the image data.
- $qrcode = str_replace('data:image/png;base64,', '', $qrcode);
- $qrcode = base64_decode($qrcode);
+ $qrcode = (new QRCode($options))->render($data);
@@
- self::saveQrCode($qrcode, $saveOptions);
+ $attachment = self::saveQrCode($qrcode, $saveOptions);
+ if ($attachment === FALSE) {
+ return FALSE;
+ }
+ return $attachment;
@@
- return TRUE;
+ return TRUE; // If keeping boolean API.Also applies to: 312-325
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/Traits/QrCodeable.php around
lines 297-305 (and similarly 312-325), the poster-specific flow currently
diverges from the main QR flow by not returning raw PNG bytes and an attachment
object; change it to mirror the main flow: render the QR as raw PNG bytes (not
only saving to disk), create and return the same attachment structure used
elsewhere (filename, mimetype image/png, raw data/blob), and remove any
alternate return types so both paths consistently produce the raw PNG +
attachment object; ensure options, output type, and ECC match the main flow and
reuse the same attachment creation logic.
This was referenced Sep 11, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add the new QR code
Summary by CodeRabbit
New Features
Enhancements