Lint Fix: Fix for new Phone numbers schema change

[dr-bizz]

Description

This PR is going to be for when this PR https://github.com/CruGlobal/mpdx_api/pull/2760/files goes live.
It fixes a lint issue and ensures a null phone number won't cause an error.

Checklist:

• I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
• I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
• I have requested a review from another person on the project

[canac]

I don't think this will help. I believe it's number that will be null, not phoneNumber. And if phoneNumber is null, all the other property access calls with throw an exception too.

[dr-bizz]

I didn't know about this one, I added it just in case. I want to test with a Null number. I'll do that tomorrow

[aws-amplify-us-east-1]

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-861.d3dytjb8adxkk5.amplifyapp.com

[dr-bizz]

In staging there is an issue with deleting an existing null phone number. But this is due to the staging file. On production, it works fine. There have been a lot of changes to the PersonModal file with the new Preferences, so it's likely that causing it. In the Production personModal, it is working fine.

[canac]

Besides the test and type suggestions I mentioned, the code looks pretty good!

Thanks to the schema tweak, invalid phone numbers shouldn't cause issues anymore, and the user can edit them if they open the Edit Person modal. I'm wondering if we need to show the user the error and should instead just show a blank number like the old MPDX. I guess my question is whether a missing phone number is important enough of an issue to warrant showing an error message like this.

[dr-bizz]

@canac Sorry, I forgot to push up my changes. My changes are also in staging.

[canac]

Code seems to work well locally! I'm going to go ahead and approve since all my suggestions are just stylistic.

[canac]

I think you could add otherwise: yup.string().required(t('This field is required')), instead of a second .when block.

[canac]

Usually variables that start with has are booleans. Would it make sense to rename this to nullPhoneNumbers?

[canac]

Actually, it might make more sense to combine these .filter and .forEach loops.

if (!phoneNumbers) {
  return;
}

phoneNumbers.forEach((phoneNumber) => {
  const index = phoneNumbers.findIndex((phone) => phone.id === phoneNumber.id);
  if (phoneNumber.number === null) {
    setFieldError(`phoneNumbers.${index}.number`, t('Please enter a valid phone number'));
  }
});

[canac]

@dr-bizz Also the tests are failing because a mock sets a non-nullable phone number to null, but that test will pass once Shelby's PR gets merged. I'm not sure which PR we plan to merge first, but I'm OK with merging this one first despite the failure if you do it that way. Just make sure to rerun the tests on main after the API PR gets merged to make sure that the tests all pass then.