Skip to content

Conversation

@tahmidrahman-dsi
Copy link
Contributor

Note

Currently, we do not run e2e tests as a check on opencrvs-countryconfig-repo PRs. Please ensure your PR doesn't break any e2e tests.

One method for doing this is to open a PR with these changes to opencrvs-farajaland as well, and see if the PR check passes there.

Description

Clearly describe what has been changed. Include relevant context or background.
Explain how the issue was fixed (if applicable) and the root cause.

Link this pull request to the GitHub issue (and optionally name the branch ocrvs-<issue #>)

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

naftis and others added 30 commits February 7, 2025 13:50
* feat: add configuration to implement logic before forwarding to mosip for id creation

* chore: bump mosip package version

* fix: amends to verify approach

* fix: minimize changes

* fix: add logs

* fix: minor improvements
Added vadym mudryi to development and e2e servers
fix: Added vadym mudryi to opencrvs servers, disabled niko
feat/disable fields when verified or authenticated
fix: wait for assignment to propagate
tahmidrahman-dsi and others added 28 commits October 17, 2025 09:14
* add development time yarn db:clear:all script for easy analytics data clearing

* fix indent

* add translation

* fix street level address validation

* add ids to data config fields

* drop analytics database as part of data clearing

* cleanup

* fix: death informant phone number error message

* add 'ConditionalType.DISPLAY_ON_REVIEW'

* use new conditional

* use ConditionalType.DISPLAY_ON_REVIEW

* missing translation added

* fix

* fix: remove print in advance from death form of print cert (opencrvs#1071)

* fix: remove print in advance from death form of print cert

* fix: remove print in advancefor other collector type

* update metabase dump

* fix: dont show birth certificate after first print

* [OCRVS-10730] Remove 'Completeness Rates Dashboard'/ 'Vital Statistics Export' links in registration dashboard

* [OCRVS-10729] Metabase copy cleanup

* [OCRVS-10722] Remove Jurisdiction filter from Registrations dashboards

* chore: update toolkit version

* feat: use the new age field in birth form

* chore: upgrade toolkit version

* feat: use new AGE field

* [OCRVS-10729] Remove location filter value completely from registrations dashboards title

* Ensure docker daemon does not depend on cryptfs logic on workers. Data directories and cryptfs should only exist on manager node

* fix: Place of death in advanced search

* fix(analytics): clear existing event_actions before upsert and mark last action as accepted on immediate confirmation (opencrvs#1094)

* feat: add validations to age field

* chore: upgrade toolkit version

* Metabase: add country wide completeness rate

* Metabase: update dashboards

* Metabase: update readme

* update toolkit

* update toolkit

---------

Co-authored-by: Riku Rouvila <[email protected]>
Co-authored-by: cibelius <[email protected]>
Co-authored-by: tareq89 <[email protected]>
Co-authored-by: jamil314 <[email protected]>
Co-authored-by: Md. Ashikul Alam <[email protected]>
Co-authored-by: Tameem Bin Haider <[email protected]>
Co-authored-by: Tameem Bin Haider <[email protected]>
Co-authored-by: naftis <[email protected]>
fix: hidden fields should not receive a value
* fix: backend doesnt get the full http payload

* dont display http or query params on review
…mosip into fix/query-param-reader-removing-review-params
…mosip into fix/query-param-reader-removing-review-params
…review-params

[OCRVS-10759] Fix query param reader removing review params
…d-type

bumps mosip-api to 1.9.0-beta.4 and fetches nid & id type
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. CHANGELOG.md, line 64-65 (link)

    logic: These improvement entries are duplicated from the 1.9.0 section above (lines 13-14) and from 1.8.0(lines 54-55).

46 files reviewed, 47 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +44 to +50
- Upgraded to `@opencrvs/mosip` v1.8.0 to support the following enhancements:
- Added QR code scanner form configuration, allowing users to scan QR codes and automatically prefill form fields with the extracted data. [#7939](https://github.com/opencrvs/opencrvs-core/issues/7939)
- Integrated E-signet authentication flow using mock identities. [#8062](https://github.com/opencrvs/opencrvs-core/issues/8062)
- Enabled online verification flow with mock identities. [#7944](https://github.com/opencrvs/opencrvs-core/issues/7944)
- Provided support for custom business logic to determine whether MOSIP processing should be triggered during registration. [#7942](https://github.com/opencrvs/opencrvs-core/issues/7942)
- Ensured that registering a death event deactivates the corresponding identity in MOSIP. [#7943](https://github.com/opencrvs/opencrvs-core/issues/7943)
- Enforced rejection of registrations if MOSIP processing fails. [#8174](https://github.com/opencrvs/opencrvs-core/issues/8174)
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Duplicate MOSIP v1.8.0 upgrade entry - these same features are listed in both 1.8.0 and 1.9.0 sections (lines 7-9 and 44-50). Remove from 1.8.0 section.

Comment on lines +54 to +55
- Added Build summary and refactored deployment workflow to be more clear [#6984](https://github.com/opencrvs/opencrvs-core/issues/6984)
- Build OpenCRVS release images for arm devices [#9455](https://github.com/opencrvs/opencrvs-core/issues/9455)
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Duplicate improvement entries - these are repeated from 1.9.0 section (lines 11-14) and again in 1.6.4 (lines 64-65). Remove duplicates.

Comment on lines +57 to 60
## 1.6.4

- Added a local virtual machine setup for testing Ansible playbooks locally (on MacOS and Ubuntu ). Check [provision.ipynb](infrastructure/local-development/provision.ipynb) for more details.

Copy link
Contributor

Choose a reason for hiding this comment

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

logic: 1.6.4 section appears twice - once here (lines 57-60) and once later (lines 272-276). Consolidate into single entry.

Comment on lines +135 to +136
conditional: and(requireMotherDetails)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Wrapping requireMotherDetails in and() with no other operands is redundant.

- SMTP_USERNAME=${SMTP_USERNAME}
- SMTP_PASSWORD=${SMTP_PASSWORD}
- SMTP_SECURE=${SMTP_SECURE}
- ESIGNET_REDIRECT_URL=${ESIGNET_REDIRECT_URL}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Missing default value for required environment variable. Unlike the other OpenID variables (lines 32-34), ESIGNET_REDIRECT_URL has no :- fallback, which will cause deployment failures if the variable is unset.

Comment on lines +299 to +300
const motherID = mother.identifier?.[0].value
const fatherID = father.identifier?.[0].value
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Unsafe non-null assertions. mother.identifier and father.identifier could be undefined or empty arrays, causing runtime errors.

getComposition(record)
)
if (!compositionSection) return undefined
const personSectionEntry = compositionSection.entry![0]
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Unsafe non-null assertion. compositionSection.entry could be an empty array, making entry[0] undefined.

Comment on lines +346 to +348
const reference = (relatedPersonEntry?.resource as fhir3.RelatedPerson)
.patient.reference
return getFromBundleById(record, reference!.split('/')[1])
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Multiple unsafe non-null assertions. relatedPersonEntry?.resource, patient.reference, and the array access can all fail at runtime.

export function getInformantFullName(bundle: fhir3.Bundle) {
const informant = getInformantPatient(bundle)
return (
informant?.name?.[0]?.given?.join(' ') + ' ' + informant?.name?.[0]?.family
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Concatenation without null checks will produce 'undefined undefined' if informant or name properties are missing.

getComposition(record)
)
if (!compositionSection) return undefined
const personSectionEntry = compositionSection.entry![0]
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Unsafe non-null assertion. compositionSection.entry could be an empty array.

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.