Development issueshttps://lab.civicrm.org/groups/dev/-/issues2023-10-30T18:30:20Zhttps://lab.civicrm.org/dev/drupal/-/issues/189Epic: Drupal 10 finalization2023-10-30T18:30:20ZtottenEpic: Drupal 10 finalizationThis is a follow-up to #176. Where #176 did the main work of achieving compatibility, this issue is for the final wrap-up to handle Drupal 10 as a publicly visible option.
* [x] Add build types for test/demo infra
* [x] Add public sandb...This is a follow-up to #176. Where #176 did the main work of achieving compatibility, this issue is for the final wrap-up to handle Drupal 10 as a publicly visible option.
* [x] Add build types for test/demo infra
* [x] Add public sandbox on https://civicrm.org/demo (https://d10-master.demo.civicrm.org/)
* [ ] Add to test matrices
* [x] CiviCRM-E2E-Matrix (master)
* [ ] CiviCRM-E2E-Matrix (stable/rc)
* [ ] Fix distinctive test failures on D10
* [x] Concurrent mail tests (https://github.com/civicrm/civicrm-core/pull/27072)
* [ ] Authx tests (*these may be non-blockers since D9 has similar issues -- but we should look again*)
* [ ] Add to `Installation Guide` ([Requirements](https://docs.civicrm.org/installation/en/latest/general/requirements/), [Install on Drupal](https://docs.civicrm.org/installation/en/latest/drupal/))
* [x] Add to https://civicrm.org/downloadhttps://lab.civicrm.org/dev/drupal/-/issues/101Error cancelling Drupal user2020-01-06T21:44:57ZedvanleeuwenError cancelling Drupal userWhen I cancel a Drupal user account, I get a non-descriptive error and cancellation has not been done. Doing this a second time does cancel the user, but the user relation is still visible in Civi because the entry in civicrm_uf_match is...When I cancel a Drupal user account, I get a non-descriptive error and cancellation has not been done. Doing this a second time does cancel the user, but the user relation is still visible in Civi because the entry in civicrm_uf_match is not removed.
Using Drupal 7.67 and CiviCRM 5.20.2.https://lab.civicrm.org/dev/translation/-/issues/79Error in link on upgrade page after update database ready2022-11-30T18:23:15ZHanVError in link on upgrade page after update database readyI have today upgraded to version 5.55.2, on the screen after the database upgrade is an error in the link in the message:
Your Date Format settings have been automatically updated. (CRM/Upgrade/Incremental/php/FiveFiftyThree.php)
In the...I have today upgraded to version 5.55.2, on the screen after the database upgrade is an error in the link in the message:
Your Date Format settings have been automatically updated. (CRM/Upgrade/Incremental/php/FiveFiftyThree.php)
In the open tag it must be: href='%1'
I found the place with transifex.https://lab.civicrm.org/dev/core/-/issues/4332Error in membership status after a) changing membership type followed by b) p...2023-11-23T07:23:38ZJoeMurrayError in membership status after a) changing membership type followed by b) payment failure## Overview
Using a pay later payment when renewing a membership can lead to problems with the membership status, membership end date and membership type being changed at the time of the renewal being initiated ; these fields are update...## Overview
Using a pay later payment when renewing a membership can lead to problems with the membership status, membership end date and membership type being changed at the time of the renewal being initiated ; these fields are updated without a payment being recorded. It is possible that a payment will never be received, or its processing may fail. It is not easy to revert the data to its former state, or what it would have become through time from date of update to when the correction is attempted. For example, a renewal with a delayed payment might change the status from Grace to Current, the End Date from May 14, 2023 to May 14, 2024, and the Membership Type from General to Student.
These are very old problems dating to at least 2013 I believe.
The problem only occurs when both membership types have the same parent organization, and only for paid memberships. It occurs whether the membership period is Rolling or Fixed, and whether a membership type is being changed or not.
## Proposal
Refactor the current implementation so that a second, temporary membership is created that can store the new information without overwriting the old information until a payment is received for it. The new temporary membership would have a status of Pending. The Pending contribution would be related to the temporary rather than existing membership. When the payment is received (status=complete), the Pending membership's information is used to update the permanent membership, and the temporary membership record is deleted.
## Relevant code
In the Contribution Confirmation page postProcess call [legacyProcessMembership](https://github.com/civicrm/civicrm-core/blob/master/CRM/Contribute/Form/Contribution/Confirm.php#L1623), various fields are updated before the doPayment call is processed [here](https://github.com/civicrm/civicrm-core/blob/master/CRM/Contribute/Form/Contribution/Confirm.php#LL1702C42-L1702C51). As a result, the existing membership record is updated with selected membership type/end date/status after user submits a payment but before the code makes payment request, e.g. to a payment processor. This works if the payment went successfully.
In the case of a payment failure such as for IPN payment processors like Paypal Standard (occasionally when there is a delay on getting IPN callback or if the IPN response is not handled properly like https://lab.civicrm.org/dev/core/-/issues/1931) or for a manual Pay Later payment that isn't received, it leaves the selected membership in current/active state with a changed end date and possibly a different membership type. There is no fallback code written to revert the membership state or set it to Pending, and it isn't easy to reconstruct the data.
## New Behaviour
1. When initiating the Payment for Membership Renewal, create a new membership record and link the contribution in pending status to it. Add a new field, renewing_membership_id, to civicrm_membership to hold a reference from this 'temporary' pending membership to the existing membership that is being renewed. The existing membership record remains unchanged.
2. When the contribution status of the related contribution changes to Complete, update the original membership with the information from the temporary membership and delete the temporary membership.
Recommendation: delay the creation of activities for Membership Renewal (id=8), Change Membership Status (id=35) and Change Membership Type (id=36) until the contribution is completed.
Recommendation: create a new Activity Type, Membership Renewal Pending, to be created when the renewal request is received. In its body, provide: "ID of Membership being renewed: xx, Number of Periods: yy, Membership Type: [Label of Membership Type".
## Implementation:
1. Modify [here](https://github.com/civicrm/civicrm-core/blob/master/CRM/Contribute/Form/Contribution/Confirm.php#L2900), CRM_Member_BAO_Membership::getContactMembership and [here](https://github.com/civicrm/civicrm-core/blob/master/CRM/Contribute/Form/Contribution/Confirm.php#L2939).
2. Add / update new / existing unit tests on these new scenarios.EdselopezEdselopezhttps://lab.civicrm.org/dev/core/-/issues/5076Error on install2024-03-12T13:27:18ZJonGoldError on installI just attempted to install via the normal web UI for the first time in a very long time. Brand new D7 site, brand new Civi. Disabled all components but CiviEvent and CiviMail. On install, I received this message:
```
CRM_Extension_E...I just attempted to install via the normal web UI for the first time in a very long time. Brand new D7 site, brand new Civi. Disabled all components but CiviEvent and CiviMail. On install, I received this message:
```
CRM_Extension_Exception_DependencyException: Cannot disable extension due to dependencies. Consider disabling all these: civi_campaign,civi_case,civi_contribute,civi_member,civi_pledge,civi_report,financialacls in CRM_Extension_Manager->disable() (line 387 of /var/www/mysite.org/web/sites/all/modules/civicrm/CRM/Extension/Manager.php).
```
Reloading the page got me past the error but since it's the first thing a new admin will see it's worth fixing IMO.https://lab.civicrm.org/dev/core/-/issues/3050Error thrown when 'remove'ing from CiviContribute Batch2022-11-02T00:48:39ZmarshError thrown when 'remove'ing from CiviContribute BatchOverview
----------------------------------------
Civi throws an error when working with Accounting Batches. It's kind of hard to pin down - the front end gives the default / standard / very helpful
No response from the server. Check yo...Overview
----------------------------------------
Civi throws an error when working with Accounting Batches. It's kind of hard to pin down - the front end gives the default / standard / very helpful
No response from the server. Check your internet connection and try reloading the page.
Reproduction steps
----------------------------------------
- add a new accounting Batch (Contributions > Accounting Batches > New Batch)
- Assign a contribution to the batch
- try to remove the contribution from the batch
- at this point the message is displayed:
message, although ConfigAndLog does give a bit more, i.e.:
$Fatal Error Details = array:3 [
"message" => "Cannot delete EntityBatch with no id."
"code" => null
"exception" => CRM_Core_Exception {#2039
-errorData: array:1 [
"error_code" => 0
]
#cause: null
-_trace: null
#message: "Cannot delete EntityBatch with no id."
#code: 0
#file: "/var/www/html/drupal7/sites/all/modules/civicrm/CRM/Core/DAO.php"
#line: 958
trace: {
/var/www/html/drupal7/sites/all/modules/civicrm/CRM/Core/DAO.php:958 {
› if (empty($record['id'])) {
› throw new CRM_Core_Exception("Cannot delete {$entityName} with no id.");
› }
}
/var/www/html/drupal7/sites/all/modules/civicrm/CRM/Batch/BAO/EntityBatch.php:39 { …}
/var/www/html/drupal7/sites/all/modules/civicrm/CRM/Financial/Page/AJAX.php:207 { …}
/var/www/html/drupal7/sites/all/modules/civicrm/CRM/Utils/REST.php:266 { …}
/var/www/html/drupal7/sites/all/modules/civicrm/CRM/Utils/REST.php:550 { …}
/var/www/html/drupal7/sites/all/modules/civicrm/CRM/Core/Invoke.php:279 { …}
/var/www/html/drupal7/sites/all/modules/civicrm/CRM/Core/Invoke.php:69 { …}
/var/www/html/drupal7/sites/all/modules/civicrm/CRM/Core/Invoke.php:36 { …}
/var/www/html/drupal7/sites/all/modules/civicrm/drupal/civicrm.module:458 { …}
/var/www/html/drupal7/includes/menu.inc:527 { …}
/var/www/html/drupal7/index.php:21 { …}
}
}
]
This has alr3eady been [raised on stackechange](https://civicrm.stackexchange.com/questions/41126/entitybatch-with-no-id-error-when-remobing-items-from-accounting-batches?noredirect=1#comment48386_41126)
Current behaviour
----------------------------------------
![image](/uploads/f3431d6145caa54bbd5c96dec58ac3cf/image.png)
![image](/uploads/4dae3c200fc6696f0ecd6f5b25fd6747/image.png)
Expected behaviour
----------------------------------------
The item is removed from the batch
Environment information
----------------------------------------
- reproduced on dmaster
- using Chromium locally on a Debian boxhttps://lab.civicrm.org/dev/core/-/issues/4458Error when viewing contact-info profile without "view deleted contacts" permi...2023-08-07T12:11:55ZcolemanwError when viewing contact-info profile without "view deleted contacts" permissionThe change in b7edabe813db467aff6dd1ea083d798089198655 switched the profile to use APIv4 to fetch email id for constructing an email link. The API call looks like this:
```php
$emailID = Email::get()->setOrderBy(['is_primary' => 'DES...The change in b7edabe813db467aff6dd1ea083d798089198655 switched the profile to use APIv4 to fetch email id for constructing an email link. The API call looks like this:
```php
$emailID = Email::get()->setOrderBy(['is_primary' => 'DESC'])->setWhere([['contact_id', '=', $this->_id], ['email', '=', $email], ['on_hold', '=', FALSE], ['contact_id.is_deceased', '=', FALSE], ['contact_id.is_deleted', '=', FALSE], ['contact_id.do_not_email', '=', FALSE]])->execute()->first()['id'];
```
It was reported on SE that this fails for users without "view deleted contacts", however I'm unable to reproduce.
See https://civicrm.stackexchange.com/questions/45313/invalid-field-contact-id-is-deceased-apiv4
This should have already been double-fixed by:
- [Revert "Add permission metadata to contact is_deleted field" #22203](https://github.com/civicrm/civicrm-core/pull/22203)
- [APIv4 - Silently ignore non-permissioned fields instead of throwing exceptions #20724](https://github.com/civicrm/civicrm-core/pull/20724)https://lab.civicrm.org/dev/core/-/issues/1328Escape-on-Input => Escape-on-Output2022-09-27T23:42:48ZtottenEscape-on-Input => Escape-on-Output# Introduction
In a multi-tier application like CiviCRM, a piece of data (such as a `first_name`) may be passed through several systems and formats (MySQL/PHP/Firefox/etc; SQL/HTML/URL/JSON/CSV/PDF/etc). In each system, this piece of d...# Introduction
In a multi-tier application like CiviCRM, a piece of data (such as a `first_name`) may be passed through several systems and formats (MySQL/PHP/Firefox/etc; SQL/HTML/URL/JSON/CSV/PDF/etc). In each system, this piece of data may be encoded in a different way. The differences are sometimes imperceptible to a casual reader (ex: "banana" looks the same in SQL, HTML, URL, JSON, and CSV), but mistakes and confusion about encoding are still problematic. The impact ranges from quirky (odd characters appearing occasionally in unexpected places) to malicious (security vulnerabilities).
CiviCRM has a long-standing technical debt with respect to encoding. This debt is not specifically a bug, feature, or vulnerability in itself. (If you know a specific piece of data that is misencoded, then that is often a security problem that should be [reported to the security team](https://civicrm.org/security) for discrete resolution.) Rather, this debt is *an unusual convention* ("Escape-on-Input") which has *unintuitive consequences* and thereby invites bugs.
See also: [Dev Guide: Sanitize inputs or outputs?](https://docs.civicrm.org/dev/en/latest/security/#strategy)
At the recent Barcelona sprint, there was a partial meeting of the security team, and attendees agreed that this was the root issue behind multiple problems and merited special attention. Unfortunately, this convention is deeply embedded. Changing it is difficult and risky, so it has lingered for several years without substantive improvement, and (with its current framing/visibility) would remain indefinitely. The aim of this filing is to have a public discussion/record which works out the what/how/why of a (possible) cleanup.
# Definitions
* __Escape-on-Input (aka Esc-In)__: An architectural style in which data is taken as input and immediately escaped; it is then written to disk in escaped (HTML-esque) format. Consequently, subsequent searches+reads yield HTML-esque strings. These can be outputted literally on HTML pages without any extra processing, but (for other media) they require double processing (decode/re-encode).
* __Escape-on-Output (aka Esc-Out)__: An architectural style in which data is is taken as input and stored in its canonical format. When the data is presented as output, it is escaped in a way that matches the current output-format.
* __Canonical (String format)__: An encoding in which strings look... like they should. For example, if a user types `first_name` of `:<`, then the string is `:<`
* __HTML-esque (String format)__: An encoding in which key HTML characters are escaped as entities. For example, canonical string `:<` becomes HTML-esque string `:<`. In HTML-esque, one does not intend to convey HTML tags.
* __O(1) Plan/Task__: A plan/task in which the number of tasks is fixed. (Ex: To change behavior of all "quickforms", you might patch+test the base class `HTML_Quickform` one time.) It may strictly be 1 or 2 or 3 steps... but the number of steps does *not* depend on how many screens/use-cases are impacted.
* __O(n) Plan/Task__: A plan/task in which the number of tasks depends on how many screens/use-cases are impacted. (Ex: To change behavior of all "quickforms", you might patch+test *every subclass*.)
* __Smarty Filter/Autoescape__: A Smarty filter allows one to programmatically change the behavior/content of all Smarty templates. For example, you can write a Smarty prefilter to automatically add HTML "escape" commands to all output.
* __Linter__: A programmer tool which scans source code and checks for compliance. For example, a linter could have the rule "all Smarty variable include the `|escape` or `|noescape` modifier."
# Safe and unsafe situations
The status quo is peculiar. It is loosely safe in some common situations, but it's surprisingly unsafe in other situations. Consider this table:
<table>
<thead>
<tr><th></th><th>Read/write data via SQL/DAO/BAO</th><th>Read/write data via APIv3/v4</th></tr>
<thead>
<tbody>
<tr><th>Process inputs+outputs via HTML_Quickform+Smarty (HTML-only)</th><td>OK</td><td>Caution</td></tr>
<tr><th>Process inputs+outputs via most frameworks (Drupal Form API, AngularJS, DOM API, PHPExcel, etc)</th><td>Caution</td><td>OK</td></tr>
<tr><th>Interchange data between SQL/DAO/BAO and APIv3/v4</th><td>Caution</td><td>Caution</td></tr>
</tbody>
</table>
In "OK" situations, you can take a string and send it to the screen -- and there's a high probability it will end-up encoded correctly without any thinking/effort.
In "Caution" situations, you should treat most strings with suspicion -- many would merit extra encoding/decoding steps.
The general aim of this cleanup issue is to be "OK" on the entire grid.
# Relevant background/reading
* (Jan 2010) [CRM-5667](https://issues.civicrm.org/jira/browse/CRM-5667): Introduction of "Escape-on-Input"
* (Dec 2012) [svn log](https://github.com/civicrm/civicrm-svn/commits/v4.2/CRM/Core/HTMLInputCoder.php): Mitigation for APIv3 consumers
* (Dec 2012) [CRM-11532](https://issues.civicrm.org/jira/browse/CRM-11532): First filing about the problematic status-quo. (Shorter)
* (Apr 2018, et al) [security/core#6](https://lab.civicrm.org/security/core/issues/6): Second filing about the problematic status-quo. (Longer)
* (2018) [Dev Guide: Sanitize inputs or outputs?](https://docs.civicrm.org/dev/en/latest/security/#strategy): Reference material for developers
* (Oct 2019) [POC: Auto-escaping in Smarty v2](https://gist.github.com/totten/5b30e10a21fe626a43a30e21ded26fc4)
* (Oct 2019) [Report: Categorize DB fields by escaping implication](https://gist.github.com/totten/ec78dcb869aa7cbbc95c5f273f7ea30d)
* (Oct 2019) [Report: List of Smarty expressions](https://gist.github.com/totten/9c7176bbdfc63f88d423f026359f50fc)
# Candidate Approaches
* __Do Nothing__:
* __How__: Kick off your shoes. Pull up Netflix.
* __Pro__: Less work
* __Con__: Encourages developers to write buggy code. Feels embarrassing whenever they realize this is a thing.
* __Epic w/Smarty Filter__:
* __Pitch__: Smarty does all HTML-escaping automatically and by default.
* __How__:
* Remove Esc-In policy (`HTMLInputCoder`)
* Write a generic upgrade step to recode all stored strings.
* To restore output-coding, add a Smarty prefilter to auto-escape output.
* __Pro__: Cleans up the database.
* __Con__: The main problem is Smarty templates are not homogeneous - *most* generate HTML, but *some* generate other formats. They *often* pass-through data from the database, but they *also* display data which is computed and/or loaded from a different source. Consequently, there is an O(n) task of re-testing all screens/fields and patching a large number of exceptions.
* __Con__: Additionally, there's the `r-tech` issue - even if we correctly change every screen/field/line in the main application, there is an open-set of third-party integrations. If these use SQL/DAO/BAO and work correctly in the current system, then they'll likely break as soon as the epic change is made, which will create stress for upgrade-scheduling.
* __Con__: Smarty is difficult to write unit tests for.
* __Epic w/DAO Filter__:
* __Pitch__: DAO does all HTML-escaping automatically and by default.
* __How__:
* Remove Esc-In policy (`HTMLInputCoder`)
* Write a generic upgrade step to recode all stored strings.
* To restore output-coding, update DAO/BAO with an optional escaping mode. (Take the current skiplist - everything else defaults to autoescaping.)
* __Pro__: Cleans up the database.
* __Pro__: Looks like an O(1) plan.
* __Pro__: Restoring output coding can be done at a single point - no need to fix existing templates.
* __Pro__: Output coding can be unit-tested.
* __Con__: Only addresses DAO-based query-building (`$d=new DAO_Foo(); $d->bar=123; $d->fetch()`). Does not address `executeQuery('SELECT foo AS bar...')`, `executeQuery('UPDATE foo SET bar...')`, or other query-building. This seems to be left as O(n) task?
* __Question__: Do we get read/write consistency?
* __Wait by Extension Approach__:
* __Pitch__: Grow new UIs on top of APIv3/v4. Obsolete everything all UIs that rely on DAO/BAO/SQL.
* __How__: Do nothing specifically for this problem. Instead, wait for extensions like Afform or Data Processor to gradually replace the HTML_Quickform/Smarty UIs. Once there's a critical mass, remove all old UIs and do an "Epic" switch.
* __Pro__: Don't have to do anything now! Creates pressure to do other useful changes.
* __Con__: You may be waiting a long time.
* __Incremental, Per-Component/Per-Field__:
* __Pitch__: Convert fields incrementally
* __How__: Make a list of all relevant DB fields (~500). In the first month, take a subset (such as "all CiviEvent fields") and transition/test the subset. In the second month, do another subset of fields (such as "all CiviCampaign fields").
* __Pro__: This allows tasks to spread out over time with more manageable/affordable chunks.
* __Con__: This still has the `r-tech` issue for third-party integrations. It creates a long-term drip-drip of compatibility breaks -- every month, some subset of fields have their data-format changed. This will make it quite challenging for extensions/integrations to keep in sync.
* __Comment__: Bespoke screens (e.g. "New participant") are amenable to simple grep+divide+conquer, but we might get boxed-in with how to update metadata-driven screens which tend to treat all strings the same way (e.g. import/export).
* __Incremental, Setting Plus Smarty Linting__:
* __Pitch__: Add toggle to allow old+new storage-formats. Incrementally enhance code to make new-storage-format work.
* __How__:
* Define a new setting to indicate whether strings are stored as "html-esque" (initial-default; long-term-deprecate) or "canonical" (initial-experimental; long-term-goal).
* Define a migration script - which can be run whenever one changes the setting.
* Update Smarty/Quickform/API to check the setting and filter accordingly. Define corresponding Smarty filter. (Naive example: `{$records[$cid]->display_name|crmEscape:'Contact.display_name'}` means "escape this data based on the escaping-rules for the `Contact` field `display_name`)
* Write a linter to report on which Smarty templates use the filter. Over time (month-by-month), incrementally update Smarty templates to use the new filters and re-test those screens.
* __Pro__: This allows tasks to be spread out over time with more manageable/affordable chunks. The linter provides a guide for helping us track "cleared ground".
* __Pro__: Site-implementers and extension/integration-authors will have greater flexibility on scheduling the change - they don't have to update all codes and all databases at the same time.
* __Con__: Requires exposing some new filters/functions/settings.
* __Incremental, Setting Plus DAO Filter__:
* __How__:
* Define a new setting to indicate whether strings are stored as "html-esque" (initial-default; long-term-deprecate) or "canonical" (initial-experimental; long-term-goal).
* Define a migration script - which can be run whenever one changes the setting.
* Update DAO/API to check the setting and filter accordingly.
* __Pro__: At a glance, this looks like an O(1) plan.
* __Pro__: Site-implementers and extension/integration-authors will have greater flexibility on scheduling the change - they don't have to update all codes and all databases at the same time.
* __Con__: Only addresses DAO-based query-building (`$d=new DAO_Foo(); $d->bar=123; $d->fetch()`). Does not address literal queries (`executeQuery('SELECT foo AS bar...')`, `executeQuery('UPDATE foo SET bar...')`) or other forms of query-building (`CRM_Utils_SQL`/APIv4/adhoc PHP). This seems to be left as O(n) task?
* __Question__: Do we get read/write consistency?
* __Incremental, SQL Views with Deprecation Alerts__:
* __Pitch__: Support both formats in MySQL *at the same time*. Borrow the idea of wide-spread VIEWs from multilingual.
* __How__:
* Every entity (eg `Contact`) should have a concrete TABLE and a derived VIEW (eg `civicrm_contact` and `civicrm_contact_new`) which present the same data in different formats (old/HTML-esque and new/canonical).
* `executeQuery()` generates a deprecation warning whenever there's a query that hits old-format table. The warning says something like "`civicrm_contact_old` is deprecated. Please use `civicrm_contact_new` instead. Be sure to update the escaping on any consumers."
* At the mid-point during transition, swap the SQL schema (eg the old-format is given by SQL VIEW and the new-format is given by SQL TABLE).
* Eventually, remove the old-format VIEWs completely.
* __Pro__: Existing (unpatched) consumers in any medium continue to work throughout the transition. But they start to emit warnings immediately.
* __Pro__: Extension/integration-authors (and site-admins/upgraders) have greater flexibility on scheduling changes - they don't have to update all codes and all databases at the same time.
* __Con__: Query performance on the filtered VIEW should be slower than the canonical TABLE. During the transitional process, queries will flop-flop around between faster/slower.https://lab.civicrm.org/dev/core/-/issues/1634Evaluate if any indexed fields are unused2022-12-04T23:07:09ZeileenEvaluate if any indexed fields are unused
**Proposal** (note this is being updated based on discussion & comments may refer to an earlier version)
1. Remove the following columns from the xml from civicrm_activity
* phone_id
* phone_number
* relationship_id
1. Remove th...
**Proposal** (note this is being updated based on discussion & comments may refer to an earlier version)
1. Remove the following columns from the xml from civicrm_activity
* phone_id
* phone_number
* relationship_id
1. Remove the index from the xml on
* medium_id
* is_deleted
1. During upgrade we drop the above columns, if empty.
**Follow ups to consider**
2. There are other columns in the civicrm_activity table that are case specific - we might consider indexing is_current_revision & original_id only when CiviCase is enabled
3. I've been doing some tests on searches and found that searching is faster if I DROP the contribution_status_id index - it might be interesting to test the activity_type_id index although I suspect it has a much greater cardinality & is more useful
**Impact of the above**
1. Data would not be lost but api fields would no longer access those fields
2. Developers who might be using them outside of core could be impacted - we can mitigate by communicating on the dev list & perhaps putting checks & deprecation notices onto sites with data in the fields for a few months before making any changes.
3. DB size would be reduced. Note that empty fields contribute notably to table size IF they are indexed
4. Dev confusion & efficiency is improved by not having unused stuff in core.
**Background**
Obviously that's not something we should rush into so I'll have to ping the dev list etc
Looking at our civicrm_activity table it appears that each index has a base size - of around a half a gig. From there, the index size increases based on how much data is in the table. So an index on an empty field is around 57% of the size of our largest index.
There are 5 fields that are indexed + empty in our database for the civicrm_activity table (
```
Original id used for CiviCase
Medium id
Phone id
relationship_id
is_deleted
```
Plus - is_current_revision is effectively null
So my first question is are these all used in other databases - e.g when civicase is in use.
I couldn't spot references to phone_id and it feels 'wrong' to me anyway as I think you would want to either link to the contact or have a hard reference. I wonder if some of these fields are quietly obsolete?
It's very unsafe to drop core fields. However, I'm pondering dropping the indexes on these fields
@DaveD I'd appreciate your thoughts....https://lab.civicrm.org/dev/core/-/issues/5027Event Confirmation Email Text Token Should be Set as text/html2024-02-28T12:46:59ZpbarmakEvent Confirmation Email Text Token Should be Set as text/htmlPer a chat with Eileen and Seamus, adding a ticket here. The Event confirmation email text is currently not rendering any input HTML tags. It seems this is because the token is not set as "text/html".
Seamus recommended changing https:/...Per a chat with Eileen and Seamus, adding a ticket here. The Event confirmation email text is currently not rendering any input HTML tags. It seems this is because the token is not set as "text/html".
Seamus recommended changing https://github.com/civicrm/civicrm-core/blob/master/CRM/Event/Tokens.php#L239 from this:
```
else {
$tokens[$fieldName]['text/plain'] = $event[$fieldName];
}
```
to this:
```
else {
$type = ($fieldName === 'confirm_email_text' ? 'text/html' : 'text/plain');
$tokens[$fieldName][$type] = $event[$fieldName];
}
```
For my testing, I just added this line around line 227:
`$tokens['confirm_email_text']['text/html'] = $event['confirm_email_text'];`
Either of those seem to solve the issue, converting the value of that field to actual html code.https://lab.civicrm.org/dev/core/-/issues/224Event confirmation emails do not populate guest details when payment is confi...2023-06-19T04:25:50ZJKingsnorthEvent confirmation emails do not populate guest details when payment is confirmed by IPN (API)This issue occurs with:
* Paid events, using an IPN payment gateway, or delayed payment
* On events that can have 'additional participants' (guests)
In this case, the event confirmation emails are missing:
* The names of additional part...This issue occurs with:
* Paid events, using an IPN payment gateway, or delayed payment
* On events that can have 'additional participants' (guests)
In this case, the event confirmation emails are missing:
* The names of additional participants
* Any custom 'participant' fields associated with the additional participants
Detailed steps to recreate:
* Create custom fields assigned to the 'Participant' entity, add them to a profile
* Create a free event with online registration, allowing multiple participants, and add the profile with custom fields to it, enable confirmation emails
* Register for the event, with an 'additional participant'
* Notice that the confirmation email contains the 'name' of the additional participant, and also the custom fields associated with their registration
* Create a paid event, with the 'pay later' option (otherwise the same options as above)
* Register for the event, with an 'additional participant' (pay later)
* Now use the Contribution - completetransaction API call to 'complete' the payment (this is the process that would happen if the IPN was triggered).
* Notice that the confirmation email does not contain the name of the participants, and any custom participant fields for the additional participants are NOT included.
Issue cause:
Information from the 'form' submission is passed into the email template when it is triggered for a free event. This information is not populated when the \CRM_Event_BAO_Event::sendMail method is called from the 'completetransaction' API call. Some information is assigned in \CRM_Event_Form_Registration_Confirm::buildQuickForm and includes the $part variable, which is assigned to the template, and subsequently used in the 'online event registration' email template. The custom fields for additional participants are also assigned to the template, based on the form (https://github.com/civicrm/civicrm-core/blob/694ccbf41adc31bce4d056fcc46930ba9c2e15e5/CRM/Event/Form/Registration/Confirm.php#L311), which is also missing from the 'completetransaction' route.
Our solution:
The code around this area of the system was quite spahettified, so we have solved this issue using a custom hook: https://gist.github.com/JKingsnorth/4d233df13a0f6493c44f69d81fa5f27a - although this isn't a great approach (in terms of being able to commit it back to core) - it does seem to resolve the issue.https://lab.civicrm.org/dev/core/-/issues/4907Event full inconsistencies2024-01-15T11:35:24ZeileenEvent full inconsistenciesThe way EventFull is calculated in apiv4 is not consistent with elsewhere
apiv4 returns a field `'remaining_participants'` which is
event.max_participants - COUNT(participant rows for the event where contact is not deleted, participant ...The way EventFull is calculated in apiv4 is not consistent with elsewhere
apiv4 returns a field `'remaining_participants'` which is
event.max_participants - COUNT(participant rows for the event where contact is not deleted, participant is not test and participant status is_counted)
By contrast apiv3 returns the contents of `CRM_Event_BAO_Participant::eventFull()` which also takes into account the participant role and other aspects of the status
`eventFull()` has some funky parameters - there are 12 calls to it
| caller | returnEmptySeats |includeWaitingList|returnWaitingCount|considerTestParticipant|onlyPositiveStatuses|
| ------ | ------ |------ |------ |------ |------ |
| apiv3-event | TRUE |TRUE|FALSE|FALSE|FALSE|
| Participant::eventFullMessage | FALSE |FALSE|FALSE|FALSE|FALSE|
| Participant::eventFullMessage(2) | FALSE |TRUE|TRUE|FALSE|FALSE|
|ParticipantStatusType::process|TRUE|FALSE|FALSE|FALSE|
|ParticipantTest|FALSE|TRUE|FALSE|FALSE|FALSE|
|Registration::preProcess|FALSE|if-event-has-waitlist|FALSE|FALSE|FALSE|
|Registration::preProcess(2)|TRUE|if-event-has-waitlist|FALSE|FALSE|FALSE|
|Event_Confirm::formRule|FALSE|if-event-has-waitlist|FALSE|FALSE|FALSE|
|ParticipantConfirm|TRUE|FALSE|TRUE|FALSE|TRUE|
|Register::preProcess|FALSE|if-event-has-waitlist|FALSE|FALSE|FALSE|
|EventInfo::run|FALSE|if-event-has-waitlist|FALSE|FALSE|FALSE|
|event-cart|TRUE|TRUE|FALSE|FALSE|FALSE|
Note that this issue somewhat relates/ explains
https://lab.civicrm.org/dev/event/-/issues/23https://lab.civicrm.org/dev/core/-/issues/4704Event Info displays "registration is closed", but it requires login2023-10-20T17:45:41ZbgmEvent Info displays "registration is closed", but it requires loginIn commit 597e807091d835fba28a636ea8f9da76bf63b1a0, the condition for assigning this variable changed:
```
--- a/CRM/Event/Page/EventInfo.php
+++ b/CRM/Event/Page/EventInfo.php
@@ -280,10 +280,8 @@ class CRM_Event_Page_EventInfo extends...In commit 597e807091d835fba28a636ea8f9da76bf63b1a0, the condition for assigning this variable changed:
```
--- a/CRM/Event/Page/EventInfo.php
+++ b/CRM/Event/Page/EventInfo.php
@@ -280,10 +280,8 @@ class CRM_Event_Page_EventInfo extends CRM_Core_Page {
$this->assign('registerURL', $url);
}
}
- elseif (CRM_Core_Permission::check('register for events')) {
- $this->assign('registerClosed', TRUE);
- }
}
+ $this->assign('registerClosed', !empty($values['event']['is_online_registration']) && !$isEventOpenForRegistration);
$this->assign('allowRegistration', $allowRegistration);
```
The template in `templates/CRM/Event/Page/EventInfo.tpl` has the following code:
```
{if $registerClosed}
<div class="spacer"></div>
<div class="messages status no-popup">
<i class="crm-i fa-info-circle" aria-hidden="true"></i>
{ts}Registration is closed for this event{/ts}
</div>
{/if}
```
On a specific project, we previously had the following conditions:
- People had to login in order to register for an event
- Registrations can be closed at some pointhttps://lab.civicrm.org/dev/core/-/issues/3462Event location search2024-01-31T10:18:23Zaydunsaidan.saunders@squiffle.ukEvent location searchOverview
----------------------------------------
Following on from #2103 - when configuring an event and reusing a location, Civi shows a message like 'This location is used by 2 other events' but does not indicate which events.
It w...Overview
----------------------------------------
Following on from #2103 - when configuring an event and reusing a location, Civi shows a message like 'This location is used by 2 other events' but does not indicate which events.
It would be useful to show a list of those locations, or include a link to search for them.
Note that SearchKit displays now handle LocBlocks (see #2676)https://lab.civicrm.org/dev/core/-/issues/4179Event online registration validation error under narrow conditions related to...2023-05-01T22:13:11ZAllenShawEvent online registration validation error under narrow conditions related to multiple participant, $0 fee, and contact reference field permissions: "Your payment information looks incomplete. Please go back to the main registration page, to complete ..."Under a very specific set of circumstances, users are unable to complete online event registration because of the validation error:
```
Please correct the following errors in the form fields below:
Your payment information looks incompl...Under a very specific set of circumstances, users are unable to complete online event registration because of the validation error:
```
Please correct the following errors in the form fields below:
Your payment information looks incomplete. Please go back to the main registration page, to complete payment information.
```
**Setup:**
To reproduce this you will need:
1. An event with the following characteristics:
* Configured for online registration, with multiple participants, and with pricing that allows for a $0 amount on the first participant.
* Includes a profile which contains a required contact reference field.
2. A user role with permissions to register for this event, but **without** the "access contact reference fields" permission.
**Repro steps:**
Once you have that set up you can reproduce as follows:
1. As a user with the given role, open the online registration form for this event.
2. Indicate that you are registering for two participants.
3. On the registration form for either of these two participants, you will notice that the contact reference field is not displayed, as expected in light of the user's permissions.
4. For the first participant, ensure that you select price options totaling $0.
5. Proceed to register the second participant, and for this participant ensure that you select a fee amount of greater than $0.
6. Submit the registration form for the second participant.
**Expected** behavior: The event registration is submitted without error (confirmation page or thank-you page is displayed, as appropriate per event configuration)
**Actual incorrect** behavior: Submission of the final participant form results in the above-stated validation error.
**Strict requirements for repro:**
It's worth noting that you can avoid this error by making any one or more of the following changes to the above steps or set up:
- as admin:
- Change permission so that this user can access contact reference fields.
- Do not make the contact reference field required in the profile.
- as user:
- For the first participant, select fees totaling greater than $0.
- For the second participant, select fees totalling $0.
**How the code allows this wackiness to happen:**
- [This line in CRM_Core_Form::validateMandatoryFields](https://github.com/civicrm/civicrm-core/blob/503cdb20e74866ffc68311c7c2be3cf9577a23bf/CRM/Core/Form.php#L2332) adds a validation error to the form if the required contact reference field has no value, without regard for whether the user had permissioned access to the field.
- [This line in CRM_Event_Form_Registration_AdditionalParticipant::validatePaymentValues](https://github.com/civicrm/civicrm-core/blob/master/CRM/Event/Form/Registration/AdditionalParticipant.php#L522) avoids calling `CRM_Core_Form::validateMandatoryFields()` if the first participant had a fee of more than $0.
(Joinery internal ticket reference: F#1041)https://lab.civicrm.org/dev/core/-/issues/3915Event registration sequence weirdness2022-10-20T07:36:12Zaydunsaidan.saunders@squiffle.ukEvent registration sequence weirdnessDocumenting some discussion at the #ManchesterSprint
We have a weird sequence in the event registration workflow whereby customers pay for the event before the total amount is determined. This causes a variety of problems:
- Customers ...Documenting some discussion at the #ManchesterSprint
We have a weird sequence in the event registration workflow whereby customers pay for the event before the total amount is determined. This causes a variety of problems:
- Customers are asked to pay before making choices which is contrary to almost any other payment workflow and therefore confuses customers.
- Payment processors (eg Stripe) need to jump through hoops to make this work which complicates the code and causes more opportunities for failure.
- Customers may be asked by their bank to confirm a payment of zero which is later updated. Those customers who read the amount assume something is broken.
Changing the registration sequence to put payment at the end would resolve all these problems, and nobody (so far) thinks the current arrangement is desirable.
Fyi @mattwirehttps://lab.civicrm.org/dev/core/-/issues/3996Event Registration that has error in Profile should continue to propagate Lin...2022-11-22T14:36:05ZshaneonabikeEvent Registration that has error in Profile should continue to propagate Line ItemsTo be honest I didn't know how to clearly define this title so feel free to change it.
## Problem
We encountered an interesting situation whereby during an Event registration the actual payment went through, but the Line items were mis...To be honest I didn't know how to clearly define this title so feel free to change it.
## Problem
We encountered an interesting situation whereby during an Event registration the actual payment went through, but the Line items were missing. When I looked further I discovered that there was an error due to missing / incorrectly configured fields in a Profile that was part of the Event registration.
This Profile was throwing an Exception, because it could not insert the record into the DB. I realize it makes sense to actually return that Exception, but what I question is whether the line items shouldn't be recorded. In this case, the transaction is completed (below), but the line items aren't there because of the exception.
![Selection_002](/uploads/45666402495106c8814a8de1d2837f1b/Selection_002.png)
## Recreate
1. Create a Custom Field set associated to a Contact, and add several fields
2. Create profile and add those fields
3. Create an Event
4. Add that Profile to the Event
5. Add a custom Price Set with several fields
6. Save
From this point, I'm not certain how this came about but I'm guessing the next steps would be:
1. Create a new set of Custom Fields identical to above and recreate associated to Participant and your Event type
1. Delete the first original Custom Field set, but not the profile
2. Register for the Event
I think where this happens is when the actual ```entity_id``` clashes (well in my case it was). So you would need to register multiple times to make this happen. In the DB teh value table for us was
```
civicrm_value_collector_det_16 | CREATE TABLE `civicrm_value_collector_det_16` (
`id` int(10) unsigned NOT NULL AUTO_INCREMENT COMMENT 'Default MySQL primary key',
`entity_id` int(10) unsigned NOT NULL COMMENT 'Table that this extends',
`name_of_individual_collecting_th_50` varchar(255) DEFAULT NULL,
`is_someone_collecting_this_order_51` tinyint(4) DEFAULT NULL,
`phone_number_52` varchar(255) DEFAULT NULL,
PRIMARY KEY (`id`),
UNIQUE KEY `unique_entity_id` (`entity_id`),
KEY `INDEX_phone_number_52` (`phone_number_52`),
KEY `INDEX_name_of_individual_collecting_th_50` (`name_of_individual_collecting_th_50`),
CONSTRAINT `FK_civicrm_value_collector_det_16_entity_id` FOREIGN KEY (`entity_id`) REFERENCES `civicrm_contact` (`id`) ON DELETE CASCADE
) ENGINE=InnoDB AUTO_INCREMENT=140 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci |
```
## Actual Error
_DB Error: Constraint_ via
```
[debug_info] => INSERT INTO civicrm_value_collector_det_16 ( `is_someone_collecting_this_order_51`,
`name_of_individual_collecting_th_50`,`phone_number_52`,`entity_id` ) VALUES ( 0,'','',128 )
ON DUPLICATE KEY UPDATE `is_someone_collecting_this_order_51` = 0,
`name_of_individual_collecting_th_50` = '',`phone_number_52` = ''
[nativecode=1452 ** Cannot add or update a child row: a foreign key constraint fails
(`sitesite`.`civicrm_value_collector_det_16`, CONSTRAINT
`FK_civicrm_value_collector_det_16_entity_id` FOREIGN KEY (`entity_id`)
REFERENCES `civicrm_contact` (`id`) ON DELETE CASCADE)]
```
## Possible improvements
1. Process the transaction
2. Process line items
3. Save to DB
4. Process Profiles after?
or
1. Process transaction
2. Process Profile (if an exception cache it)
3. Continue as before
4. Throw Exception at end?
I'm not sure, but the fact that the transaction is passed we should ensure that the line items are saved too.
Sorry for the long text!https://lab.civicrm.org/dev/financial/-/issues/25event registration: support partial payments when constructing contribution2020-02-17T04:43:01Zlcdwebevent registration: support partial payments when constructing contributionThe contribution BAO add method supports two parameters that can be used to create a contribution with a partial payment: partial_payment_total and partial_amount_to_pay.
I'd like to modify CRM_Event_Form_Registration::processContributi...The contribution BAO add method supports two parameters that can be used to create a contribution with a partial payment: partial_payment_total and partial_amount_to_pay.
I'd like to modify CRM_Event_Form_Registration::processContribution() to add support for these parameters. Doing so would enable hook based modifications to pass a partial payment amount (e.g. to support a "deposit" amount on an event registration). There should be minimal impact on existing functionality as those params are not actively used. We would simply support them if they are passed to that function.
(looking for concept approval)lcdweblcdwebhttps://lab.civicrm.org/dev/core/-/issues/3959Event Scheduled Reminder Behavior2022-11-10T07:20:20ZLKuttnerEvent Scheduled Reminder BehaviorI am wondering if this behavior has been changed-- I did not see any online mention of a change with Scheduled Reminders.
Scenario: A schedule event reminder was set for 24 hours before the event start time 4:00 PM Tuesday.
On Monday at...I am wondering if this behavior has been changed-- I did not see any online mention of a change with Scheduled Reminders.
Scenario: A schedule event reminder was set for 24 hours before the event start time 4:00 PM Tuesday.
On Monday at 4:00 PM, 24 Hours before the event, the reminder is successfully sent to registered attendees.
On Tuesday, the day of the event, additional contacts register.
Expected behavior: The scheduled reminder that was sent on the previous day is NOT sent to new registrants.
Experienced behavior: The reminder scheduled for the previous day is sent ten minutes after a new contact registered.
I do not find any explanation for the later sent reminder emails.
This is with CiviCRM 5.45.7 on Drupal 7 with PHP 7.3.29https://lab.civicrm.org/dev/core/-/issues/4496Event template fix-ups2023-08-17T14:18:50ZeileenEvent template fix-upsGenerally the goal is to make event templates not dependent on the form layer (using tokens & the MessageTemplate harness)
I'm creating this tracking issue to load up some screen shots for comparisonsGenerally the goal is to make event templates not dependent on the form layer (using tokens & the MessageTemplate harness)
I'm creating this tracking issue to load up some screen shots for comparisons