Merge pull request #19228 from eileenmcnaughton/inv_assign

Remove duplicated tax assignments from copied code

Merge pull request #19224 from artfulrobot/artfulrobot-fix-bulkmail

Fix lab issue 2254 is_bulkmail cannot be set through UI

Merge pull request #19231 from eileenmcnaughton/processConfirm

Convert previously shared function from static to non-static

Merge pull request #19234 from eileenmcnaughton/cms

Only do cms account create from the one relevant place

Merge pull request #19226 from eileenmcnaughton/odd

Remove unused tpl assigns

Merge pull request #19233 from civicrm/5.33

5.33

Only do cms account create from the one relevant place

This function is called from 3 places - create CMS user is not applicable
to the back office form and the other place actually blocks it...

Convert previously shared function from static to non-static

This no longer needs to be static as it is no longer shared with other forms
(although it needs to be public to support the test class

Merge pull request #19230 from totten/master-phpseclib-cxnrpc

composer.json - Update civicrm-cxn-rpc and phpseclib

composer.json - Update civicrm-cxn-rpc and phpseclib

Before
------

Require `civicrm-cxn-rpc` v0.19 (with `phpseclib` v1.x)

After
-----

Require either of:

* `civicrm-cxn-rpc` v0.20 (with `phpseclib` v2.x)
* `civicrm-cxn-rpc` v0.19 (with `phpseclib` v1.x)

Technical Details
-----------------

* The public interfaces from civicrm-cxn-rpc are the same in 0.19+0.20.
  They only differ in which verison of `phpseclib` is used.
* As pointed out in https://github.com/civicrm/civicrm-cxn-rpc/issues/9, we're not
  the only folks using phpseclib, so some flexibility on that seems good.
* The primary change in phpseclib 2.x is the use of PHP namespaces
  (e.g. `Crypt_AES` => `\phpseclib\Crypt\AES`).
* There are newer versions of both v0.19 and v0.20 which bundle an updated certificate.

Remove duplicated tax assignments from copied code

The assignment of tax data is happening twice on the form - once in generic
code and once in code only reached for recurring contributions that
was in previously-shared code. We can be fairly comfortable that
in this latter case we don't need it as this is a marginal flow
on this form whereas the main flow is being used 90% of the time
& is doing the assignment

Fix lab issue 2254: cannot set is_bulkmail from UI

Remove legacy tpl assigns

These really look like they were copied over from participant forms -
they are not used in the membership tpl receipts

Merge pull request #19216 from laryn/patch-3

dev/core#2211 Make sure addressee field fits column

dev/core#2211 Make sure addressee field fits column

Truncate if addressee field goes beyond the available 255 characters in an export that merges contacts by shared address.

Merge pull request #19223 from civicrm/5.33

5.33

Merge pull request #19220 from eileenmcnaughton/nfc

Minor code cleanup

Merge pull request #19222 from eileenmcnaughton/mem_form

Remove all handling related to pledge, cms user from newly separated function

Merge pull request #19219 from colemanw/targetTableFix

Fix Invalid argument PHP warning

Merge pull request #19221 from civicrm/5.33

5.33

Remove all handling related to pledge, cms user from newly separated function

Merge pull request #19217 from totten/master-assert-perm

DispatchPolicy - Actively report any upgrade problems with hook_civicrm_permission

Merge pull request #19211 from eileenmcnaughton/form_move

Duplicate processFormContribution only Membership form

Minor code cleanup

Fix Invalid argument PHP warning

DispatchPolicy - Actively report any upgrade problems with hook_civicrm_permission

Overview
--------

This is a preventive/diagnostic revision which would bring to light potential
problems with firing `cleanupPermissions()`/`hook_civicrm_permission` at the
wrong moment during an upgrade.

Before
------

If `cleanupPermissions()` (and its `hook_civicrm_permission`) are fired too
early during an upgrade, then the cleanup runs quietly but omits important
results (because the hook is dropped).  On several UF's, the
`cleanupPermissions()` **revokes access** for any omitted permissions.  The
sysadmin would have manually re-grant access.

After
-----

If `cleanupPermissions()` (and its `hook_civicrm_permission`) are fired too
early during an upgrade, then it raises an error during the upgrade.

Surely, it's better to report the error rather than silently drop the data.

Comments
--------

* This revision is preventive/diagnostic/speculative.  It's based on rumor that
  somebody had a problem with permissions in an upgrade.  I don't actually have
  steps to reproduce a probelmatic case.

* To see this preventive mechanism in action, I provoked a trivial violation:
    * Setup a DB with 5.32
    * Checkout the code for 5.34.
    * Add a local hack https://gist.github.com/totten/e1448b343f94ff9c3d971402a1b3db3d
      which has the effect of running `cleanupPermissions()` prematurely.
    * Observe: `cv upgrade:db` fails (cv v0.3.5+)

* It's tempting to think of `hook_civicrm_permission` as returning a static list of
  strings -- in which case, there's no real harm to firing during the defensive
  phase.  The problem is that `hook_civicrm_permission` can also be used for
  dynamic scenarios.  (The power of hooks!) That's useful for any site-building
  extension (e.g.  bespoke forms/views/APIs/dashlets) where you generate new
  permissions based on site configuration.  That will depend on the
  correct-functioning of the extension+configuration...  which cannot be
  ensured during the defensive upgrade-phase...  and which creates a parallel
  choice between incorrect-results (data-loss) or fatal-error.  This is the
  real reason why one shouldn't run `cleanupPermissions()` during the defensive
  phase.  (Of course, we should run it...  during the liberal phase...)

Merge pull request #19019 from eileenmcnaughton/total_fail

dev/core#927  Fully remove cancel & fail from Contribution BAO

Merge pull request #19207 from eileenmcnaughton/member_ids

[REF] Clean up on $ids['contribution']

Duplicate processFormContribution only Membership form

Merge pull request #19212 from eileenmcnaughton/form_move2

Move processConfirm function from Utils file back to form class

Merge pull request #19215 from totten/master-dispatch-doc

(NFC) DispatchPolicy - Add comments to docblock

Merge pull request #19213 from eileenmcnaughton/dep

Remove deprecated function

(NFC) DispatchPolicy - Add comments to docblock

This updates some of the docblocks to reflect https://github.com/civicrm/civicrm-core/pull/17126

Remove deprecated function

Move processConfirm function from Utils file back to only form still using it

This function is really part of the Confirm form. People thought sharing it would
be good once. They were wrong. More specifically sharing large blocks of
code that attempt to service many diferent needs is not helpful

Merge pull request #19208 from eileenmcnaughton/mem_deb

Remove some more variable variables + some test cleanup

Merge pull request #19181 from colemanw/api4DateRange

APIv4: Support relative date range input

Merge pull request #19206 from seamuslee001/migrate_print_array

#REF Migrate the print_array smarty plugin from in packages into core…

Cleanup on test classes to support more flexible use of price set work

Remove some more variable variables

Merge pull request #19205 from eileenmcnaughton/mem_forms

[REF] Move function to shared parent so MemberForm can use it too

REF Migrate the print_array smarty plugin from in packages into core as it seems to not be supplied by the upstream package

[REF] Clean up on ids['contribution']

This removes ids['contribution'] and uses the simpler variable contributionID
In addition 2 if clauses are wrapped in if (contributionID) to make it clear
that both are only reachable when there is no contributionID

Merge pull request #19068 from eileenmcnaughton/actsched

Add column created_date to action_schedule

Merge pull request #19204 from eileenmcnaughton/ref

Stop passing ids as reference

Merge pull request #19203 from eileenmcnaughton/import

Remove unreachable code.

[REF] Move function to shared parent so MemberForm can use it too

Merge pull request #19201 from eileenmcnaughton/notice

Enotice fix

Stop passing ids as reference

There are 3 remaining places in the code that call Membership::create with
the ids variable. From my digging none of them use ids afterwards
so there is no need to pass by reference

Remove unreachable code.

The addressee() function doesn't exist so we can be pretty sure it's not being reached

Don't run singleValueAlter on modified_date or create_date

Since we expect these to be managed by mysql to some extent they seem best ignored

Add effective start and end date plus created & modified date to action schedule table

Enotice fix

Merge pull request #19196 from seamuslee001/apiv4_blob_fields

REF Allow for fields of type Blob or Mediumblob in Apiv4

Merge pull request #19198 from seamuslee001/xdebug_singlevalue

NFC When printing out the result of the correctly update in single va…

NFC When printing out the result of the correctly update in single value alter ensure that xdebug isn't printed

Merge pull request #19190 from eileenmcnaughton/import22

Squash 2 if clauses into 1

Merge pull request #19197 from demeritcowboy/key-comments

[NFC] Update comments in CRM/Core/Key

update comments

REF Allow for fields of type Blob or Mediumblob in Apiv4

Merge pull request #19187 from civicrm/5.33

5.33

Merge pull request #19145 from totten/master-qfkey

CRM_Core_Key - Provide more debugging hints about mismatched`qfKey`s

Merge pull request #19193 from eileenmcnaughton/ref

[REF] Extract determination of subscription status information

Merge pull request #19192 from totten/5.33-upgrade

dev/core#2232 - Upgrade UI contaminates cache via l10n-js. Consolidate isUpgradeMode().

Merge pull request #19160 from eileenmcnaughton/import

Clean up error handling in legacy functions in import parser

Merge pull request #19191 from eileenmcnaughton/deprecated

Remove some deprecated code chunks

Merge pull request #19195 from seamuslee001/5322_rn

Add release-notes/5.32.2.md

Add release-notes/5.32.2.md

[REF] Extract determination of subscription status information

This gets us away from it being buried in a switch

Remove some deprecated code chunks

Clean up error handling in legacy functions in import parser

This makes the handling of errors cleaner & makes it easier for us to unravel what is going on here.

Merge pull request #19189 from eileenmcnaughton/533q1

Fix failure to assign view tpl variables to view page if context=search is in the url

Partial revert "dev/core#2232 Permit hook_civicrm_container and some other prebootish hooks to run during upgrade and clear out the asset builder cache post upgrade"

This reverts commit 756d9e0dbbe4ff66d2568f02a8ee1f152bd9c5e5.

dev/core#2232 - Upgrade UI contaminates cache via l10n-js. Consolidate isUpgradeMode().

This patch fixes an upgrade bug where `CachedCiviContainer` has stale data after an upgrade.

To do this, it removes an edge-case where two overlapping functions disagree.

Steps to Reproduce
------------------

1. Create a new/empty WP build
2. Install CiviCRM 5.30.1 from zipball
3. Install Mosaico 2.5
4. Download+extract new 5.32.1 zipball
5. Navigate to the GUI upgrade screen. Execute upgrade
6. Click to go back to the CiviCRM dashboard.
7. Receive error about the service 'mosaico_graphics'

Discussion
----------

The problem is created while running the upgrade GUI (step 5).  The GUI sends several HTTP
requests (`civicrm/upgrade`, `civicrm/upgrade/queue/ajax/runNext`, etc).  At the end, there is
an HTTP request for `civicrm/ajax/l10n-js/en_US`.  The `l10n-js` request creates a flawed copy
of `CachedCiviContainer` which is responsible for subsequent errors.

This shouldn't happen - there are defensive mechanism which prevent it from happening (e.g.
during `civicrm/upgrade`).  What makes `civicrm/ajax/l10n-js/en_US` different?  It turns on
the implementation of `isUpgradeMode()` which activates the defensive mechanisms.

Before
------

There are two implementations of `isUpgradeMode()` (via `CRM_Core_Config` and
`CRM_Utils_System`).  They often agree, but not always.

Some defensive measures trigger on `CRM_Core_Config::isUpgradeMode()` and others trigger on
`CRM_Utils_System::isUpgradeMode()`.  They often trigger together, but not always.

Let's see how these can playout in a few HTTP requests:

1. `civicrm/dashboard`: The functions agree -- it's a regular page, not an upgrade.  Therefore:
     * (a) It allows extensions to run fully.
     * (b) It enables `CachedCiviContainer` (read or write automatically).
     * (a+b) It puts good/full information in the cache.
2. `civicrm/upgrade`: The functions agree -- it's an upgrade. Therefore:
     * (a) It runs in paranoid mode (suspended extensions).
     * (b) It disables `CachedCiviContainer`.
     * (a+b) It puts no information in the cache.
3. `civicrm/ajax/l10n-js/en_US`: The functions *disagree*. In this case:
     * (a) It runs in paranoid mode (suspended extensions).
     * (b) It enables `CachedCiviContainer` (read or write automatically).
     * (a+b) It puts bad/incomplete information in the cache.

After
-----

There is one implementation of `isUpgradeMode()`.  It may be called through
either class (`CRM_Core_Config` or `CRM_Utils_System`), but the results will
always agree.

This produces the same appropriate outcome for cases of agreement (1) (2), and it fixes the
wonky/mismatched behavior in (3).

Squash 2 if clauses into 1

The action is the same for both criteria so in_array rather than 2 * if =
makes it more readable

Merge pull request #19185 from eileenmcnaughton/533q

dev/core#1019 Fix currency formatting of Total Amount on Event and Contribution pages (with multi-currency form support)

Fix failure to assign view tpl variables to view page if context=search is in the url

Whack-a-mole round 10

Merge pull request #19183 from eileenmcnaughton/533e

dev/core#2248 Ensure variables are assigned to tpl for urls

dev/core#1019 Fix currency formatting of Total Amount on Event and Contribution pages (with multi-currency form support)

dev/core#2248 Ensure variables are assigned to tpl for urls

Merge pull request #19182 from eileenmcnaughton/else

Remove extraneous elses

Merge pull request #19085 from eileenmcnaughton/words

dev/financial#158  change UI parts of contribution soft schema to soft credit

CRM_Core_Key - Add some basic unit tests

CRM_Core_Key - Provide more debugging hints about `qfKey`s

Overview
--------

The `qfKey` parameter is a security mechanism (CSRF). The content of `qfKey` is also inscrutable - so when there's a problem
with `qfKey`, it can be difficult to determine the origin/nature of the problem.

Before
------

The `qfKey` provides a digital signature based on (1) session ID, (2) the
form being processed, (3) a random private-key (unique to the user/session).
In some cases, the `qfKey` also has a random nonce appended to distinguish
between concurrent tabs that work with the same form.

* Ex: `2abc4b7f23d9ae4dfcdbb0c692cc666e5f11256fe84ace7662b0e075834b81958` (w/o nonce)
* Ex: `2abc4b7f23d9ae4dfcdbb0c692cc666e5f11256fe84ace7662b0e075834b81958_9527` (w/nonce)

If there is a logic problem where the `qfKey` of form A gets mixed-up with
form B, then it's extremely hard to understand the mismatch All you see are
two long random codes.

* Inputted qfKey: `2abc4b7f23d9ae4dfcdbb0c692cc666e5f11256fe84ace7662b0e075834b81958`
* Expected fqKey: `89874b7a23192e4d6aab10c622ca369e5e11226f784a5c7652b95075536b81a5e`

After
-----

The `qfKey` has a prefix to indicate the token's intended usage (as well as a digital signature).

* Ex: `CRMContactControllerSearch154eitbf74v3ko0sw94k08gogo8448asdfw4goggkkwgkww08c` (w/o nonce)
* Ex: `CRMContactControllerSearch154eitbf74v3ko0sw94k08gogo8448asdfw4goggkkwgkww08c_9355` (w/ nonce)

If there is a logic problem where the `qfKey` of form A gets mixed-up with
form B, then the prefix can help you understand. Compare:

* Inputted qfKey: `CRMContactControllerSearch154eitbf74v3ko0sw94k08gogo8448asdfw4goggkkwgkww08c`
* Expected qfKey: `CRMContributeFormContribution5o2due205mp0384080sgc8omw8kwggoksw47sswchs80gw0kgs`

This tells you that there's logical mismatch - it's trying to render the contribution
screen using the key for the contact-search screen.

Comments
--------

* The identity of the `formname` is not sensitive information.
* The `qfKey` might look prettier with more delimeters (`{formname}_{signature}_{nonce}`). However, I belive there are random bits of existing code
  which use `explode('_')` to split apart the signature and the nonce. This formula seems to be drop-in/interoperable.
* I switched the encoding of the signature from hex (0-9a-f) to base-36 (0-9a-z) to make it a bit shorter.

Remove extraneous elses

dev/financial#158  change UI parts of contribution soft schema to soft credit

APIv4: Support relative date range input

Supports relative date range expressions with most operators and adds test coverage

Merge pull request #19180 from civicrm/5.33

5.33

Merge pull request #19179 from demeritcowboy/depwarn

[NFC] Convert civi.tag-deprecated to use central function

Merge pull request #19176 from eileenmcnaughton/533m

dev/core#2246 Fix failure to filter exports

Merge pull request #19172 from eileenmcnaughton/dep_match

[REF] extract getIdsOfMatchingContact

Merge pull request #19167 from demeritcowboy/legacy-warnings

Unused writeLegacyWarnings() in PropertyBag

convert to central function

[REF] extract getIdsOfMatchingContact

It seems this really needs sorting to be shared between the parser classes
to fix up the error handling

Merge pull request #19171 from eileenmcnaughton/else

Remove extraneous elses

Merge pull request #19178 from civicrm/5.33

5.33 to master

Merge pull request #19173 from eileenmcnaughton/533

dev/core#2244 Simplify and consistently apply checking of whether financial acls are enabled

dev/core#2246 Fix failure to filter exports

This fixes regression caused by us more consistently setting componentTable.

The short version here is that the presence of a componentClause should take precedences
over the inner join intended as a blunt, and maybe unnecessary, measure to
limit the export to the entity in question

Merge pull request #19164 from eileenmcnaughton/member

Switch to non-static functions

dev/core#2244 Simplify and consistently apply checking of whether financial acls are enabled

Per https://lab.civicrm.org/dev/core/-/issues/2244 the move of some handling to this function
did not consistently include an early return when the setting is toggled off.

Eventually there will be no setting - just 'is this extension enabled or not' but
in the transition we need this. The new version is easier to copy and paste :-)
and uses a function in the extension. The function in the main code base is
mostly a lot of code to handle the way the setting used to be stored & it
could just check the setting too now