Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(condo): DOMA-9944 parse QR code in base64 format in ValidateQRCodeService #5217

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

VKislov
Copy link
Contributor

@VKislov VKislov commented Sep 11, 2024

No description provided.

@VKislov VKislov force-pushed the DOMA-9944/condo/feat/parse-qr-code-from-base64-string branch from 744fb56 to 2968d76 Compare September 17, 2024 06:03
@pahaz
Copy link
Member

pahaz commented Sep 23, 2024

It looks like you should add at least one REAL WORLD TEST CASE where we have a problem with encoding.


if (!matches) {
logger.error({ msg:'Error qr-code parsing', data: { qrStr, decodedRequisitesStr: decodedQrStr, detectedEncoding } })
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
logger.error({ msg:'Error qr-code parsing', data: { qrStr, decodedRequisitesStr: decodedQrStr, detectedEncoding } })
logger.error({ msg:'Error qr-code parsing', data: { qrStr, decodedQrStr, detectedEncoding } })

throw new Error('Invalid QR code')
}
logger.info({ msg:'Parsed qr-code', data: { qrStr, decodedRequisitesStr: decodedQrStr, detectedEncoding } })
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
logger.info({ msg:'Parsed qr-code', data: { qrStr, decodedRequisitesStr: decodedQrStr, detectedEncoding } })
logger.info({ msg:'Parsed qr-code', data: { qrStr, decodedQrStr, detectedEncoding } })

@VKislov VKislov force-pushed the DOMA-9944/condo/feat/parse-qr-code-from-base64-string branch from c8bd005 to af09626 Compare October 9, 2024 14:46
Copy link

sonarcloud bot commented Oct 9, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants