Finish unsharing hacks for contribution preferences form

[eileenmcnaughton]

Overview

This is (almost) the last of a series of updates to remove technical debt in the 'preferences' forms.

Before

Hacks to support non-spec settings for invoicing distributed in weird & wonderful places

After

Hacks constrained into the relevant form & commented

Technical Details

Most detail is in https://lab.civicrm.org/dev/core/issues/495

Comments

I have 2 issues still with this

1. the wysiwig field isn't working properly for me - but I think it may be a local issue
2. the help text on the financial_type_acl setting is lost - I'd rather tackle this as a follow up as we can establish metadata for it per the main DAO schema

I think it would be easier to merge #13046 first & I'll rebase this. It IS a bit of a hatchett job & is mostly about UI testing on the one form

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

[mattwire]

@eileenmcnaughton Please ping when this one is ready

[eileenmcnaughton]

@mattwire it is ready but merging #13046 first & rebasing is cleaner

[eileenmcnaughton]

@mattwire @pradpnayak this is rebased now

[pradpnayak]

Agreed, 'contribution_invoice_settings' is being stored in unusual way. Changing this now might cause problems for Sales tax or Deferred revenue since I believe they all rely on this setting.

[eileenmcnaughton]

was I too grumpy in my code comments :-)

[pradpnayak]

@eileenmcnaughton I tested this and it works very well now and also the field labels are not printed twice. Also checked 'contribution_invoice_settings' setting variable, it does hold all the Sales Tax settings. Thank you so much for code cleanup.

However there is one issue like you mentioned
-- Help Text(?) for 'Enable Access Control by Financial Type'

The wysiwig field does show up on my machine.

I am not sure about the ACL test file if they have any meaning to this PR. But this is GOOD TO MERGE!

Thanks
Pradeep

[eileenmcnaughton]

Thanks @pradpnayak once this is merged the things I / anyone prepared to help will ideally do are

1. address the help text issue
2. see if I can create the generic form per https://lab.civicrm.org/dev/core/issues/495
3. update docs
4. look at handling casing in a noisy but non-fail-y way
5. if you want to put your PR up as an improvement I'll review & merge that
6. audit settings / preferences forms to check for any failures