Development issueshttps://lab.civicrm.org/groups/dev/-/issues2023-09-24T22:52:19Zhttps://lab.civicrm.org/dev/core/-/issues/2863Agree / document how tokens should be rendered from outside core2023-09-24T22:52:19ZeileenAgree / document how tokens should be rendered from outside coreIdeally we would have one recommended api way to do this. This would be a way that is usable via api explorer and all the ways we use the api and core code would be converted to do things using that same api method
There is [a documenta...Ideally we would have one recommended api way to do this. This would be a way that is usable via api explorer and all the ways we use the api and core code would be converted to do things using that same api method
There is [a documentation pr here](https://lab.civicrm.org/documentation/docs/dev/-/merge_requests/962) - at the moment I don't think it meets this need.
**Permissions & non-workflow rendering**
There is a question as to whether the api would support non-workflow messaging (in which case the api would likely be more like `Message::render()`. The argument against is that the permissions are more complex. However some notes on that
1) the most common use case is probably when checkPermissions = FALSE in php code.
2) there is an appropriate permission already in core
```
render templates' => [
$prefix . ts('render templates'),
ts('Render open-ended template content. (Additional constraints may apply to autoloaded records and specific notations.)'),
],
```
3) currently the v3 MessageTemplate.send api allows access to the core function - it required a messageTemplateID to be set - but this can probably be faked to allow 'any text'https://lab.civicrm.org/dev/core/-/issues/2848Activity tokens - case id - explain?2023-09-24T22:52:50ZeileenActivity tokens - case id - explain?@ayduns I recently fixed up some activity tokens that weren't working - but in the process @DaveD queried whether 'case_id' makes sense as a token on activity tokens since there could be more than one - just opening this put that to you....@ayduns I recently fixed up some activity tokens that weren't working - but in the process @DaveD queried whether 'case_id' makes sense as a token on activity tokens since there could be more than one - just opening this put that to you....
https://github.com/civicrm/civicrm-core/pull/21489#pullrequestreview-757039309https://lab.civicrm.org/dev/core/-/issues/2846Improve validation of start and end dates for events, contribution pages and ...2023-11-23T07:09:32ZJKingsnorthImprove validation of start and end dates for events, contribution pages and more**Motivation**
There are holes in the validation of start and end dates on contribution pages.
eg:
- Create a new contribution page
- Enter a start date
- Enter an end 'time' but leave the date part empty:
![image](/uploads/e75411e618...**Motivation**
There are holes in the validation of start and end dates on contribution pages.
eg:
- Create a new contribution page
- Enter a start date
- Enter an end 'time' but leave the date part empty:
![image](/uploads/e75411e61813ee5ae55b808f49fa3637/image.png)
- Result is a fatal DB error.
This may seem like an edge-case, but it can also happen when you are 'scrolling' down the page, and accidentally 'scroll' on the time field. This sets a time.
The same is true for event pages event 'start' and 'end' dates.
The same is true for event registration open and close dates:
![image](/uploads/270044386b6b0895da079268d9147e8c/image.png)
The same is true for price fields, with the 'active on' and 'expire on', where setting a time without a date results in a fatal error and the endlessly spinning civi icon of doom.
And for price fields, there is currently no validation to ensure the 'expire on' is before the 'active on'.
Creating campaigns also:
![image](/uploads/c51922aa11942eafa7e58e6d153a5fae/image.png)
**Proposed solution**
I think the validation should really be in the date picker element somewhere, to ensure a date and time are both set - where we expect it.
But I also thought we could take this opportunity to standardise the validation of start and end dates in:
- Event pages (event start and end)
- Event pages (registration open/close)
- Contribution pages (start and end)
- Price fields (active on, expire on)
- Campaign (start, end dates)
I am working on this now.
**After**
![image](/uploads/968b1bafab430a8c7cbc2e5a684e1b01/image.png)
**Follow-up / part 2**
Validation at the API/BAO level?https://lab.civicrm.org/dev/financial/-/issues/186Accounting entries incorrect in a number of cases... especially with pending ...2023-05-01T17:18:33ZJamie Novick - CompucoAccounting entries incorrect in a number of cases... especially with pending refunds and overpaymentsFirstly - sorry for the wall of text. This has ended up being a much longer piece of analysis than expected but I thought I should share it so that we can agree a way ahead.
This all started as we were looking at the best way to impleme...Firstly - sorry for the wall of text. This has ended up being a much longer piece of analysis than expected but I thought I should share it so that we can agree a way ahead.
This all started as we were looking at the best way to implement refunds for a payment processor. Some work was done here to discuss this: https://lab.civicrm.org/dev/financial/-/issues/87 but I can see this stalled somewhat - probably due to some of the issues I'm going to discuss here.
# Background: The problems with the contribution status field
Just for complete disclosure I hate the CiviCRM contribution status field. Hopefully everyone already knows that. I've probably started discussions on getting rid of it at every CiviCON and documented this on every financial gitlab ticket. It is the biggest blocker we have to reaching an accounting implementation that aligns to standard concepts of accounting.
Why is that?
We appear to have 2 use cases for it, both of which are undocumented.
1. For users to quickly change whether the amounts owed are
a) expected to be paid or
b) are paid.
2. To reflect the payment status of the contribution
i.e. whether the total sum of the line items is >, < or = to the net amount paid in or refunded on that contribution (this is important and I’ll come back to it)
This is a bit of a problem as it’s all a bit chicken and egg! If the user changes the status we need to understand whether we should be creating payments or not (or creating refund payments or not), and if the user records payments or refund payments we need to update the status to something meaningful and consistent.
The following is an analysis of the allowed contribution status changes from and to that a user can perform:
**Table 1:**
- Black = Option hidden
- X - Val = Option shown but user cannot change to it as we have a validation message
- Y (black) = Option shown, user can select this and I don’t see issues with accounting that occurs
- N/A = Couldn’t test
- Y (red) = Option shown, user can select this and there are issues with accounting that occurs
![Screenshot_2021-09-15_at_15.33.12](/uploads/8db825597f091f0bf34d590b38b2ed2f/Screenshot_2021-09-15_at_15.33.12.png)
I’d note that it’s a bit confusing to users to see a value in the dropdown they can’t use. I think we should just hide values they cannot select as this would be much clearer for them.
We end up with some very strange and in some cases incorrect behaviour when allowing users to change the status in a "partially paid" and "pending refund" scenarios:
**Table 2:**
![Screenshot_2021-09-15_at_15.47.11](/uploads/827aafd531d7dbe54866b8237758ce42/Screenshot_2021-09-15_at_15.47.11.png)
# Ground Rules
As such I would like to suggest some ground rules to work towards:
1. The only time a user can change the status of a contribution should be when it has no payments or refunds attached to it.
2. Once a payment has been received we manage everything through either the line item editor or the “change selections” on the event form or "record payment” or “record refund”. If any changes to each are made to any of these we recalculate the contribution status based on one set of rules:
The rules to calculate the contribution status should be:
**Table 3:**
![Screenshot_2021-09-15_at_15.38.43](/uploads/b77e518446862474f4e6823e045e11a3/Screenshot_2021-09-15_at_15.38.43.png)
Delving deeper, there are a number of situations within CiviCRM that do not confirm to this currently and instead we end up with a status this is confusing for the user:
(Note this is not be a comprehensive list - just to give some examples).
**Table 4:**
![Screenshot_2021-09-15_at_15.39.45](/uploads/3a9e238f9ec1071de6b056c543c68522/Screenshot_2021-09-15_at_15.39.45.png)
# Specific Scenarios
This all said there are some specific use cases that we need to consider and have appropriate workflows for:
Table 5:![Screenshot_2021-09-15_at_15.40.49](/uploads/a922e2c0f5a52b92adba895c5fd89654/Screenshot_2021-09-15_at_15.40.49.png)
# Actions:
As such I think the actions could be:
1. Agree to the "ground rules" for the contribution status field above so we are all working towards a common goal.
1. I think we should hide values they cannot select as this would be much clearer for them. i.e. all items that are marked as “X-val” in the table above. Perhaps we can refactor the code that show/hides the options to a central place.
1. I think we should change the way the status field is calculated to use the business logic suggested in Table 3 above in all cases. This shouldn’t be something that extensions should take care of but should be calculated any time a contribution haas it’s line items / financial transactions changed. Obviously I don’t know what this means from a code perspective and I assume this would be a significant refactor but I think we could clear up a lot of business logic by doing so.
1. Discuss and agree which of the options from table 5.1 are suitable and then agree the UX for this.
1. So that we can then remove the option to make the status changes as per table 1: 3f, 4e, 4f without degrading users ability to do the things they need.
I actually think if we do these things we can consider CiviCRM’s accounting to be “correct” if not completely intuitive (at least for now).
@mattwire @eileen @KarinG @JoeMurrayhttps://lab.civicrm.org/dev/financial/-/issues/185Contributions created as pending can be later completed in test mode, behavin...2021-09-15T13:25:34ZFrancis (Agileware)Contributions created as pending can be later completed in test mode, behaving like live contributionsContributions created as pending can be later completed in test mode using the `civicrm/contribute/transact` path, behaving like live contributions.
e.g. linked Memberships are moved from Pending to New
1. As an admin, create a Contrib...Contributions created as pending can be later completed in test mode using the `civicrm/contribute/transact` path, behaving like live contributions.
e.g. linked Memberships are moved from Pending to New
1. As an admin, create a Contribution Page that performs "real-time transactions" allows the end user to leave a Contribution in Pending state (either pay later or with a payment processor that defers payment with "incomplete transaction"). For complete coverage, include membership sign up configuration.
2. As a normal user, visit the Contribution Page in live mode and submit details, but do not complete the transaction
3. As the same user, reload the Contribution Page in test mode with the relevant contribution id as URL parameter
4. Complete the transaction using test credit card details
5. As an admin, observe that the contribution has been updated as though it were completed in live mode including relevant changes to any linked entities.
Suggest that this page should probably refuse access in test mode if it attempts to load an existing contribution that is not marked `is_test`https://lab.civicrm.org/dev/core/-/issues/2811Menu Angular error after upgrade2023-01-08T01:26:08ZStoobMenu Angular error after upgradeI do a fair amount of upgrades, and anecdotally after about a _half_ of recent upgrades, the CiviCRM menu fails to load. For half of those, a simple reload/refresh of the browser resolves the issue. For the other half, the civicrm/menu...I do a fair amount of upgrades, and anecdotally after about a _half_ of recent upgrades, the CiviCRM menu fails to load. For half of those, a simple reload/refresh of the browser resolves the issue. For the other half, the civicrm/menu/rebuild cache (or drush cc) must be cleared in order for the menu to appear. It is usually an Angular issue, seen attached.
Two questions, if these steps are required with such frequency can the upgrade process be improved by either:
1. clearing the cache automatically when the upgrade is finished
2. providing instructions and/or link on the upgrade screen to clear cache
![menu-load](/uploads/70b9bc8bcfbae067b5169d330bd16545/menu-load.png)
![upg](/uploads/19aa40afb6bda323d97cafe3c3cec5b3/upg.png)https://lab.civicrm.org/dev/core/-/issues/2796Regression? No way to change the tax rate and tax amount using buildamount hook2023-11-24T22:45:42ZPradeep Nayakpradpnayak@gmail.comRegression? No way to change the tax rate and tax amount using buildamount hookIn 5.35.x LineItem create api allowed one to set a different tax_amount in api params for a financial type that had tax enabled. The tax amount could then be calculated programatically based on region, location etc instead of using the c...In 5.35.x LineItem create api allowed one to set a different tax_amount in api params for a financial type that had tax enabled. The tax amount could then be calculated programatically based on region, location etc instead of using the config specified in the UI.
```
civicrm_api3('LineItem', 'create', [
'entity_id' => 284,
'qty' => 1,
'unit_price' => 100,
'line_total' => 100,
'entity_table' => "civicrm_contribution",
'tax_amount' => 20,
'financial_type_id' => "Donation",
'price_field_id' => "contribution_amount",
'price_field_value_id' => "single",
]);
```
Another way of calculating tax on online forms was using the [buildamount hook](https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_buildAmount/)
```
function advanceinvoicing_civicrm_buildAmount($pageType, &$form, &$amount) {
$amount[$priceFieldId]['options'][$priceFieldValueId]['tax_rate'] = 10;
$amount[$priceFieldId]['options'][$priceFieldValueId]['tax_amount'] = 0.1;
}
```
A recent [change](https://github.com/civicrm/civicrm-core/commit/e967ce8fe2b#diff-a16d4d7449cf5f3a0616d1d282a32f27ab6d3f7d2726d076c02ad1d4d655af41R56) to the line item create function has stopped allowing tax amount to be set programmatically.
Is this intentional? Is there any way to support different tax rate for single financial type?https://lab.civicrm.org/dev/core/-/issues/2795PHP8: Use of error-suppression operator (@) behaves differently in error_hand...2023-12-12T19:51:20ZDaveDPHP8: Use of error-suppression operator (@) behaves differently in error_handler functions in php8Parking what's left in https://github.com/civicrm/civicrm-core/pull/21064 here for now since I've cleared out a bunch and am taking a break from it. The gist is that in php8 the error suppression operator (e.g. `@fopen('/nonexistent/file...Parking what's left in https://github.com/civicrm/civicrm-core/pull/21064 here for now since I've cleared out a bunch and am taking a break from it. The gist is that in php8 the error suppression operator (e.g. `@fopen('/nonexistent/file.txt', 'r')` works differently with respect to error_handler functions. In a stock install as of right now you won't notice any difference, just there will be more errors depending on what 3rd party/custom error_handlers you're using and may behave differently on different CMSs in future depending on how their error-handling may change.
### smarty default modifier
The smarty modifier `|default` is a tricky one because its whole purpose is to deal with the situation where the var might be missing, but the way smarty compiles it is something like `<?php $this->assign('to', ((is_array($_tmp=@$this->_tpl_vars['to'])) ? $this->_run_mod_handler('default', true, $_tmp, '_high') : smarty_modifier_default($_tmp, '_high'))); ?>` which uses the @ to try to silence the error. In addition to [templates/CRM/Core/DatePickerRange.tpl and templates/CRM/Core/DatePickerRangeWrapper.tpl](https://github.com/civicrm/civicrm-core/commit/f671f500b658aa772fedc682dbb320cf456bc904), the below are all instances of this. They could all be rewritten to not use the modifier, but the modifier itself is part of smarty so can't easily be prevented from being used in future.
* CRM_Core_InvokeTest::testInvokeDashboardForNonAdmin
* CRM/Contact/Page/DashboardDashlet.tpl - $communityMessages
* CRM_Core_SessionTest::testSetStatusWithTextOnly
* CRM/common/info.tpl
* CRM_Core_SessionTest::testSetStatusWithTitleOnly
* CRM/common/info.tpl
* CRM_Core_SessionTest::testSetStatusWithBoth
* CRM/common/info.tpl
### opendir
```
CRM_Utils_File::cleanDir
if ($dh = @opendir($target)) {
```
### unlink and mkdir
Various calls to `@unlink` and `@mkdir`
### httpclient
```
CRM_Utils_HttpClientTest::testFetchHttp_badOutFile
Exception: fopen(/ba/d/path/too/utput): Failed to open stream: No such file or directory
...\tests\phpunit\CiviTest\CiviUnitTestCase.php:253
...\CRM\Utils\HttpClient.php:79 <---- see here
...\tests\phpunit\CRM\Utils\HttpClientTest.php:77
...\tests\phpunit\CiviTest\CiviUnitTestCase.php:267
...\phpunit8:671
```
### angular changeset
Aside: The code in question here suggests phpQuery has its own debug setting which maybe could be turned on automatically in future if you have civi debugging enabled:
`$return = phpQuery::$debug === 2 ? this->document->loadHTML($markup) : @$this->document->loadHTML($markup);`
```
Civi\Angular\ChangeSetTest::testInsertAfter
Exception: DOMDocument::loadHTML(): htmlParseEntityRef: no name in Entity, line: 2
...\tests\phpunit\CiviTest\CiviUnitTestCase.php:253
...\vendor\electrolinux\phpquery\phpQuery\phpQuery\DOMDocumentWrapper.php:200
...\vendor\electrolinux\phpquery\phpQuery\phpQuery\DOMDocumentWrapper.php:542
...\vendor\electrolinux\phpquery\phpQuery\phpQuery\DOMDocumentWrapper.php:505
...\vendor\electrolinux\phpquery\phpQuery\phpQuery\DOMDocumentWrapper.php:469
...\vendor\electrolinux\phpquery\phpQuery\phpQuery\phpQueryObject.php:2066
...\vendor\electrolinux\phpquery\phpQuery\phpQuery\phpQueryObject.php:2010
...\tests\phpunit\Civi\Angular\ChangeSetTest.php:28
...\Civi\Angular\ChangeSet.php:59
...\Civi\Angular\ChangeSet.php:19
...\tests\phpunit\Civi\Angular\ChangeSetTest.php:39
...\tests\phpunit\CiviTest\CiviUnitTestCase.php:267
...\phpunit8:671
```
* not ok 103 - Error: Civi\Angular\PartialSyntaxTest::testConsistencyExamples with data set 6 `('<div ng-if="a && b"></div>', '<div ng-if="a && b"></div>')`
* not ok 104 - Error: Civi\Angular\PartialSyntaxTest::testAllPartials
* These are the same issue as the changeset tests.
### testing of deprecations
These deliberately test deprecations
* not ok 223 - Error: Civi\Payment\PropertyBagTest::testSetContactIDLegacyWay
* not ok 227 - Error: Civi\Payment\PropertyBagTest::testSetCustomProp
### testGetfieldsHasTitle
```
api_v3_SyntaxConformanceTest::testGetfieldsHasTitle with data set 32 ('Entity')
Failure in api call for Entity getactions: include_once(api/v3/batch_create.php): failed to open stream: No such file or directory
0 ...\api\v3\Entity.php(18): CiviUnitTestCase->letssee(2, 'include_once(ap...', '\path\to\...', 18, Array)
1 ...\api\v3\Entity.php(18): include_once()
2 ...\api\v3\utils.php(2461): _civicrm_api3_entity_deprecation(Array)
3 ...\api\v3\utils.php(251): _civicrm_api3_deprecation_check('Entity', Array)
4 ...\Civi\API\Provider\ReflectionProvider.php(108): civicrm_api3_create_success(Array, Array, 'Entity', 'getactions')
5 ...\Civi\API\Kernel.php(149): Civi\API\Provider\ReflectionProvider->invoke(Array)
6 ...\Civi\API\Kernel.php(81): Civi\API\Kernel->runRequest(Array)
7 ...\api\api.php(22): Civi\API\Kernel->runSafe('Entity', 'getactions', Array)
8 ...\Civi\Test\Api3TestTrait.php(292): civicrm_api('Entity', 'getactions', Array)
9 ...\Civi\Test\Api3TestTrait.php(173): CiviUnitTestCase->civicrm_api('Entity', 'getactions', Array)
10 ...\tests\phpunit\api\v3\SyntaxConformanceTest.php(1662): CiviUnitTestCase->callAPISuccess('Entity', 'getactions', Array)
11 phar://.../phpunit8/phpunit/Framework/TestCase.php(1213): api_v3_SyntaxConformanceTest->testGetfieldsHasTitle('Entity')
12 ...\tests\phpunit\CiviTest\CiviUnitTestCase.php(267): PHPUnit\Framework\TestCase->runTest()
13 phar://.../phpunit8/phpunit/Framework/TestCase.php(889): CiviUnitTestCase->runTest()
14 phar://.../phpunit8/phpunit/Framework/TestResult.php(577): PHPUnit\Framework\TestCase->runBare()
15 phar://.../phpunit8/phpunit/Framework/TestCase.php(663): PHPUnit\Framework\TestResult->run(Object(api_v3_SyntaxConformanceTest))
16 phar://.../phpunit8/phpunit/Framework/TestSuite.php(481): PHPUnit\Framework\TestCase->run(Object(PHPUnit\Framework\TestResult))
17 phar://.../phpunit8/phpunit/Framework/TestSuite.php(481): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
18 phar://.../phpunit8/phpunit/TextUI/TestRunner.php(462): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
19 phar://.../phpunit8/phpunit/TextUI/Command.php(129): PHPUnit\TextUI\TestRunner->doRun(Object(PHPUnit\Framework\TestSuite), Array, Array, true)
20 phar://.../phpunit8/phpunit/TextUI/Command.php(101): PHPUnit\TextUI\Command->run(Array, true)
21 ...\phpunit8(671): PHPUnit\TextUI\Command::main()
22 {main}
Failed asserting that a integer is empty.
...\Civi\Test\Api3TestTrait.php:110
...\Civi\Test\Api3TestTrait.php:174
...\tests\phpunit\api\v3\SyntaxConformanceTest.php:1662
...\tests\phpunit\CiviTest\CiviUnitTestCase.php:267
...\phpunit8:671
```
### mailing testConcurrency
This doesn't run properly on windows even without the error_handler, and I don't feel like taking the time right now to deal with that:
not ok 1161 - Error: api_v3_JobProcessMailingTest::testConcurrency with data set #0 (array(20, 3, 10, 4, 1), array(2, 1), 4)https://lab.civicrm.org/dev/core/-/issues/2788Add tokenlist api2023-09-24T22:53:09ZeileenAdd tokenlist apiWe need an api to call via ajax to determine the available tokens for a given entity. An example is the Scheduled reminders UI which currently presents the tokens for all entities (available or not) but would ideally look up the tokens v...We need an api to call via ajax to determine the available tokens for a given entity. An example is the Scheduled reminders UI which currently presents the tokens for all entities (available or not) but would ideally look up the tokens via ajax once the entity is selected.
I think the hardest part of doing this is agreeing how it should look - eg. entity, action, parameters - eg
```
Civi\Api4\Tokens::list()
->setSchema(['contributionId', 'contactId')
->setWhere('contact_type', '=', 'Organization')
->execute();
```
Possible entity names
- Token
- Tokens
- TokenProcessor
- EntityTokens
Possible actions
-list
- getTokens
- listTokens
Schema - maybe this one is clear cut
And - how do we pass relatively free form tokens such as activity_type_id, contact_type or (possibly in the future) saved_search_id where search kit plays a rolehttps://lab.civicrm.org/dev/financial/-/issues/182Possible regression - civicrm_line_item.line_total is incorrectly tax inclusi...2023-11-23T07:17:29ZeileenPossible regression - civicrm_line_item.line_total is incorrectly tax inclusive - potentially 5.39+In 5.39 there was a fix to stop the situation where the contribution.total_amount was recalculated on edit when tax was involved. However this fix had a bug and civicrm_line_item.line_total [stored the tax INCLUSIVE amount](see https://...In 5.39 there was a fix to stop the situation where the contribution.total_amount was recalculated on edit when tax was involved. However this fix had a bug and civicrm_line_item.line_total [stored the tax INCLUSIVE amount](see https://github.com/civicrm/civicrm-core/commit/e967ce8fe2b58b94e2163dde395542e55599da13#diff-4c9d0b1abe07057a4eea2b47bc627eecb95face8ed8d86c1c005312a52cca811L4026-L4035) We [have merged a fix to the rc](https://github.com/civicrm/civicrm-core/pull/21212) but I opened this to try to determine if any released versions are affected. This rose to a bigger issue in the rc because of changes on top of it - which assumed it was right.
The reason this went awol is largely because we had too behaviours - if the contribution bao didn't receive tax_amount it treated total_amount as exclusive and if it did it treated it as inclusive. The former was locked in by tests & not the latter leading to confusion (I'm still confused)
At this stage I'm unsure about real-world issues with this as
1) it didn't affect any core forms that I'm aware of as they pass in 'line_item' or 'skipLineItem'
2) this was the 'correct' behaviour historically for Contribution.create api calls that should have tax_amount passed in, but didn't
3) we have a [deprecation notice](https://github.com/civicrm/civicrm-core/commit/e967ce8fe2b58b94e2163dde395542e55599da13#diff-4c9d0b1abe07057a4eea2b47bc627eecb95face8ed8d86c1c005312a52cca811R193) in the code that would likely have been triggered if line item tax totals did not add up to contribution totals & it's unlikely people would miss wrong contribution totals.
However, my head breaks a bit when I try to think through this
I *think* this would pick up any wrong rows
```
SELECT
SUM(li.line_total + li.tax_amount) as lines_total,
count(li.id) as c,
SUM(c.total_amount) as total_amount
FROM civicrm_contribution c LEFT JOIN civicrm_line_item li ON li.contribution_id = c.id
GROUP BY c.id
HAVING c > 1
AND lines_total <> total_amount
```https://lab.civicrm.org/dev/core/-/issues/2775Scheduled reminders fail silently if there is an error in the template2023-03-27T16:26:57ZwmortadaScheduled reminders fail silently if there is an error in the templateOverview
----------------------------------------
We have recently come across an issue whereby scheduled reminders weren't being sent but we were completely unaware this was happening. We managed to track it down to an error in the temp...Overview
----------------------------------------
We have recently come across an issue whereby scheduled reminders weren't being sent but we were completely unaware this was happening. We managed to track it down to an error in the template for one of the event reminders we had set up - the template contained CSS and this caused a fatal error in Smarty.
```
PHP Fatal error: Smarty error: [in string:<meta content="text/html; charset=utf-8" http-equiv="Content-Type" />
<title></title>
<style type="text/css">html { font-size: 100%;}
html, button, input, select, textarea { font-family: sans-serif; }
body, h1, h2, h3, h4, h5, h6, p { margin: 0; }
table { border-collapse: collapse; border-spacing: 0; }
td img { display: block; }
...
```
The issue is that this stopped _all_ scheduled reminders from being sent - not just the event reminder. This meant that none of our membership renewal reminder emails were being sent either. We were completely unaware of this because there was no visible error message in the UI.
Reproduction steps
----------------------------------------
1. Create a scheduled reminder that works correctly.
1. Check that this is sending emails as expected.
1. Create another scheduled reminder with CSS in the template (see above) so that it causes a Smarty error.
1. Note that none of the scheduled reminder emails are being sent.
Current behaviour
----------------------------------------
CiviCRM fails silently when there is an error in a template for a scheduled reminder. This stops all scheduled reminders from being sent.
Expected behaviour
----------------------------------------
There are several things that should happen if there is an error in a Smarty template:
1. CiviCRM alerts the user at the point of creating/editing a scheduled reminder with a Smarty error in the template.
1. CiviCRM alerts the user if there is scheduled reminders fail to be sent - see #1933.
1. CiviCRM should continue to send the other scheduled reminders as normal so only the reminder with an error fails, rather than no reminders being sent at all.
Environment information
----------------------------------------
* __Browser:__ N/A
* __CiviCRM:__ _5.33.5_
* __PHP:__ _7.3_
* __CMS:__ _WordPress 5.4.6_
* __Database:__ _Maria DB 10.3.23._
* __Web Server:__ _Nginx 1.15.0_
Comments
-----------------------------------------
See this post on StackExchange for info about the Smarty error: [SMARTY error with template](https://civicrm.stackexchange.com/questions/31866/smarty-error-with-template).https://lab.civicrm.org/dev/user-interface/-/issues/42Proposal to re-design the CiviCRM Mailing, Unsubscribe form to be more in-lin...2021-09-09T13:51:47Zjustinfreeman (Agileware)Proposal to re-design the CiviCRM Mailing, Unsubscribe form to be more in-line with what users experience on other mailing / newsletter systemsThis request is to initiate an discussion on the re-design of the CiviCRM Mailing, Unsubscribe form so that it is more in-line with what users experience with other mailing / newsletter systems.
If you are subscribed to a CiviCRM mailin...This request is to initiate an discussion on the re-design of the CiviCRM Mailing, Unsubscribe form so that it is more in-line with what users experience with other mailing / newsletter systems.
If you are subscribed to a CiviCRM mailing list and click the Unsubscribe link, the user experience flow is:
1. The Mailing Group Title and Group Description are shown, or if set, the Mailing Public Title and Public Description are shown
2. You are directed to a page where a masked email address is shown.
3. You need to enter the subscribed email address.
4. Click Unsubscribe.
5. The system then confirms the unsubscription, but in a way that also is not clear as it re-loads the unsubscribe form again.
The requirement to enter the subscribed email address is problematic for a number of reasons:
1. It is masked, so if the user cannot tell exactly what it was sent too. In some cases, the user may not know what address it was when they click on the link - perhaps the email is forwarded internally from a mailbox that has since been archived or the mailbox is unmonitored, team mailbox etc.
2. Re-entering the email address is another action the user has to take, if they want to unsubscribe and have clicked the link then that's all the confirmation that should be required at this point.
The improvement I would really like to see is clearer messaging and no user input, quicker process. For example:
1. Click unsubscribe.
2. CiviCRM Unsubscribe page opens, confirming the unsubscribe has been done. That's it.
3. Optionally, a feedback form can shown for a quick survey or reason for unsubscribing (good use of a Profile). User may just close the window at this step.
You could include a link on the Unsubcribe form to say, "Click here to re-subscribe, if you made a mistake" or something similar. I've seen this on a few other CRM Unsubscribe forms.
What do people think? Do we at least agree the current user experience needs to be improved? Ping @bgm @artfulrobot @mattwire - you guys seem to care about user experience too :smile:
# Screenshots of the current user experience in CiviCRM, Unsubscribe page
(Copied from Matt's [recent PR](https://github.com/civicrm/civicrm-core/pull/21174))
![129920549-c3ee348d-a5a0-4ac8-8b00-e97adfb45331](/uploads/02b3c3abd8628b691e4163baad267118/129920549-c3ee348d-a5a0-4ac8-8b00-e97adfb45331.png)
![129920610-db419978-678e-4fa2-8cb2-3d1cf58842c1](/uploads/43c96495d009fa8b4fffb492c75ae773/129920610-db419978-678e-4fa2-8cb2-3d1cf58842c1.png)
# By comparison, here's what other mailing systems have on their Unsubscribe page
## Mailchimp
![Screenshot_20210820_173624](/uploads/60abb50e4a4e288f6797867ac986140c/Screenshot_20210820_173624.png)
## Another CRM
![Screenshot_20210820_173613](/uploads/e1635e316e86e7a150e1cda09443fee3/Screenshot_20210820_173613.png)
Agileware Ref: CIVIBLD-271https://lab.civicrm.org/dev/financial/-/issues/181Update billing details fails when no State/Province entered2022-09-16T21:08:32ZlarsssandergreenUpdate billing details fails when no State/Province enteredUpdate billing details gives "Page could not be loaded" on submit if the user has not entered a State/Province. In some cases, it is valid for a user to not enter a State/Province if they live in a country that does not have states. This...Update billing details gives "Page could not be loaded" on submit if the user has not entered a State/Province. In some cases, it is valid for a user to not enter a State/Province if they live in a country that does not have states. This works as expected on contribution pages, etc, but not on the update billing details page. Tested on 5.35.2.
I will have a look at how this is handled for contribution pages and port the solution to update billing details, at some point.https://lab.civicrm.org/dev/core/-/issues/2761Proposal: Change Pay Later Billing Address to look up a profile rather than a...2022-10-04T21:07:14ZBarijohnProposal: Change Pay Later Billing Address to look up a profile rather than a coded block.Overview
----------------------------------------
Currently on the pay later option when you use the option of Billing Address the fields are hard coded. This would be better to use a profile then it could be edited by the end user witho...Overview
----------------------------------------
Currently on the pay later option when you use the option of Billing Address the fields are hard coded. This would be better to use a profile then it could be edited by the end user without having to mess with the code.
Example use-case
----------------------------------------
Click on Pay Later Option and enable Billing Fields. This shows multiple fields that might not be required depending on the part of the world. So in that case there could be a reserved profile just for billing fields. This could have a help field to show where this profile is saved, so it would easily amended.
Current behaviour
----------------------------------------
Currently this is set in Core/Payment.php and /civicrm/templates/CRM/Core/BillingBlock.tpl (based on communications with our devs).
Proposed behaviour
----------------------------------------
Make this block look up a billing address profile instead.
Comments
----------------------------------------
Currently this field asks for information that isn't required in the UK and most of our clients would like to be able to edit this directly.https://lab.civicrm.org/dev/core/-/issues/2759Intermittent cookies error after contribution2023-03-14T07:16:59ZandyburnsIntermittent cookies error after contributionWe experience a cookies error intermittently after submitting a donation. Currently on Civi 5.35.2 and WP 5.7.2. This is multisite so may be multisite related.
![118401618_379155863073198_2489825481780434605_n](/uploads/1125c66666dd23c3...We experience a cookies error intermittently after submitting a donation. Currently on Civi 5.35.2 and WP 5.7.2. This is multisite so may be multisite related.
![118401618_379155863073198_2489825481780434605_n](/uploads/1125c66666dd23c31102ac79e053f06f/118401618_379155863073198_2489825481780434605_n.png)
A contributor does not get to the thank you page but gets the error above. The donation does succeed. This issue has persisted for a long-time (2+ years) and is a probably the worst civi issue I've encountered seeing it has to do with contribution pages and has no pattern that we can find. Tried all the go to solutions posted on SE with no luck. I would estimate that it occurs ~10-20% of the time.
This is related: https://lab.civicrm.org/dev/core/-/issues/517 and says using PHP 7.2 resolved, but were on 7.2 and now on 7.3
Some steps I've done:
- Set `Clean-up Temporary Data and Files` scheduled job run frequency from Hourly to Daily. It is on every site.
- Changed cookie domain constant in wp-config.php
```
// define('COOKIE_DOMAIN', false);
to
define('COOKIE_DOMAIN', $_SERVER\['HTTP_HOST'\] );
```
- Enabled https://lab.civicrm.org/extensions/qfsessionwarning and set timeout at 120 minutes.
`$civicrm_setting['core']['secure_cache_timeout_minutes'] = 120;`
We have considered changing how garbage collection works as sessions will die if garbage collection is run (server does that every 15 minutes).
[This post](https://civicrm.stackexchange.com/questions/39156/payments-fail-with-paypal-standard-and-could-not-find-a-valid-session-key) seems to have found a solution using the Fatal Error Handler. Is this recommended and how do you implement? Seems like a workaround symptom fix 'solution'.
Also getting this for additional event participants all the time (usually) or not at all depending on when I test: https://lab.civicrm.org/dev/core/-/issues/2708
Help?https://lab.civicrm.org/dev/user-interface/-/issues/40Menu: have a quick way to access "My Contact"2021-08-30T16:07:35ZbgmMenu: have a quick way to access "My Contact"Bits from #38:
> J: If you want to change or set up your signature, you have to find your CiviCRM Contact, by searching for it - and of course, make sure that it's the right one not a duplicate.
> M: Maybe we could add a "My Contact Re...Bits from #38:
> J: If you want to change or set up your signature, you have to find your CiviCRM Contact, by searching for it - and of course, make sure that it's the right one not a duplicate.
> M: Maybe we could add a "My Contact Record" link from the CiviCRM logo menu item?
> J: It's not an obvious position to put it, you really want something that's clearly labelled and visible without having to click on a icon to see it. The icon itself doesn't indicate at all what is there - I didn't even know this was a menu (no jokes). I would suggest adding a "My Preferences" or a "My Account" top-level menu item between the Reports and Administer menus.
Random thoughts:
* For now, we do not have the concept of an account or preferences in CiviCRM, we only have contact records, so I think that it should be called something like "My contact".
* The CiviCRM menu is overwhelmingly huge (for admins), and despite what we recommend them, many admins tend to add custom reports on the top-level menu. I would be reluctant to add more items that take a lot of space.
* (tangent: I suspect it's because the default "Report" menu is not useful to them, so they want something more obvious, which then creates a mess; I have a few ideas for that, but it's another tangent, see also dev/report#74).
* The CiviCRM logo menu item is not obvious at all to a lot of people. It does have a logout link, which makes it a good candidate I think. I would have been tempted to change the logo to a user icon, but also has other things. Maybe need a better hint that it is a menu item?
* (other tangent: I think we should remove the "Hide menu" option from there, since we also have a toggle at the end, and it makes little sense to users who do not have access to the CMS admin menu.)
* It could be a menu item at the end of the "Contacts" menu? It could be a less-disruptive first step towards the concept.
Other ideas?
Random ping of UX folks with different point of views: @justinfreeman @nicol @artfulrobot @andrewhunt @roshani @jamie @jamienovickhttps://lab.civicrm.org/dev/user-interface/-/issues/39Email Signature: changing the "from" email does not update the signature2021-08-17T03:40:42ZbgmEmail Signature: changing the "from" email does not update the signatureFrom #38:
> The signature doesn't change when you change the FROM address on the email form.
To reproduce:
* Add a second email to your contact record
* Add different signatures to both emails
* Click to send an email (it can be to se...From #38:
> The signature doesn't change when you change the FROM address on the email form.
To reproduce:
* Add a second email to your contact record
* Add different signatures to both emails
* Click to send an email (it can be to self, does not matter)
* Primary email signature is loaded
* Change the "from" email to the secondary email (of our logged-in user)
* Notice that the signature did not change.
Tangential:
* Loading a message template removes the email signature. Kind of annoying.
* Switching from/signatures could assume that we are reloading the content, but should probably warn that the content is about to be lost.
cc @DaveD @justinfreemanhttps://lab.civicrm.org/dev/financial/-/issues/179Search kit bugs2021-08-04T22:27:21ZeileenSearch kit bugs@colemanw I hit a couple of things writing up about [how to do a payments search](https://docs.google.com/document/d/1OM8T-HsqFQDVGMB83iXx6CkC4mtRh4dA0hNK0MIX4hc/edit#)
I figured I'd just stick them in one ticket for now
1) The amount...@colemanw I hit a couple of things writing up about [how to do a payments search](https://docs.google.com/document/d/1OM8T-HsqFQDVGMB83iXx6CkC4mtRh4dA0hNK0MIX4hc/edit#)
I figured I'd just stick them in one ticket for now
1) The amount column from the financial_trxn table was not available to add as an afform filter
![image](/uploads/3608db6fc7f5c8289b4f276d40396927/image.png)
2) It 'looked' like I could make check_number 'edit-in-placeable' on the financial_trxn table - but it didn't work - note that check_number IS a field that should be editable as a data entry issue with no financial transaction implications (if we were editing payment method I think we would to be sure that the edit-in-place is calling v3 Payment.create not v4 FinancialTrxn.create - the former is a wrapper for the latter with more logic)
![image](/uploads/67ed36272f37bbc2fe9bdeacbe199c5d/image.png)
![image](/uploads/58fab89fd8ca820dfb2f17a0a4ffa726/image.png)https://lab.civicrm.org/dev/core/-/issues/2693Remove all calls to BAO_Contribution::completeOrder other than from Payment.c...2023-09-10T15:41:47ZeileenRemove all calls to BAO_Contribution::completeOrder other than from Payment.createThe goal is that we should either
1) call Payment.create to add a payment - which would itself call completeOrder
2) OR call repeatTransaction via the repeattransaction api to create a pending order - which we would then update to cance...The goal is that we should either
1) call Payment.create to add a payment - which would itself call completeOrder
2) OR call repeatTransaction via the repeattransaction api to create a pending order - which we would then update to cancelled or failed if need be or - if completed call payment.create
Note that currently Payment.create calls Contribution.completetransaction - the goal is to reverse this
I'm currently working on removing calls to completeOrder other than from the 2 api calls - at which point we can change the api calls to no longer call completeOrder directlyhttps://lab.civicrm.org/dev/financial/-/issues/177Batch Transactions: Search cannot filter by Soft-Credit2023-07-21T17:13:51ZmhowisonBatch Transactions: Search cannot filter by Soft-CreditI'm trying to create a batch of existing contacts who have all made a contribution via Donor-Advised Fund (DAF).
When I use the Find Contributions search feature, I'm able to produce a list of contacts who gave through Donor-Advised Fun...I'm trying to create a batch of existing contacts who have all made a contribution via Donor-Advised Fund (DAF).
When I use the Find Contributions search feature, I'm able to produce a list of contacts who gave through Donor-Advised Funds in FY21 (Date Received=Previous Fiscal Year, Contributions AND Their Soft Credits, Soft Credit Type=Donor-Advised Fund).
However, when I run this same query through the search feature in the 'add existing transactions to a batch' screen, the results are incorrect. It filters for previous fiscal year contributions, but it returns all contributions rather than just contributions with a soft credit type of DAF.
The Soft Credit Type criteria doesn't seem to be registering. This shows you the 'add existing transactions to a batch' screen I'm referring to: (https://docs.civicrm.org/user/en/latest/contributions/accounting-integration/#create-a-new-batch-from-existing-transactions.)