dev/core#1936 Make the price field value label field not required

[seamuslee001]

Overview

This reverses a change made in 5.27 which makes the price field value, label field non required preventing string 'null' from being saved

Before

Field is required

After

Field is not always required

Technical Details

I made sure apiv4 matched apiv3 in that the label is required if your using the API

ping @eileenmcnaughton @KarinG @demeritcowboy

[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).

[eileenmcnaughton]

@seamuslee001 so I'm inclined to go with it not being required in either api - I think the reason it was required was nasty munging behaviour. If that's hard to deal with I'm OK with apiv4 not matching v3 - as long as v4 is 'right'

(am also OK punting that for master)

[seamuslee001]

I have no idea but it seems it has been required in the API since v4.5 e825f02 so it would feel very strange to break that but maybe its ok for v4. Also the JIRA mentioned in that commit looks wrong https://issues.civicrm.org/jira/browse/CRM-14788. I would say keep label required for the moment and can manage in master

[eileenmcnaughton]

Looks good to me - I'll see what jenkins & @KarinG have to say

[KarinG]

Hi - my d8civicrm.local is a 5.27.4 at the moment so figured I would try this:

1. I took https://patch-diff.githubusercontent.com/raw/civicrm/civicrm-core/pull/18123.diffand applied the hunks possible

karins-MBP:civicrm-core sysadmin$ cat 18123.diff |patch -p1
patching file CRM/Core/I18n/SchemaStructure.php
patching file CRM/Price/DAO/PriceFieldValue.php
Hunk #1 FAILED at 6.
Hunk #2 succeeded at 219 (offset -7 lines).
Hunk #3 succeeded at 237 (offset -7 lines).
1 out of 3 hunks FAILED -- saving rejects to file CRM/Price/DAO/PriceFieldValue.php.rej
patching file CRM/Upgrade/Incremental/php/FiveTwentyNine.php
Hunk #1 FAILED at 74.
1 out of 1 hunk FAILED -- saving rejects to file CRM/Upgrade/Incremental/php/FiveTwentyNine.php.rej
patching file Civi/Api4/Service/Spec/Provider/PriceFieldValueCreationSpecProvider.php
patching file tests/phpunit/CRM/Price/BAO/PriceFieldValueTest.php
patching file xml/schema/Price/PriceFieldValue.xml

2. I took the content in CRM/Upgrade/Incremental/php/FiveTwentyNine.php and merged that function into FiveTwentySeven.php replacing the upgrade function that had already been executed with the new bits and naming it 2

3. I reset my civicrm_domain.version in the db back to 5.27.3

4. Then ran the upgrade (figured that was an important part here) - upgrade ran - and was happy:

Checking the db -> I still see "null"

Ok trying this manually:
mysql> UPDATE civicrm_price_field_value SET label = NULL WHERE label = 'null';
ERROR 1048 (23000): Column 'label' cannot be null

Manually phpMyAdmin will let me change 'null' to NULL

Ok manually changed -> I get (same contribution pages I reproduced the issue on)

Back to Configure -> my empty labels have NULL now - removing them.

Saving -> DB Constraint violation - ( ! )

Aug 10 20:55:47  [error] $Fatal Error Details = Array
(
    [callback] => Array
        (
            [0] => CRM_Core_Error
            [1] => handle
        )

    [code] => -3
    [message] => DB Error: constraint violation
    [mode] => 16
    [debug_info] => UPDATE  `civicrm_price_field_value`  SET `price_field_id` = 2 , `label` = NULL , `amount` = 5.00 , `weight` = 1 , `is_default` = 0 , `is_active` = 1 , `financial_type_id` = 1 , `visibility_id` = 1   WHERE (  `civicrm_price_field_value`.`id` = 2 )   [nativecode=1048 ** Column 'label' cannot be null]
    [type] => DB_Error
    [user_info] => UPDATE  `civicrm_price_field_value`  SET `price_field_id` = 2 , `label` = NULL , `amount` = 5.00 , `weight` = 1 , `is_default` = 0 , `is_active` = 1 , `financial_type_id` = 1 , `visibility_id` = 1   WHERE (  `civicrm_price_field_value`.`id` = 2 )   [nativecode=1048 ** Column 'label' cannot be null]
    [to_string] => [db_error: message="DB Error: constraint violation" code=-3 mode=callback callback=CRM_Core_Error::handle prefix="" info="UPDATE  `civicrm_price_field_value`  SET `price_field_id` = 2 , `label` = NULL , `amount` = 5.00 , `weight` = 1 , `is_default` = 0 , `is_active` = 1 , `financial_type_id` = 1 , `visibility_id` = 1   WHERE (  `civicrm_price_field_value`.`id` = 2 )   [nativecode=1048 ** Column 'label' cannot be null]"]
)

So what did we do before -> on a 5.27.3 site - I can't do [simple/clean no labels]

[seamuslee001]

adding merge on pass based on @KarinG testing on another PR and this being an identical copy except for the upgrade step