[REF] Fix e-notice in APIv3 Profile Test on PHP8

[seamuslee001]

Overview

This fixes an e-notice error on php 8 unit tests on 5.40, 5.41, master branches

Before

Test e-notice

api_v3_ProfileTest::testProfileSubmitInvalidProfileId
TypeError: uasort(): Argument #1 ($array) must be of type array, null given

/home/jenkins/bknix-edge/build/build-0/web/sites/all/modules/civicrm/api/v3/Profile.php:607

After

Test passes on php8

ping @eileenmcnaughton @totten

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[demeritcowboy]

At this point in the test, $profileFields is an empty array, so I would expect is_array($profileFields[$profileID]) to give an undefined index. In fact if you put an echo 'hi'; right after this line it doesn't output, so it's giving an E_NOTICE which is inadvertently fixing it by fullfilling the fail. If you put a var_dump($apiResult); in assertAPIFailure itself you can see this is what's happening.

Further, if there's no valid reason for this situation, would throwing an exception directly here be a better thing to do, because always converting to a valid thing might potentially hide other issues?

[seamuslee001]

@demeritcowboy the problem is that it is not an empty array on php8 it is NULL for $profileFields[$profileID]

[demeritcowboy]

$profileFields is an empty array, so $profileFields[$profileID] gives an ENOTICE when you use is_array on it.

[seamuslee001]

right sorry got confused, does my update work for you @demeritcowboy

[demeritcowboy]

Thanks. I'm going to put merge-ready because I'm confused about a couple things, but if someone wants to merge I won't object.

1. It seems like a (unintended?) change from dev/core#2743 fix api v3 to not unnecessarily load options #21099. Before it used to say "'1000' is not a valid option for field uf_group_id", and now, with this patch, it says "Invalid value for profile_id", which in this case yes is more accurate.
2. I'm still wondering about my comment from before: If there's never a valid reason for $profileFields[$profileID] to be null here, then converting it to a valid value instead of throwing an exception might end up hiding something else. In either case it's a change from before, because what it used to do before, funnily enough, was to actually create an entry for 1000 inside the array, i.e. after the uasort $profileFields = [1000 => NULL], and now after the uasort $profileFields = [1000 => []].

[eileenmcnaughton]

hmm - I think - yeah good points - but I guess if we are going to do a better fix it would be fine to do it in master & if no-one actually cares enough to do it then I guess that's fine if there is no reproduceable regression.

So I'm happy to merge this & maybe someone will fix more better in master