Merge pull request #13810 from totten/master-wp-stdrr

(ops#878) phpunit.xml.dist - Prevent conflict in WordPress E2E with "stderr=true"

Merge pull request #13811 from totten/master-wp-loadboot

(ops#878) Enforce common signature for CRM_Utils_System::loadBootStrap(). Fix WP E2E error.

Merge pull request #13383 from seamuslee001/report_cleanup_mailing_summary

(dev/core#491) Standardise the adding of campaign fields on the maili…

Merge pull request #13814 from mattwire/testentity

Add a class to handle test entities consistently

Merge pull request #13812 from mattwire/corepayment_comments

Add comments about usage for doPayment() function

Merge pull request #13817 from civicrm/5.12

5.12 to master

Merge pull request #13808 from totten/5.12-wp-e2e

(ops#878) (Fast)ArrayDecorator - Emit expected exception when using WP and strict PSR-16

(ops#878) Enforce common signature for CRM_Utils_System::loadBootStrap(). Fix WP E2E error.

Before
------

1.  Most `CRM_Utils_System_*` classes implement a method `loadBootStrap($params = array(),...)`.  However, `WordPress`
   and `UnitTests` implement a different signature (`loadBootStrap($name = NULL, $pass = NULL)`), and `Soap` does not
   implement it all.  This is problematic -- because the function is invoked in a centralized way (e.g.
   `CRM_Utils_System::loadBootStrap()` invokes `$config->userSystem->loadBootStrap($params, $loadUser, $throwError,
   $realPath)`), which implies that all UF drivers need to support this signature.

2. When running any of the end-to-end (E2E) tests on CiviCRM-WordPress, the output is polluted with these warnings:

```
bknix-dfl:~/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm] phpunit5 tests/phpunit/E2E/Cache/FastArrayDecoratorTest.php ; echo "exit=$?"
PHPUnit 5.7.27 by Sebastian Bergmann and contributors.

PHP Warning:  strip_tags() expects parameter 1 to be string, array given in /Users/totten/bknix/build/wpmaster/wp-includes/formatting.php on line 4649
PHP Stack trace:
PHP   1. {main}() /Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar:0
PHP   2. PHPUnit_TextUI_Command::main() /Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar:598
PHP   3. PHPUnit_TextUI_Command->run() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/Command.php:116
PHP   4. PHPUnit_TextUI_TestRunner->doRun() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/Command.php:186
PHP   5. PHPUnit_Framework_TestSuite->run() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/TestRunner.php:517
PHP   6. call_user_func:{phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679}() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679
PHP   7. E2E_Cache_CacheTestCase::setUpBeforeClass() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679
PHP   8. CRM_Utils_System::loadBootStrap() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/tests/phpunit/E2E/Cache/CacheTestCase.php:43
PHP   9. CRM_Utils_System_WordPress->loadBootStrap() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/CRM/Utils/System.php:1477
PHP  10. wp_authenticate() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/CRM/Utils/System/WordPress.php:495
PHP  11. sanitize_user() /Users/totten/bknix/build/wpmaster/wp-includes/pluggable.php:515
PHP  12. wp_strip_all_tags() /Users/totten/bknix/build/wpmaster/wp-includes/formatting.php:1850
PHP  13. strip_tags() /Users/totten/bknix/build/wpmaster/wp-includes/formatting.php:4649

Warning: strip_tags() expects parameter 1 to be string, array given in /Users/totten/bknix/build/wpmaster/wp-includes/formatting.php on line 4649

Call Stack:
    0.0026     425040   1. {main}() /Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar:0
    0.0766   14246216   2. PHPUnit_TextUI_Command::main() /Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar:598
    0.0766   14246952   3. PHPUnit_TextUI_Command->run() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/Command.php:116
    0.4267   23405608   4. PHPUnit_TextUI_TestRunner->doRun() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/Command.php:186
    0.4336   23511184   5. PHPUnit_Framework_TestSuite->run() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/TestRunner.php:517
    0.4403   23516840   6. call_user_func:{phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679}() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679
    0.4403   23516920   7. E2E_Cache_CacheTestCase::setUpBeforeClass() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679
    0.5065   31975824   8. CRM_Utils_System::loadBootStrap() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/tests/phpunit/E2E/Cache/CacheTestCase.php:43
    0.5065   31976344   9. CRM_Utils_System_WordPress->loadBootStrap() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/CRM/Utils/System.php:1477
    0.6517   57998912  10. wp_authenticate() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/CRM/Utils/System/WordPress.php:495
    0.6517   57998992  11. sanitize_user() /Users/totten/bknix/build/wpmaster/wp-includes/pluggable.php:515
    0.6517   57999088  12. wp_strip_all_tags() /Users/totten/bknix/build/wpmaster/wp-includes/formatting.php:1850
    0.6517   57999736  13. strip_tags() /Users/totten/bknix/build/wpmaster/wp-includes/formatting.php:4649
```

3.  The warning indicates that (on WordPress) the E2E test is not doing what it's trying to do -- specifically,
    `E2E_Cache_CacheTestCase::setUpBeforeClass()` (or, similarly, `CiviEndToEndTestCase::setUpBeforeClass()`) calls
    `CRM_Utils_System::loadBootStrap()` to authenticate as the CMS administrator, but this fails because WP's
    implementation of `loadBootStrap()` does not respect the same signature.

After
-----

1. `CRM_Utils_System_Base` defines an abstract function `loadBootStrap($params = array(),...)`. Any non-compliant driver (which
   uses a different signature or which omits the function) will produce a syntax error.

2. The classes `CRM_Utils_System_{WordPress,UnitTests}` have been updated to comply with the signature.

3. The class `CRM_Utils_System_Soap` nominally complies. However, this class isn't really a UF driver, and it doesn't make sense for
   it to extend this base class, and the implementation is a stub. But we don't need to scope-creep the bugfix.

4. The `loadBootStrap...strip_tags()...` warnings are no longer generated when running E2E tests on WP.

Comments
--------

* I verified that all extant classes in `CRM/Utils/System/*.php` are loadable/compilable (i.e they now comply with the delcared signature).
* The SOAP integration has test-coverage viia `E2E_Extern_SoapTest`.
* In the PR test output, you'll only see E2E results from D7.  *However*, this patch is part of a bigger effort
  (infrastructure/ops#878) which will eventually give some visibility on that.

Add a class to handle test entities consistently

Add comments about usage for doPayment() function

(ops#878) phpunit.xml.dist - Prevent conflict in WordPress E2E with "stderr=true"

Before
------

When running any of the end-to-end (E2E) tests on CiviCRM-WordPress, the output is polluted with these warnings:

```
[bknix-dfl:~/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm] phpunit5 tests/phpunit/E2E/Cache/FastArrayDecoratorTest.php
PHPUnit 5.7.27 by Sebastian Bergmann and contributors.

PHP Warning:  session_start(): Cannot send session cookie - headers already sent by (output started at phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Util/Printer.php:110) in /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm.php on line 346
PHP Stack trace:
PHP   1. {main}() /Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar:0
PHP   2. PHPUnit_TextUI_Command::main() /Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar:598
PHP   3. PHPUnit_TextUI_Command->run() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/Command.php:116
PHP   4. PHPUnit_TextUI_TestRunner->doRun() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/Command.php:186
PHP   5. PHPUnit_Framework_TestSuite->run() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/TextUI/TestRunner.php:517
PHP   6. call_user_func:{phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679}() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679
PHP   7. E2E_Cache_CacheTestCase::setUpBeforeClass() phar:///Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar/phpunit/Framework/TestSuite.php:679
PHP   8. CRM_Utils_System::loadBootStrap() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/tests/phpunit/E2E/Cache/CacheTestCase.php:43
PHP   9. CRM_Utils_System_WordPress->loadBootStrap() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/CRM/Utils/System.php:1477
PHP  10. require_once() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/CRM/Utils/System/WordPress.php:479
PHP  11. require_once() /Users/totten/bknix/build/wpmaster/wp-load.php:37
PHP  12. require_once() /Users/totten/bknix/build/wpmaster/wp-config.php:94
PHP  13. do_action() /Users/totten/bknix/build/wpmaster/wp-settings.php:325
PHP  14. WP_Hook->do_action() /Users/totten/bknix/build/wpmaster/wp-includes/plugin.php:453
PHP  15. WP_Hook->apply_filters() /Users/totten/bknix/build/wpmaster/wp-includes/class-wp-hook.php:323
PHP  16. call_user_func_array:{/Users/totten/bknix/build/wpmaster/wp-includes/class-wp-hook.php:298}() /Users/totten/bknix/build/wpmaster/wp-includes/class-wp-hook.php:298
PHP  17. CiviCRM_For_WordPress->setup_instance() /Users/totten/bknix/build/wpmaster/wp-includes/class-wp-hook.php:298
PHP  18. session_start() /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm.php:346
```

After
-----

The session_start warnings are no longer generated..

Comments
--------

The problem appears to be this:

1. When PHPUnit starts, it puts some messages on STDOUT.
2. When the specific tests/test-suites are setup, they bootstrap WordPress, and WordPress tries to setup a session, which requires setting some headers
3. But we've already started writing STDOUT, and we can't send any headers.

The patch is based on suggestion [a StackExchange
discussion](https://stackoverflow.com/questions/23270650/cannot-send-session-cookie-headers-already-sent-phpunit-laravel/38045422)
-- basically, by setting the `stderr` option (which is documented lightly in the cli `phpunit5 --help`), we prevent
step 1 above -- because PHPUnit now prints messages to STDERR instead of STDOUT.

The only concern (`r-tech`) would be...  is there anything in our ecosystem which depends on piping information from
PHPUnit's STDOUT?  Obviously, I can't speak for third-parties, but the general pattern in `civicrm.org` CI scripts
(e.g.  `civi-test-run`) is to evaluate PHPUnit output through a mix of (1) exit-codes and (2) JUnit files.  In which case...
it doesn't matter if PHPUnit writes messages to STDERR or STDOUT.

I'd suggest paying close attention to the console output for the PR-test here; if that's good, then we're probably OK.

(Fast)ArrayDecorator - Emit expected exception when using WP and strict PSR-16

Suppose someone calls `ArrayDecorator::get()` or `FastArrayDecorator::get()`
with an invalid key (e.g.  passing `float` or an `array` instead of a
`string`).  This patch improves error-reporting for that scenario.

This is primarily about fixing multiple test failures in `E2E_Cache_ArrayDecoratorTest` reported on `wp-demo`. These
generally look like

```
1) E2E_Cache_ArrayDecoratorTest::testGetInvalidKeys with data set #1 (true)
array_key_exists(): The first argument should be either a string or an integer

/Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/CRM/Utils/Cache/ArrayDecorator.php:102
/Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/packages/Cache/IntegrationTests/LegacySimpleCacheTest.php:409
/Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar:598
```

Before
------

The ArrayDecorator first checks its front cache (`array_key_exists`) to see if the key is defined.  In the `wp-demo`
environment, this produces a warning and causes the test to fail.

The condition *is* erroneous, but PSR-16 specifies that the error should be reported as exception.

After
-----

The condition is reported as the expected exception. The test passes on `wp-demo`.

Comment
-------

This brings the code in `(Fast)ArrayDecorator.php` into alignment with most of the other `CRM/Utils/Cache/*.php`
drivers; in most drivers, it is typical to validate the key at the start of most functions.

Merge pull request #13806 from eileenmcnaughton/array_form

[NFC] reformat Contact api file to switch to new array formatting

Merge pull request #13803 from herbdool/issue-13788-alt

 civicrm/admin/setting/uf - Fix advice about Backdrop Views ($database_prefix)

[NFC] reformt Contact api file to switch to new array formatting

This is just an IDE reformat.  A few tweaks beyond the straight array swap but not much

CRM/Admin/Form/Setting/UF.tpl - Change "add" to "add or update"

If an admin follows the advice literally (and merely *adds* the item),
and if there's already an existing as directive higher-up (as
there would be in a stock Backdrop `settings.php`), then the
`settings.php` would have two copies of the directive.

That'd be liable to cause confusion during subsequent maintenance.

Merge pull request #13805 from mattwire/static

checkEditInboundEmailsPermissions should be a static function

(dev/core#491) Standardise the adding of campaign fields on the mailing summary report and add a post upgrade message about needing to re-save report given the changes made to the report template

Update language on postupgrade message

Move upgrade to 5.13

Merge pull request #13783 from eileenmcnaughton/confirm_btn

[REF] minor code simplification - remove over-handling of amount comp with zero

checkEditInboundEmailsPermissions should be a static function

Merge pull request #13791 from eileenmcnaughton/revert

Attempted test fix

Merge pull request #13781 from eileenmcnaughton/title

Standardise setTitle method on forms

alt to issue-13788 assign db mapping to database_prefix

Standardise setTitle method on forms

We have inconsistent methodology for setting the title. This is a minor simplification and
consistency improvement. It also means hooks can call 'getTitle' - ideally all forms would call thi
s & title handling would be consistent.... one day

Merge pull request #13700 from colemanw/CRM-21008

CRM-21008 - Checkbox to explicitly change employer when sharing address

Attempted test fix

This revert test has started failing frequently. It's possible the reason is performance
Improvements, resulting ineffectively no change in time between the entries.

I's also possible the issue is with the timestamp being used but I think that has
a permitted variance

Merge pull request #13792 from yashodha/reports

CRM-21677 - sybunt report clean up

CRM-21677 - sybunt report clean up

Merge pull request #13790 from yashodha/reports

CRM-21677 - report clean up

CRM-21677 - report clean up

Merge pull request #13774 from mattwire/membership_fataltothrow

Fatal error to exception on Membership BAO

Merge pull request #13786 from eileenmcnaughton/free

dev/core#562 remove more instances of ->free()

Merge pull request #13784 from eileenmcnaughton/confirm_google

Remove reference to google checkout

dev/core#562 remove more instances of ->free()

Per https://lab.civicrm.org/dev/core/issues/562 we have been incrementally removing these as
1) we free resources on DAO->_destruct now and
2) in some edge cases freeing resources in an 'inner loop' can free outer loop
resources - resulting in a hard-to-analyse bug

Remove reference to google checkout

Merge pull request #13782 from eileenmcnaughton/tplwhite

[NFC] template whitespace cleanup

[REF] minor code simplification

This just removes the duplicate assign codes & simplifies the /bin/bash condition

[NFC] template whitespace cleanup

Merge pull request #13780 from yashodha/reports

CRM-21677 - report clean up

CRM-21677 - report clean up

Set version to 5.13.alpha1

Set version to 5.12.beta1

Merge pull request #13777 from seamuslee001/master

Merge forward 5.11 => master

Merge in 5.11

Merge pull request #13594 from agh1/5.11.0-releasenotes

5.11.0 release notes

5.11.0 release notes: late changes

Set version to 5.11.0

Merge pull request #13773 from eileenmcnaughton/5.11-

dev/core#745 Further on_hold fix - it has been suggested the value might be an array like ['']

5.11.0 release notes: bulk of work from Alice

* moving things to misc

* organizing prs

* linking to gitlab issues

* first stab

* more release notes

* more release notes progress

* annotating

* cleaning up

* updating contributers

5.11.0 release notes: added boilerplate

5.11.0 release notes: raw from script

Merge pull request #13770 from eileenmcnaughton/mcrypt

Remove mcrypt system status check

Merge pull request #13766 from eileenmcnaughton/api_between

Fix api bug whereby 0 & '0' are not accepted as range parameters for BETWEEN

Merge pull request #13283 from aydun/token_note_fix

Render Note field tokens correctly - they are already HTML.

Merge pull request #13776 from lcdservices/dev-core-644

dev/core#644 fix from address before calling hook

dev/core#644 fix from address before calling hook

Fatal error to exception on Membership BAO

Merge pull request #13756 from eileenmcnaughton/payment_try2

[REF] separate financial handling & component transitioning in Payment.create

Further on_hold fix - it has been suggested the value might be an array like ['']

This would filter that out

Merge pull request #13754 from eileenmcnaughton/5.11-

dev/core#745 do not filter on 'on_hold' if it is an empty string

Merge pull request #13771 from mattwire/typo

Fix typo in comments

Merge pull request #13769 from eileenmcnaughton/matt

Removes redundant IF

Typo in comments

Merge pull request #13758 from eileenmcnaughton/no_load

Do not load membership contribution as updateMembershipBasedOnCompletionOfContribution loads it within the function

Remove mcrypt system status check

Php 7.2 doesn't support this & it is being deprecated so having this here causes
sites to try to install it - creating a migration problem they wouldn't otherwise have

The situation here is unresolved but this check doesn't solve more confusion than it creates

Removes redundant IF

 self::getRelatedMemberships already filters out empty rows. I left removing this
IF for it's own PR due to the legibility issues in the whitespace change

Merge pull request #13759 from eileenmcnaughton/matt

[REF] towards cleanup of update membership code

Merge pull request #13735 from eileenmcnaughton/query_extract

Extract getSearchSQLParts function

Merge pull request #13746 from eileenmcnaughton/activity_search

Convert activity_date_time field to datepicker and add support for url input

Add enotice handling for weird s....'

Fix related search classes to have the same signature

Add form rule for chronological ordeR

Fix group conversion to add relative key

Old format doesn't seem to have it

Add upgrade routine

Convert activity_date_time field to datepicker and add support for url input

This converts the form to being a metadata defined field & a date-picker field as well
as adding standardised url support for input.

The input format is
activity_date_time_high=20100101

I stripped out the complex and probably broken based on feedback  & not used in core
existing hard-coded url support.

Note that there are some outstanding items
1) the upgrade script item needs to be added - this includes dealing with renaming of the field
since I made it more consistent
2) I stripped out the check to ensure the high date is greater than the low date
- if we still want this we should re-add as a generic thing. My guess is it's a bit optional

Fix api bug whereby 0 & '0' are not accepted as range parameters for BETWEEN

In other news I'm not thrilled about the 0 casting to a string here but that is for another time

Call getSearchSQLParts to get the desired part instead of sql wrangling

Extract getSearchSQLParts function

Merge pull request #13720 from eileenmcnaughton/extract_2

Speed up contribution results by removing join on civicrm_financial_type table when rendering search results.

Merge pull request #13765 from seamuslee001/test_empty_on_hold

Try and add data set example where email_on_hold / on_hold is NULL in…

[REF] towards cleanup of update membership code

This PR limits itself to siphoning off how the memberships are fetched & having that function
- return as an array of arrays rather than an array of objects
- index the array by id rather than membership type

This is a step towards the changes in https://github.com/civicrm/civicrm-core/pull/12542

& it should allow a separation of concerns relating to the LOADING of memberships from the
processing.

I note that the PR I'm looking at changes what gets loaded & that should be it's own PR
but also that there is an extraneous if() here - I didn't alter that because
it is a big white space change & doing on it's own will make that easy to review

Eliminate unnecessary join to civicrm_financial_type table to improve performance

Although this join is indexed it affects index choice, often causing the right index not to
be used and slowing performance. I got subtantial speed improvement by eliminating this join

Add hack for qill

This is a very minor extension of an existing hack but we should de-hack at some point.

Before this the qill is broken on payment instrument & contribution page too

Use generic pseudoconstant declaration methodology to add financial_type to pseudoconstants

Getting away from the hard-hack allows us to have more consistent metadata for generic fields

Merge pull request #13751 from seamuslee001/upgrade_karma

Upgrade Karma version to latest version

Try and add data set eaxmple where email_on_hold / on_hold is NULL in the formValues

Merge pull request #13664 from eileenmcnaughton/activity_perms

Rationalise Activity api ACLs for consistency, to respect the hook & improve performance

Merge pull request #13761 from yashodha/reports

report clean up - remove redundant code

Merge pull request #13726 from avall-llovera/patch-1

dev/core#767 Add 'Cancelled / Refunded Date' and 'Cancellation / Refund Reason'

report clean up - remove redundant code

Merge pull request #13760 from civicrm/5.11

5.11

Merge pull request #13653 from MegaphoneJon/reporting-8

reporting#8 - add thank-you dates to Contribution Summary/Detail reports

Merge pull request #13757 from eileenmcnaughton/payment_extract

[REF] extract getToFinancialAccount from CRM_Contribute_PseudoConstantt::contributionStatus

Merge pull request #13755 from eileenmcnaughton/event_fix

dev/core#766 fix loss of custom fields when creating an event from template

[REF] extract getToFinancialAccount from CRM_Contribute_PseudoConstant::contributionStatus

This makes the function much more readable. I felt that the is_payment logic should go but
resisted & just commented

Do not load membership contribution as updateMembershipBasedOnCompletionOfContribution loads it within the function

Merge pull request #13695 from eileenmcnaughton/extract_activity

[ref] Extract activity payment creation

[REF] separate financial handling & component transitioning in Payment.create

dev/core#766 fix loss of custom fields when creating an event from template

dev/core#745 do not filter on 'on_hold' if it is an empty string

Although I couldn't replicate this apparently in groups created in an earlier version it could
be. We only care about number (0 or 1) or arrays (from the select widget)
so add an extra check