Clean up Field types #8578

makelicious · 2025-02-05T11:15:20Z

Changes:

Create guards for each FieldType to use instead of casting
Remove "nominal"-style typing for field values. It might make a comeback*
Move FieldType related mappings to single file.
Ensure Pages are rendered with default form values (should fix the A component is changing an uncontrolled input of type text to be controlled)
- Might not be a good idea. The point was that when new field is introduced, it is easy to keep in sync.
Rename optionValues to options (<select><option value=""></select> might trip SI otherwise)
Rename options to configuration

* Since country-config defines the configuration, we are left with union type when developing client or events backend. Only way to regain a bit of safety is to use guards.

github-actions · 2025-02-05T11:15:35Z

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?".

makelicious · 2025-02-05T11:16:39Z

packages/client/src/v2-events/features/events/registered-fields/Location.tsx

Input used string under the hood earlier.

makelicious · 2025-02-05T11:16:45Z

packages/client/src/v2-events/features/events/registered-fields/LocationSearch.tsx

Input used string under the hood earlier.

makelicious · 2025-02-05T11:16:57Z

packages/client/src/v2-events/features/events/registered-fields/Select.tsx

Input used string under the hood earlier.

makelicious · 2025-02-05T11:17:03Z

packages/client/src/v2-events/features/events/registered-fields/SelectCountry.tsx

Input used string under the hood earlier.

makelicious · 2025-02-05T11:18:43Z

packages/commons/src/events/.eslintrc.js

I suppose the license header in both makes this assumption.

Added linting since it is nicer to program with it.

makelicious · 2025-02-05T11:18:55Z

packages/commons/src/events/FieldConfig.ts

@@ -88,43 +77,10 @@ const BaseField = z.object({

 export type BaseField = z.infer<typeof BaseField>

-export const FieldType = {


Moved under FieldType

rikukissa · 2025-02-05T12:02:54Z

packages/client/src/v2-events/components/forms/utils.ts

    case FieldType.CHECKBOX:
-      return checkboxToString(value as CheckboxFieldValue)
+      // @TODO: This should be a boolean?


I think this function used to be the display formatting function, so if checkbox was checked, then in the review view for instance, you saw the value as Yes or No. Should have been message descriptors tho

Ah, sorry the function comment threw me off a bit. But yea, checkbox value should be real boolean and not strings

rikukissa · 2025-02-05T12:07:13Z

packages/commons/src/events/FieldType.ts

+export function mapFieldTypeToElasticsearch(field: FieldConfig) {
+  switch (field.type) {
+    case FieldType.DATE:
+      // @TODO: This should be changed back to 'date'
+      // When we have proper validation of custom fields.
+      return { type: 'text' }
+    case FieldType.TEXT:
+    case FieldType.PARAGRAPH:
+    case FieldType.BULLET_LIST:
+    case FieldType.PAGE_HEADER:
+      return { type: 'text' }
+    case FieldType.DIVIDER:
+    case FieldType.RADIO_GROUP:
+    case FieldType.SELECT:
+    case FieldType.COUNTRY:
+    case FieldType.CHECKBOX:
+    case FieldType.LOCATION:
+      return { type: 'keyword' }
+    case FieldType.FILE:
+      return {
+        type: 'object',
+        properties: {
+          filename: { type: 'keyword' },
+          originalFilename: { type: 'keyword' },
+          type: { type: 'keyword' }
+        }
+      }
+  }
+}


Let's keep this somewhere closer to the indexing logic. Commons doesn't need to know about Elasticsearch being used

rikukissa · 2025-02-05T12:17:21Z

packages/client/src/v2-events/components/forms/utils.ts

 }

-export function getInitialValues(fields: FieldConfig[]) {
-  return fields.reduce((initialValues: Record<string, FieldValue>, field) => {
+export function setDefaultsForMissingFields(


This seems to be unused

rikukissa · 2025-02-05T13:41:17Z

packages/client/src/v2-events/components/forms/utils.ts

    case FieldType.CHECKBOX:
-      return checkboxToString(value as CheckboxFieldValue)
+      // @TODO: This should be a boolean?
+      return value === 'true' ? 'Yes' : 'No'


TRPCError: [ { "received": "No", "code": "invalid_enum_value", "options": [ "true", "false" ], "path": [ "recommender.none" ], "message": "Invalid enum value. Expected 'true' | 'false', received 'No'" } ] at Object.<anonymous> (/Users/riku/Code/OpenCRVS/opencrvs-core/packages/events/src/router/middleware/validate/index.ts:54:13) at Generator.next (<anonymous>) at fulfilled (/Users/riku/Code/OpenCRVS/opencrvs-core/packages/events/src/router/middleware/validate/index.ts:15:58) at processTicksAndRejections (node:internal/process/task_queues:95:5)

Payload

{ "json": { "eventId": "4a60fb38-80dd-49e4-a0a2-3ea372b8df72", "data": { "applicant.firstname": "Riku", "applicant.surname": "Rouvila", "applicant.dob": "2024-05-03", "applicant.image.label": "My driver's license", "recommender.none": "No", "recommender.firstname": "Pyry", "recommender.surname": "Rouvila", "recommender.id": "234234234", "applicant.image": { "filename": "7f879543-3320-4bea-b6a9-55100b48140e.png", "originalFilename": "Screenshot 2025-01-29 at 10.33.24.png", "type": "image/png" } }, "transactionId": "f959db23-cd01-4879-9d90-dd7d645154b5" } }

rikukissa · 2025-02-05T13:51:38Z

packages/client/src/v2-events/components/forms/utils.ts

+      const selectedCountry = countries.find(
+        (country) => country.value === value
      )
-    case FieldType.COUNTRY:
-      return selectCountryFieldToString(value as SelectFieldValue, intl)
+      return selectedCountry ? intl.formatMessage(selectedCountry.label) : ''


This logic should be inside the <Country /> registered input

rikukissa · 2025-02-05T13:54:17Z

packages/client/src/v2-events/components/forms/utils.ts

-        fieldConfig.options,
-        intl
+
+      return selectedOption ? intl.formatMessage(selectedOption.label) : ''


Is this correct? I'm guessing we do not want to store formatted values in the form data instead, right?

Each field type provides both methods for transforming values

rikukissa · 2025-02-06T14:18:45Z

packages/client/src/v2-events/features/events/components/Output.tsx

+export function Output({
+  field,
+  value,
+  previousValue,
+  showPreviouslyMissingValuesAsChanged = true
+}: {
+  field: FieldConfig
+  value?: FieldValue
+  previousValue?: FieldValue
+  showPreviouslyMissingValuesAsChanged: boolean
+}) {
+  if (!value) {
+    if (previousValue) {
+      return <ValueOutput config={field} value={previousValue} />
+    }
+
+    return ''
+  }
+
+  if (previousValue && previousValue !== value) {
+    return (
+      <>
+        <Deleted>
+          <ValueOutput config={field} value={previousValue} />
+        </Deleted>
+        <br />
+        <ValueOutput config={field} value={value} />
+      </>
+    )
+  }
+  if (!previousValue && value && showPreviouslyMissingValuesAsChanged) {
+    return (
+      <>
+        <Deleted>
+          <ValueOutput config={field} value={'-'} />
+        </Deleted>
+        <br />
+        <ValueOutput config={field} value={value} />
+      </>
+    )
+  }
+
+  return <ValueOutput config={field} value={value} />
+}


This function could almost inside components/Review/Output.tsx as it's only used in the Review page

Goes over the file length limit :(

reverts earlier check

ocrvs-bot · 2025-02-07T07:01:11Z

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

makelicious · 2025-02-07T09:27:29Z

on e2e, death 6.2. failed (the flaky one). Risking it all and merging

chore: refactor FieldValue types to be simpler to use

77cc1b3

makelicious commented Feb 5, 2025

View reviewed changes

makelicious added 2 commits February 5, 2025 13:33

fix: unify options and configuration properties for all fields

8af94e1

fix: add default values for all action value forms

6e5db73

makelicious requested review from rikukissa, tareq89, jamil314 and Nil20 February 5, 2025 11:46

rikukissa reviewed Feb 5, 2025

View reviewed changes

makelicious added 2 commits February 5, 2025 14:08

chore: add linting for commons/events

cf4bca7

fix: resolve circular dependencies

7e03cab

rikukissa approved these changes Feb 5, 2025

View reviewed changes

rikukissa reviewed Feb 5, 2025

View reviewed changes

makelicious added 3 commits February 6, 2025 15:38

feat: initial refactoring to use input-ouput pattern

6de0098

Each field type provides both methods for transforming values

Merge branch 'develop' into ocrvs-8397-b

a5ec079

fix: stop requiring defined fields, fix checkbox issue

197fb68

makelicious requested a review from rikukissa February 6, 2025 14:01

rikukissa reviewed Feb 6, 2025

View reviewed changes

rikukissa approved these changes Feb 6, 2025

View reviewed changes

makelicious added 5 commits February 6, 2025 17:20

chore: remove unneeded checks and imports

9ecfb7b

chore: remove unused exports

01a800f

chore: remove linting for now

f0a9a84

reverts earlier check

fix: use correct import format

ef41ee5

chore: remove comments

f0200b6

makelicious added the 🚀 Ready to deploy Deployment automation should pick this PR up and start auto-deploying it label Feb 6, 2025

github-actions bot temporarily deployed to ocrvs-8397-b February 6, 2025 15:48 Inactive

makelicious added 2 commits February 6, 2025 17:50

fix: bring back accidentally removed schema

206cf7c

fix: add comments and use correct import

f28086c

makelicious temporarily deployed to ocrvs-8397-b February 6, 2025 15:50 — with GitHub Actions Inactive

makelicious added 🚀 Ready to deploy Deployment automation should pick this PR up and start auto-deploying it and removed 🚀 Ready to deploy Deployment automation should pick this PR up and start auto-deploying it labels Feb 7, 2025

github-actions bot temporarily deployed to ocrvs-8397-b February 7, 2025 06:54 Inactive

chore: upgrade toolkit

dd50986

makelicious added 3 commits February 7, 2025 09:20

fix: use correct type for page header

370b8af

fix: update checkbox logic

b165324

chore: update toolkit, allow passing boolean as param

0f16abe

makelicious deployed to ocrvs-8397-b February 7, 2025 09:00 — with GitHub Actions Active

makelicious merged commit f6786bf into develop Feb 7, 2025
73 of 74 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up Field types #8578

Clean up Field types #8578

makelicious commented Feb 5, 2025 •

edited

Loading

github-actions bot commented Feb 5, 2025

makelicious Feb 5, 2025

makelicious Feb 5, 2025

makelicious Feb 5, 2025

makelicious Feb 5, 2025

makelicious Feb 5, 2025

makelicious Feb 5, 2025

rikukissa Feb 5, 2025

rikukissa Feb 5, 2025

rikukissa Feb 5, 2025

rikukissa Feb 5, 2025

rikukissa Feb 5, 2025 •

edited

Loading

rikukissa Feb 5, 2025

rikukissa Feb 5, 2025

rikukissa Feb 6, 2025

makelicious Feb 6, 2025

ocrvs-bot commented Feb 7, 2025

makelicious commented Feb 7, 2025

		@@ -88,43 +77,10 @@ const BaseField = z.object({

		export type BaseField = z.infer<typeof BaseField>

		export const FieldType = {

Clean up Field types #8578

Clean up Field types #8578

Conversation

makelicious commented Feb 5, 2025 • edited Loading

github-actions bot commented Feb 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rikukissa Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocrvs-bot commented Feb 7, 2025

makelicious commented Feb 7, 2025

makelicious commented Feb 5, 2025 •

edited

Loading

rikukissa Feb 5, 2025 •

edited

Loading