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: Add constants.humanName to allow countries to have custom full name format #7957

Merged
merged 14 commits into from
Jan 27, 2025

Conversation

Siyasanga
Copy link
Collaborator

@Siyasanga Siyasanga commented Nov 12, 2024

In order to allow countries to define their own full name format, we are adding a constants.humanName into the client.csv translations, which will then be used everywhere where we are displaying system users and citizen's name in the systems.

Addresses #6830

Countryconfig: opencrvs/opencrvs-countryconfig#314
Farajaland: opencrvs/opencrvs-farajaland#1164

@Siyasanga Siyasanga force-pushed the ocrvs-6830-human-name-constant branch from 1a13fc2 to b66f929 Compare November 14, 2024 08:32
@Siyasanga Siyasanga added the 🚀 Ready to deploy Deployment automation should pick this PR up and start auto-deploying it label Nov 14, 2024
@github-actions github-actions bot temporarily deployed to ocrvs-6830-human-name-constant November 14, 2024 08:36 Inactive
@Siyasanga Siyasanga marked this pull request as ready for review November 14, 2024 08:42
@ocrvs-bot
Copy link
Collaborator

Your environment is deployed to https://ocrvs-6830-human-name-constant.opencrvs.dev

@Siyasanga Siyasanga temporarily deployed to ocrvs-6830-human-name-constant November 14, 2024 08:46 — with GitHub Actions Inactive
@Siyasanga Siyasanga self-assigned this Nov 14, 2024
@Siyasanga Siyasanga added this to the v1.7.0 milestone Nov 14, 2024
@Siyasanga Siyasanga force-pushed the ocrvs-6830-human-name-constant branch from b66f929 to 948771b Compare November 14, 2024 09:55
@Siyasanga Siyasanga temporarily deployed to ocrvs-6830-human-name-constant November 14, 2024 09:55 — with GitHub Actions Inactive
@Siyasanga Siyasanga temporarily deployed to ocrvs-6830-human-name-constant November 14, 2024 11:32 — with GitHub Actions Inactive
@Siyasanga Siyasanga temporarily deployed to ocrvs-6830-human-name-constant November 14, 2024 23:34 — with GitHub Actions Inactive
Copy link
Collaborator

@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.

@Siyasanga After taking a look at the changes made to the e2e tests, I would say it's better to change all those places in core to use the new format instead. As if only some of the places use the new format, then it would just create confusion

@Zangetsu101 Zangetsu101 added 🔧Waiting for fixes and removed 🚀 Ready to deploy Deployment automation should pick this PR up and start auto-deploying it labels Nov 22, 2024
@Siyasanga Siyasanga force-pushed the ocrvs-6830-human-name-constant branch from a2148db to c6f8c7a Compare December 18, 2024 22:30
@Siyasanga Siyasanga force-pushed the ocrvs-6830-human-name-constant branch 4 times, most recently from 40966ab to fc69672 Compare January 15, 2025 07:38
@Siyasanga Siyasanga added 🚀 Ready to deploy Deployment automation should pick this PR up and start auto-deploying it and removed 🔧Waiting for fixes labels Jan 15, 2025
@github-actions github-actions bot temporarily deployed to ocrvs-6830-human-name-constant January 15, 2025 07:48 Inactive
@Siyasanga
Copy link
Collaborator Author

Hi @Zangetsu101, hope you're good. Please review this PR when you have a chance.

I have reverted the changes that were trying to generate the full name for the form field in the review screens as we had discussed. I also removed the assertions that were trying to prove that the name is customisable via the config. We'll only depend on the unit tests for now untill we get a country that uses full name that is different to farajahland.

I've also removed the changes on the integration tests for the Farajahland repo.

@Siyasanga Siyasanga temporarily deployed to ocrvs-6830-human-name-constant January 15, 2025 07:54 — with GitHub Actions Inactive
@Siyasanga Siyasanga temporarily deployed to ocrvs-6830-human-name-constant January 16, 2025 06:51 — with GitHub Actions Inactive
@Siyasanga Siyasanga temporarily deployed to ocrvs-6830-human-name-constant January 17, 2025 06:16 — with GitHub Actions Inactive
Comment on lines 30 to 36
const intlBangla = createIntl(
{
locale: 'en',
messages: {}
},
cache
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this supposed to be like so?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice catch, thanks Tameem. Fixed

Replace older logic to get the name which was based on an assumption that we support names in multiple languages

#6830
We've found cleaner way to make the rendered name customizable for each country through client copy from country-config

#6830
We need to update all the places where a citizen's name is being referenced to show it in the format that the country chooses

#6830
The expected name should match the format for the given country.

#6830
@Siyasanga Siyasanga force-pushed the ocrvs-6830-human-name-constant branch from 5e12e50 to 8cf8cc0 Compare January 24, 2025 08:53
@Siyasanga Siyasanga temporarily deployed to ocrvs-6830-human-name-constant January 24, 2025 08:54 — with GitHub Actions Inactive
@Siyasanga Siyasanga temporarily deployed to ocrvs-6830-human-name-constant January 24, 2025 13:55 — with GitHub Actions Inactive
@euanmillar
Copy link
Collaborator

@Zangetsu101 @Siyasanga and I dont know why knip is failing here but tests are all passing and think this is ready to merge?

@Zangetsu101
Copy link
Collaborator

@Siyasanga What's the status of e2e tests? As long as they are passing, we can merge this in

@Siyasanga Siyasanga temporarily deployed to ocrvs-6830-human-name-constant January 27, 2025 06:25 — with GitHub Actions Inactive
@Siyasanga Siyasanga deployed to ocrvs-6830-human-name-constant January 27, 2025 10:00 — with GitHub Actions Active
@Siyasanga
Copy link
Collaborator Author

@Siyasanga What's the status of e2e tests? As long as they are passing, we can merge this in

They are passing so I will merge this and the related contry-config PR

@Siyasanga Siyasanga enabled auto-merge January 27, 2025 10:01
@Siyasanga Siyasanga disabled auto-merge January 27, 2025 10:02
@Siyasanga Siyasanga enabled auto-merge (squash) January 27, 2025 10:02
@@ -29,6 +29,8 @@
- A new GraphQL mutation `upsertRegistrationIdentifier` is added to allow updating the patient identifiers of a registration record such as NID [#8034](https://github.com/opencrvs/opencrvs-core/pull/8034)
- A new GraphQL mutation `updateField` is added to allow updating any field in a record [#8291](https://github.com/opencrvs/opencrvs-core/pull/8291)
- Updated GraphQL mutation `confirmRegistration` to allow adding a `comment` for record audit [#8197](https://github.com/opencrvs/opencrvs-core/pull/8197)
- Introduced a new customisable UI component: Banner [#8276](https://github.com/opencrvs/opencrvs-core/issues/8276)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- Introduced a new customisable UI component: Banner [#8276](https://github.com/opencrvs/opencrvs-core/issues/8276)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems to be something irrelevant

@Siyasanga Siyasanga merged commit 677c7b3 into develop Jan 27, 2025
73 checks passed
@Zangetsu101 Zangetsu101 deleted the ocrvs-6830-human-name-constant branch January 27, 2025 13:09
github-actions bot pushed a commit that referenced this pull request Jan 27, 2025
github-actions bot pushed a commit that referenced this pull request Jan 27, 2025
jamil314 pushed a commit that referenced this pull request Feb 3, 2025
…name format (#7957)

* Add humanName to constants.ts

To allow countries to have custom ordering for full names

#6830

* Use constants.humanName on the UserList.tsx comp

To ensure that we get the format from the country-config

#6830

* Create a utility function getLocalisedname()

To make the name more usable we had to extract the name formatting logic into it's own function.

#6830

* Use getLocalisedName on the UserList component

#6830

* Use getLocalisedName on the UserAudit comp

Replace older logic to get the name which was based on an assumption that we support names in multiple languages

#6830

* refactor: use getLocalisedName() @ InProgress.tsx

We've found cleaner way to make the rendered name customizable for each country through client copy from country-config

#6830

* Refactor the work queues to use getLocalisedName()

We need to update all the places where a citizen's name is being referenced to show it in the format that the country chooses

#6830

* Record changes in the CHANGELOG for this PR

#6830

* Use humanName for other search/transformer.ts

#6830

* Fix type error 4 the findSavedReference()

* Fix faling unit test after using humanName

The expected name should match the format for the given country.

#6830

* Add the correct local for intlBangla

#6830

* Pass intl object for myDrafts transformDraftContent

#6830
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 Ready to deploy Deployment automation should pick this PR up and start auto-deploying it 💅 Waiting For Review
Projects
Status: In Code Review
4 participants