fix: add badge data to show on swal popup on badge check in #671
Conversation
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR adds badge data extraction functionality to display proper information in the SweetAlert popup when checking in badges via QR code scanning. The change replaces a previously used qrValid variable with a new utility function that parses badge QR codes.
- Introduces a new
formatBadgeQRutility function to parse QR codes into structured badge data - Updates the badge check-in page to use the new function for displaying badge holder information in the success popup
- Adds a minor improvement by setting the type attribute on a button element
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/utils/methods.js | Adds new formatBadgeQR function to parse badge QR codes and extract badge prefix, ticket number, email, and full name |
| src/pages/attendees/badge-checkin-page.js | Updates badge check-in flow to use formatBadgeQR for extracting and displaying badge data in the success popup; adds type="button" attribute to camera switch button |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setScanning(true); | ||
| checkInBadge(data) | ||
| .then(() => { | ||
| const badgeData = formatBadgeQR(data, currentSummit); |
There was a problem hiding this comment.
The formatBadgeQR function can return false when the QR code format is invalid, but this code doesn't handle that case. Attempting to access properties like fullName and email on false will cause a runtime error. Add validation to check if badgeData is falsy before accessing its properties, and handle the error case appropriately.
| const badgeData = formatBadgeQR(data, currentSummit); | |
| const badgeData = formatBadgeQR(data, currentSummit); | |
| if (!badgeData) { | |
| Swal.fire( | |
| "Error", | |
| "Invalid badge QR code format. Please try again.", | |
| "warning" | |
| ); | |
| return; | |
| } |
|
@rabbit review |
src/utils/methods.js
Outdated
| export const formatBadgeQR = (code, summit) => { | ||
| const qrCodeArray = code.split(summit.qr_registry_field_delimiter); | ||
|
|
||
| if (qrCodeArray.length > TWO && qrCodeArray[0] === summit.badge_qr_prefix) { |
There was a problem hiding this comment.
here u check for TWO
- its does not provided any readability
- u are accessing 4 elements
so lets change TWO by const QR_ELEMENTS_SIZE = 4
and check that qrCodeArray.length =>QR_ELEMENTS_SIZE
| import { formatBadgeQR } from "../../utils/methods"; | ||
| import styles from "../../styles/badge-checkin-page.module.less"; | ||
|
|
||
| const BadgeCheckinPage = ({ match, currentSummit, checkInBadge }) => { |
There was a problem hiding this comment.
current summit is never used removed it if not used
…badge data Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdded badge QR parsing: a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/utils/constants.js (1)
228-228: Fix typo in constant name and adjust value to match usage.Two issues with this constant:
- Typo: "MINIMUN" should be "MINIMUM"
- Incorrect value: The constant is set to
2, butformatBadgeQRchecksqrCodeArray.length > BADGE_QR_MINIMUN_EXPECTED_FIELDS(i.e.,> 2or>= 3) and then accessesqrCodeArray[3], which requires at least 4 elements. This mismatch can cause undefined values when accessing optional fields.Based on the past review comments, the recommendation is to set this to
4and use>=comparison, or rename to clarify intent.🔎 Proposed fix
-export const BADGE_QR_MINIMUN_EXPECTED_FIELDS = 2; +export const BADGE_QR_MINIMUM_EXPECTED_FIELDS = 4;Then update the comparison in
formatBadgeQR(src/utils/methods.js) to use>=:- if ( - qrCodeArray.length > BADGE_QR_MINIMUN_EXPECTED_FIELDS && - qrCodeArray[0] === summit.badge_qr_prefix - ) { + if ( + qrCodeArray.length >= BADGE_QR_MINIMUM_EXPECTED_FIELDS && + qrCodeArray[0] === summit.badge_qr_prefix + ) {Also update the import in both files to use the corrected name.
src/utils/methods.js (2)
33-34: Update constant name after typo fix.Once the typo in the constant name is fixed in
constants.js(MINIMUN→MINIMUM), update this import accordingly.
509-525: Add defensive null checks and fix array access logic.This function has several issues:
- Missing null checks: No validation that
summit,summit.qr_registry_field_delimiter, orsummit.badge_qr_prefixexist before accessing them, which could cause runtime errors.- Missing type validation: No check that
codeis a string before calling.split().- Logic bug: The condition checks
qrCodeArray.length > BADGE_QR_MINIMUN_EXPECTED_FIELDS(i.e.,> 2), but then accessesqrCodeArray[3]. If length is 3, accessing index 3 returnsundefined. The check should be>= 4to safely access all four indices.🔎 Proposed fix with defensive checks
export const formatBadgeQR = (code, summit) => { + // Validate inputs + if ( + typeof code !== "string" || + !summit || + !summit.qr_registry_field_delimiter || + !summit.badge_qr_prefix + ) { + return null; + } + const qrCodeArray = code.split(summit.qr_registry_field_delimiter); if ( - qrCodeArray.length > BADGE_QR_MINIMUN_EXPECTED_FIELDS && + qrCodeArray.length >= BADGE_QR_MINIMUM_EXPECTED_FIELDS && qrCodeArray[0] === summit.badge_qr_prefix ) { return { badgePrefix: qrCodeArray[0], ticketNumber: qrCodeArray[1], email: qrCodeArray[2] || "", fullName: qrCodeArray[3] || "" }; } return null; };
🧹 Nitpick comments (1)
src/pages/attendees/badge-checkin-page.js (1)
34-43: Consider validating badge format before check-in API call.The current flow calls
checkInBadge(data)first, then validates the badge format withformatBadgeQR. If the badge format is invalid, the user sees an error message even though the server-side check-in may have succeeded (if the server has different validation rules).Consider validating the badge format before making the API call to fail fast and provide clearer error messages:
🔎 Proposed refactor to validate before API call
const handleCheckIn = (data) => { if (data && !scanning) { + const badgeData = formatBadgeQR(data, currentSummit); + + if (!badgeData) { + Swal.fire( + T.translate("general.error"), + T.translate("badge_checkin.error_invalid_qr"), + "warning" + ); + return; + } + setScanning(true); checkInBadge(data) .then(() => { - const badgeData = formatBadgeQR(data, currentSummit); - - if (!badgeData) { - Swal.fire( - T.translate("general.error"), - T.translate("badge_checkin.error_invalid_qr"), - "warning" - ); - return; - } - Swal.fire( T.translate("badge_checkin.checked_in"), `${badgeData.fullName} (${badgeData.email}) checked in!`, "success" ); }) .finally(() => { setScanning(false); }); } };Note: Only apply this refactor if the server-side validation in
checkInBadgeand client-side validation informatBadgeQRare meant to be identical. If the server accepts a broader set of QR formats, keep the current flow but fix the validation logic informatBadgeQR.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/pages/attendees/badge-checkin-page.jssrc/utils/constants.jssrc/utils/methods.js
🧰 Additional context used
🧬 Code graph analysis (2)
src/utils/methods.js (3)
src/reducers/tickets/ticket-list-reducer.js (1)
summit(51-51)src/reducers/tickets/ticket-reducer.js (1)
summit(73-73)src/utils/constants.js (2)
BADGE_QR_MINIMUN_EXPECTED_FIELDS(228-228)BADGE_QR_MINIMUN_EXPECTED_FIELDS(228-228)
src/pages/attendees/badge-checkin-page.js (1)
src/utils/methods.js (2)
formatBadgeQR(509-525)formatBadgeQR(509-525)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (2)
src/pages/attendees/badge-checkin-page.js (2)
59-60: Good use of internationalization for error messages.Replacing hardcoded error text with translated strings improves maintainability and i18n support.
92-92: Good addition of explicit button type.Adding
type="button"prevents accidental form submission behavior, which is a best practice for buttons that don't submit forms.
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @src/utils/constants.js:
- Line 228: The constant BADGE_QR_MINIMUM_EXPECTED_FIELDS is set to 3 but
formatBadgeQR reads qrCodeArray[0..3]; update BADGE_QR_MINIMUM_EXPECTED_FIELDS
to 4 so the validation (using BADGE_QR_MINIMUM_EXPECTED_FIELDS) guarantees
indices 0 through 3 exist before formatBadgeQR accesses them, ensuring
consistent validation for the fourth field.
In @src/utils/methods.js:
- Around line 509-525: formatBadgeQR currently assumes inputs exist and accesses
qrCodeArray[3] while only checking for BADGE_QR_MINIMUM_EXPECTED_FIELDS (should
be 4); update the function to first validate that `code` and `summit` are
defined and that `summit.qr_registry_field_delimiter` and
`summit.badge_qr_prefix` are non-empty strings, then split using the delimiter
and check that qrCodeArray.length >= BADGE_QR_MINIMUM_EXPECTED_FIELDS (ensure
BADGE_QR_MINIMUM_EXPECTED_FIELDS is set to 4) before indexing; when building the
return object, trim string fields and validate that `ticketNumber` and `email`
are non-empty (return null or a clear error result if required) so no
out-of-bounds or empty critical fields propagate downstream.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/pages/attendees/badge-checkin-page.jssrc/utils/constants.jssrc/utils/methods.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/attendees/badge-checkin-page.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/utils/methods.js (1)
src/utils/constants.js (2)
BADGE_QR_MINIMUM_EXPECTED_FIELDS(228-228)BADGE_QR_MINIMUM_EXPECTED_FIELDS(228-228)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (1)
src/utils/methods.js (1)
33-34: LGTM!The import statement correctly includes the new constant.
| "OVERFLOW" | ||
| ]; | ||
|
|
||
| export const BADGE_QR_MINIMUM_EXPECTED_FIELDS = 3; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the constant definition
cat -n src/utils/constants.js | grep -A 2 -B 2 "BADGE_QR_MINIMUM_EXPECTED_FIELDS"Repository: fntechgit/summit-admin
Length of output: 141
🏁 Script executed:
# Search for all usages of BADGE_QR_MINIMUM_EXPECTED_FIELDS
rg "BADGE_QR_MINIMUM_EXPECTED_FIELDS" -n -B 3 -A 3Repository: fntechgit/summit-admin
Length of output: 969
🏁 Script executed:
# Find and examine the formatBadgeQR function
rg "formatBadgeQR" -n -A 20 src/utils/methods.jsRepository: fntechgit/summit-admin
Length of output: 560
Constant value doesn't align with array access pattern in formatBadgeQR.
The constant is set to 3, but formatBadgeQR in src/utils/methods.js accesses qrCodeArray[0], qrCodeArray[1], qrCodeArray[2], and qrCodeArray[3] (lines 517-520). The validation check at line 513 only guarantees qrCodeArray.length >= 3, which ensures indices 0-2 exist. Accessing qrCodeArray[3] without validating it first creates an inconsistency—the || "" fallback prevents crashes but doesn't properly validate the 4th field.
Change the constant to 4 to ensure all accessed array indices are validated before use.
-export const BADGE_QR_MINIMUM_EXPECTED_FIELDS = 3;
+export const BADGE_QR_MINIMUM_EXPECTED_FIELDS = 4;🤖 Prompt for AI Agents
In @src/utils/constants.js around line 228, The constant
BADGE_QR_MINIMUM_EXPECTED_FIELDS is set to 3 but formatBadgeQR reads
qrCodeArray[0..3]; update BADGE_QR_MINIMUM_EXPECTED_FIELDS to 4 so the
validation (using BADGE_QR_MINIMUM_EXPECTED_FIELDS) guarantees indices 0 through
3 exist before formatBadgeQR accesses them, ensuring consistent validation for
the fourth field.
| export const formatBadgeQR = (code, summit) => { | ||
| const qrCodeArray = code.split(summit.qr_registry_field_delimiter); | ||
|
|
||
| if ( | ||
| qrCodeArray.length >= BADGE_QR_MINIMUM_EXPECTED_FIELDS && | ||
| qrCodeArray[0] === summit.badge_qr_prefix | ||
| ) { | ||
| return { | ||
| badgePrefix: qrCodeArray[0], | ||
| ticketNumber: qrCodeArray[1], | ||
| email: qrCodeArray[2] || "", | ||
| fullName: qrCodeArray[3] || "" | ||
| }; | ||
| } | ||
|
|
||
| return null; | ||
| }; |
There was a problem hiding this comment.
Add input validation and ensure array bounds are properly checked.
Several concerns with this function:
-
Array access mismatch: The function checks
qrCodeArray.length >= BADGE_QR_MINIMUM_EXPECTED_FIELDS(which is 3) but accessesqrCodeArray[3]. This doesn't align with the maintainer's previous feedback requesting a check for 4 elements. -
Missing null/undefined checks: The function doesn't validate that
code,summit.qr_registry_field_delimiter, orsummit.badge_qr_prefixare defined before use, which could cause runtime errors. -
Empty field validation: Consider validating that critical fields like
ticketNumberandemailare not empty strings after parsing, as empty values may cause issues downstream when displaying badge data in the SweetAlert popup.
Based on past review comments from smarcet.
🔎 Proposed improvements
export const formatBadgeQR = (code, summit) => {
+ if (!code || !summit?.qr_registry_field_delimiter || !summit?.badge_qr_prefix) {
+ return null;
+ }
+
const qrCodeArray = code.split(summit.qr_registry_field_delimiter);
if (
qrCodeArray.length >= BADGE_QR_MINIMUM_EXPECTED_FIELDS &&
qrCodeArray[0] === summit.badge_qr_prefix
) {
+ const ticketNumber = qrCodeArray[1]?.trim();
+ const email = qrCodeArray[2]?.trim() || "";
+ const fullName = qrCodeArray[3]?.trim() || "";
+
+ // Validate that required fields are not empty
+ if (!ticketNumber) {
+ return null;
+ }
+
return {
badgePrefix: qrCodeArray[0],
- ticketNumber: qrCodeArray[1],
- email: qrCodeArray[2] || "",
- fullName: qrCodeArray[3] || ""
+ ticketNumber,
+ email,
+ fullName
};
}
return null;
};Note: Also ensure BADGE_QR_MINIMUM_EXPECTED_FIELDS is set to 4 as discussed in the constants.js review.
📝 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.
| export const formatBadgeQR = (code, summit) => { | |
| const qrCodeArray = code.split(summit.qr_registry_field_delimiter); | |
| if ( | |
| qrCodeArray.length >= BADGE_QR_MINIMUM_EXPECTED_FIELDS && | |
| qrCodeArray[0] === summit.badge_qr_prefix | |
| ) { | |
| return { | |
| badgePrefix: qrCodeArray[0], | |
| ticketNumber: qrCodeArray[1], | |
| email: qrCodeArray[2] || "", | |
| fullName: qrCodeArray[3] || "" | |
| }; | |
| } | |
| return null; | |
| }; | |
| export const formatBadgeQR = (code, summit) => { | |
| if (!code || !summit?.qr_registry_field_delimiter || !summit?.badge_qr_prefix) { | |
| return null; | |
| } | |
| const qrCodeArray = code.split(summit.qr_registry_field_delimiter); | |
| if ( | |
| qrCodeArray.length >= BADGE_QR_MINIMUM_EXPECTED_FIELDS && | |
| qrCodeArray[0] === summit.badge_qr_prefix | |
| ) { | |
| const ticketNumber = qrCodeArray[1]?.trim(); | |
| const email = qrCodeArray[2]?.trim() || ""; | |
| const fullName = qrCodeArray[3]?.trim() || ""; | |
| // Validate that required fields are not empty | |
| if (!ticketNumber) { | |
| return null; | |
| } | |
| return { | |
| badgePrefix: qrCodeArray[0], | |
| ticketNumber, | |
| email, | |
| fullName | |
| }; | |
| } | |
| return null; | |
| }; |
🤖 Prompt for AI Agents
In @src/utils/methods.js around lines 509-525, formatBadgeQR currently assumes
inputs exist and accesses qrCodeArray[3] while only checking for
BADGE_QR_MINIMUM_EXPECTED_FIELDS (should be 4); update the function to first
validate that `code` and `summit` are defined and that
`summit.qr_registry_field_delimiter` and `summit.badge_qr_prefix` are non-empty
strings, then split using the delimiter and check that qrCodeArray.length >=
BADGE_QR_MINIMUM_EXPECTED_FIELDS (ensure BADGE_QR_MINIMUM_EXPECTED_FIELDS is set
to 4) before indexing; when building the return object, trim string fields and
validate that `ticketNumber` and `email` are non-empty (return null or a clear
error result if required) so no out-of-bounds or empty critical fields propagate
downstream.
ref: https://app.clickup.com/t/86b736jyw
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.