Skip to content

Conversation

@tareq89
Copy link
Contributor

@tareq89 tareq89 commented Sep 30, 2025

issue: #10584
country config: opencrvs/opencrvs-countryconfig#1052

Summary

This PR updates the toCertificateVariables function in Address.tsx to handle foreign-country addresses more gracefully.

Key Changes

  • Refactored stringifier usage

    • Moved getFormDataStringifier and stringifiedResult creation to the top of the function to avoid duplication.
  • Added early return for foreign-country addresses

    • If value.country !== window.config.COUNTRY, the function now returns a merged object of:
      • The stringified form data, and
      • streetLevelDetails
    • This ensures foreign addresses bypass the local administrative hierarchy logic.
  • Preserved local-country logic

    • For addresses within the configured country, the function continues to build adminLevelHierarchy as before.

Why

  • Prevents incorrect or incomplete stringification for addresses outside the configured country.
  • Makes the code cleaner by reducing duplicate calls to the stringifier.
  • Improves robustness for multi-country support.

Impact

  • Foreign-country addresses → Now display both stringified fields and street-level details correctly.
  • Local-country addresses → Behavior remains unchanged.

Checklist

  • I have linked the correct Github issue under "Development"
  • I have tested the changes locally, and written appropriate tests
  • I have tested beyond the happy path (e.g. edge cases, failure paths)
  • I have updated the changelog with this change (if applicable)
  • I have updated the GitHub issue status accordingly

@github-actions
Copy link

Oops! Looks like you forgot to update the changelog. When updating CHANGELOG.md, please consider the following:

  • Changelog is read by country implementors who might not always be familiar with all technical details of OpenCRVS. Keep language high-level, user friendly and avoid technical references to internals.
  • Answer "What's new?", "Why was the change made?" and "Why should I care?" for each change.
  • If it's a breaking change, include a migration guide answering "What do I need to do to upgrade?".

@ocrvs-bot
Copy link
Contributor

Your environment is deployed to https://ocrvs-10584.opencrvs.dev

Copy link
Contributor

@Zangetsu101 Zangetsu101 left a comment

Choose a reason for hiding this comment

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

Some test cases would be appreciated 🙏

@Nil20
Copy link
Contributor

Nil20 commented Sep 30, 2025

You'd probably need to update client/src/v2-events/features/events/registered-fields/Address.stories.tsx as the change in the UI tests suggests

Copy link
Contributor

@makelicious makelicious left a comment

Choose a reason for hiding this comment

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

Could you add a test case for this so regression won't happen

@tareq89 tareq89 force-pushed the ocrvs-10584 branch 2 times, most recently from ee3bd07 to 48614fd Compare October 6, 2025 09:42
@tareq89
Copy link
Contributor Author

tareq89 commented Oct 6, 2025

Test added

@makelicious
Copy link
Contributor

makelicious commented Oct 6, 2025

https://github.com/opencrvs/opencrvs-core/pull/10228/files

It seems that it broke here. Initially $lookup converted to the stringified form. I sugggest we change it back and write a test case so it won't happen again. Then we would not need to have

$lookup, lookup, $declaration, $originalDeclaration

Copy link
Contributor

@makelicious makelicious left a comment

Choose a reason for hiding this comment

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

Could you add a test case for this so regression won't happen

@tareq89 tareq89 merged commit 6a0cf65 into develop Oct 7, 2025
82 of 84 checks passed
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.

7 participants