core#1132 - don't fire hook_civicrm_fieldOptions before hook_civicrm_…

[MegaphoneJon]

…config loads all files

https://lab.civicrm.org/dev/core/-/issues/1132

Overview

hook_civicrm_config is responsible for adding files to the autoloader. hook_civicrm_config calls hook_civicrm_fieldOptions. As a result, hook_civicrm_fieldOptions crashes if you reference a class that's not in core (i.e. in an extension).

Before

Crashes.

After

No crashes.

Technical Details

The backtrace is on https://lab.civicrm.org/dev/core/-/issues/1132, which explains why this happens.

Comments

This will hopefully fix the conflict that Mosaico 2.6 has with other extensions (like Areas).

Steps to replicate

• Create a new extension.
• Create a method on a class in that extension. Here's an example:

<?php

class CRM_Contribute_BAO_PaymentMethodByDomain {

  public static function getPaymentInstrument() {
    return TRUE;
  }

}

• In hook_civicrm_fieldOptions, call this method.
  At this point, most of CiviCRM will be broken. Clearing cache will certainly trigger the bug.

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

[seamuslee001]

So this would mean that extensions can't modify using the buildOptions Hook the list of available languages but I think that is an ok thing In my opinion ping @mlutfy @haystack @mattwire

[MegaphoneJon]

@seamuslee001 true. I wrote this mostly as a way to get some more knowledgeable eyes on the bug. If someone tells me this is the right general approach, I'll deal with test failures etc., but I'm not very confident this is the correct fix.

[mlutfy]

I'm OK with not being able to modify the list of languages via fieldOptions().

I've also encountered something similar on some niche configurations (emailapi + multilingual + symbiotic's monitoring extension, which does setLocale calls to make sure that the system supports all enabled translations with native gettext).

I can't comment on the other technical issues around it, but it would definitely need a code comment to avoid regressions :)

[MegaphoneJon]

I'll consider @mlutfy's comments to be a preliminary "concept approved", so I'll fix the issues present in this patch.

[eileenmcnaughton]

@mlutfy do you want to give this some light testing - it seems as agreed

[alifrumin]

I ran into veda-consulting-company/uk.co.vedaconsulting.mosaico#427 and this patch fixed it! Thanks @MegaphoneJon

[colemanw]

I think this probably should have gotten merged into the RC if it's fixing hard crashes. Anyone object to changing the base for this PR to the RC branch?

[eileenmcnaughton]

I'm happy with this going into the rc

[MegaphoneJon]

Now this is against the RC.