Skip to content

Comments

Qr codes#14

Closed
eswarchandravidyasagar wants to merge 15 commits intomainfrom
qr-codes
Closed

Qr codes#14
eswarchandravidyasagar wants to merge 15 commits intomainfrom
qr-codes

Conversation

@eswarchandravidyasagar
Copy link
Collaborator

This pull request introduces QR code support for client immunization notices, updates both English and French templates to include a QR code column, and refactors data handling to support these changes. The most significant updates are grouped below by theme.

QR Code Generation and Data Integration

  • Added a generate_qr_code function to scripts/utils.py that creates a QR code PNG for each client and returns its file path. The function ensures output directories exist and uses client-specific filenames. [1] [2]
  • Modified the notice-building process in scripts/preprocess.py to generate and attach a QR code for each client, including relevant client data in the QR code. [1] [2] [3]

Template and Table Updates (English & French)

  • Updated both client_info_tbl_en and client_info_tbl_fr table functions in output/conf.typ to add a third column for the QR code, adjust column widths, and display the QR image if available. [1] [2]
  • Refactored the English and French template scripts to use the updated table functions and ensure the QR code appears in notices. This includes changes to how data is passed and displayed in the templates. [1] [2] [3] [4] [5] [6]

General Refactoring and Bug Fixes

  • Standardized variable names and improved the passing of client data and IDs throughout the scripts to support the new QR code logic and avoid mismatches. [1] [2]

These changes collectively enable QR code functionality in the generated immunization notices and ensure consistent formatting and data handling across both English and French versions.

- Added QR code generation utility function in utils.py
- Modified preprocess.py to generate QR codes for each client
- Updated French and English template scripts to include QR codes
- Modified conf.typ to display QR codes in PDF notices
- QR codes contain client ID, name, DOB, and school information
- QR codes are saved as PNG files and embedded in PDFs
- Added QR code column to client_info_tbl_fr and client_info_tbl_en functions
- QR codes are displayed in the top-right corner of immunization notices
- Uses image.decode to render base64 encoded QR code images
- Maintains responsive layout with proper column widths
Copilot AI review requested due to automatic review settings October 6, 2025 21:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request introduces QR code functionality to client immunization notices, allowing QR codes containing client information to be embedded in both English and French notice templates.

  • Added QR code generation capability with client-specific data encoding
  • Updated English and French templates to include a new QR code column in client information tables
  • Refactored template functions to support the three-column layout with QR code display

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
scripts/utils.py Added QR code generation function with PNG file output
scripts/preprocess.py Integrated QR code generation into notice building process
scripts/2025_mock_generate_template_french.sh Updated French template to include QR code column and fix parameter handling
scripts/2025_mock_generate_template_english.sh Updated English template to include QR code column and fix syntax errors
output/conf.typ Modified table functions to support three-column layout with QR code display

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

scripts/utils.py Outdated
else:
filename = "qr_code.png"

filepath = f"../qr_codes/{filename}"
Copy link

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filepath variable points to '../qr_codes/' but the actual file is saved to '../output/qr_codes/'. This mismatch will cause issues when the QR code image is referenced.

Suggested change
filepath = f"../qr_codes/{filename}"
filepath = f"../output/qr_codes/{filename}"

Copilot uses AI. Check for mistakes.
scripts/utils.py Outdated
filename = "qr_code.png"

filepath = f"../qr_codes/{filename}"
import os
Copy link

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import statements should be placed at the top of the file with other imports, not inside functions.

Copilot uses AI. Check for mistakes.
@@ -1,9 +1,51 @@
#!/bin/bash
#!/bi#let immunization_notice(data, value, immunizations_due, date, font_size) = block[
Copy link

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shebang line is corrupted. It should be '#!/bin/bash' but contains embedded typst code.

Suggested change
#!/bi#let immunization_notice(data, value, immunizations_due, date, font_size) = block[
#!/bin/bash

Copilot uses AI. Check for mistakes.
]
]],
)
]NDIR=${1}
Copy link

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable assignment is malformed. It should be 'INDIR=${1}' but has extra characters at the beginning.

Suggested change
]NDIR=${1}
INDIR=${1}

Copilot uses AI. Check for mistakes.
@eswarchandravidyasagar
Copy link
Collaborator Author

closes #3

@eswarchandravidyasagar eswarchandravidyasagar marked this pull request as draft October 7, 2025 13:24
@kassyray
Copy link
Contributor

kassyray commented Oct 7, 2025

@eswarchandravidyasagar I think we may want the QR code in a different location for privacy purposes. The client information table provides information for the envelope (the way it is folded).

@eswarchandravidyasagar
Copy link
Collaborator Author

@eswarchandravidyasagar I think we may want the QR code in a different location for privacy purposes. The client information table provides information for the envelope (the way it is folded).

Feel free to commit that change based on what you think makes the most sense.

@jangevaare
Copy link
Member

@eswarchandravidyasagar are there pros/cons of doing this via during data preparation rather than using a typst module?

@jangevaare
Copy link
Member

@kassyray do you know if there is a performance hit to using vector graphics formats over compressed graphics formats in typst?

"name": f"{row.FIRST_NAME} {row.LAST_NAME}",
"dob": row.DATE_OF_BIRTH,
"school": row.SCHOOL_NAME
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's likely that this data would vary by application - and is likely a URL with parameters

Wonder if we could potentially surface within an configuration in an f-string-like configuration, documenting and expanding available parameters in the readme. And validating the configuration as part of the generation pipeline?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be better within a separate function within the class, and then it can be an optional variable in this function and within the conf.typ function as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jangevaare @eswarchandravidyasagar requested changes based on this as well as corruption to some variable names - please see comments under each change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved Qr code generation to its own function and also improved configuration for QR codes and contact information

@eswarchandravidyasagar
Copy link
Collaborator Author

@eswarchandravidyasagar are there pros/cons of doing this via during data preparation rather than using a typst module?

I couldn't get the typst module to work for QR codes. Ran into some compatibility issues

…ality; update template columns for better layout in English and French versions.
@eswarchandravidyasagar eswarchandravidyasagar marked this pull request as ready for review October 10, 2025 17:55
@eswarchandravidyasagar
Copy link
Collaborator Author

closes #3

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants