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

fix(sgid): add guard clause to ensure field is present before formatting #7997

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

KenLSM
Copy link
Contributor

@KenLSM KenLSM commented Dec 16, 2024

Problem

When an initial scope is requested to sgID, the scope could have been outdated if the admin updates the form while the respondent is approving the sgID request.

This results in sgID returning fields requested in the initial scope. When BE receives and assess scope, there are fields that would be formatted (i.e., requiring and assuming that the fields are rightly returned). These formatters would then fail (and crash the system) since sgID response did not contain the latest form data.

Solution

Add undefined check before calling formatX which attempts to manipulates the fields.

Note: The form definition would be checked on submission, which invalidates the submission thereafter.

Breaking Changes

  • No - this PR is backwards compatible

Tests

Updated form when requested with old sgid scope should not crash BE

  • Create a sgid form
  • Open form
  • As a respondent login (but hold at the qrcode scanning)
  • As an admin add address field to form
  • As a respondent continue with login
  • Ensure that form is loaded

@KenLSM KenLSM requested a review from scottheng96 December 16, 2024 04:24
@KenLSM KenLSM merged commit 223cf93 into develop Dec 16, 2024
26 of 27 checks passed
@KenLSM KenLSM deleted the fix/handle-sgid-empty-adapter-data branch December 16, 2024 04:25
@KenLSM KenLSM mentioned this pull request Dec 16, 2024
6 tasks
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.

2 participants