Dev 5925 #1
Open
Dev 5925 #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thomst
pushed a commit
that referenced
this pull request
Nov 15, 2018
…ing filters
Suppose you run a search ("Find Contact", "Advanced Search", "Custom Search", etc). The result screen includes
several elements (which we'll reference below):
1. Standard pagination (Previous/Next; First/Last; Jump-To)
2. Numerical option for page-size
3. Sortable columns
4. An alphabetical filter
5. Checkboxes
As you work with these options, the content of the `civicrm_prevnext_cache` table may change. This patch does
not substantively change what's in that cache, but makes the column `cacheKey` simpler and more consistent.
Both Before and After (Unchanged)
---------------------------------
* The form's qfKey identifies the current screen/filters/cache.
* If you navigate to the next/previous page (`#1`) or adjust the page-size (`civicrm#2`), the content in `civicrm_prevnext_cache` remains the same (for the given qfKey).
* If you change the sort column (`civicrm#3`) or alphabetic filter (`civicrm#4`), the content in `civicrm_prevnext_cache` is deleted and repopulated (for the given qfKey).
* If you toggle a checkbox, the `civicrm_prevnext_cache.is_selected` property updates accordingly. These selections are retained when changing pages (`#1`/`civicrm#2`),
but they're reset if you use sort or alphabet options (`civicrm#3`/`civicrm#4`).
Before
------
* The content of `civicrm_prevnext_cache.cacheKey` takes one of two forms, depending on whether you're using an alphabetic filter (`civicrm#4`).
* `civicrm search {qfKey}` (typical, without any alphabetic filter)
* `civicrm search {qfKey}_alphabet` (less common, with an alphabetic filter)
* The queries which read or delete the query-cache use a prefix+wildcard, i.e. `WHERE cacheKey LIKE 'civicrm search {qfKey}%'`.
After
-----
* The content of `civicrm_prevnext_cache.cacheKey` takes only one form
* `civicrm search {qfKey}`
* The queries which read or delete the query-cache use an exact match, i.e. `WHERE cacheKey = 'civicrm search {qfKey}'`.`
* The text `_alphabet` does not appear in the PHP source folders (CRM, Civi, bin, api, extern, tests).
Comments
--------
In theory, one can imagine that it's desireable to keep the cached results for each of the sorted/filtered variants of the query.
That might allow the user to quickly switch among different sortings and different alphabetic-filters, or it might
allow some kind of clever management of the selections. But this is not so. As we see (both before and after), the substance
of the cache is deleted whenever the user changes `civicrm#3`/`civicrm#4`. In reality, one user browsing a search screen corresponds to exactly
one query-cache. As near as I can tell, the old code made the names change for no real reason at all.
To observe the behavior empirically, I would twiddle the UI widgets and concurrently inspect the content of the cache tables. For example:
```
mysql> select group_name, path, FROM_BASE64(data), expired_date from civicrm_cache where path like 'civicrm search%';
select 'Total records in' as label, cacheKey, count(*), min(id), max(id) from civicrm_prevnext_cache group by cacheKey
union select 'Selected records in ', cacheKey, count(*), min(id), max(id) from civicrm_prevnext_cache where is_selected=1 group by cacheKey;
+------------------------------+------------------------------------------------------+--------------------------------------------------------------+--------------+
| group_name | path | FROM_BASE64(data) | expired_date |
+------------------------------+------------------------------------------------------+--------------------------------------------------------------+--------------+
| CiviCRM Search PrevNextCache | civicrm search a8ed1e2039241c41457a88f65aa8a8ee_7845 | s:52:"civicrm search a8ed1e2039241c41457a88f65aa8a8ee_7845"; | NULL |
+------------------------------+------------------------------------------------------+--------------------------------------------------------------+--------------+
1 row in set (0.00 sec)
+----------------------+------------------------------------------------------+----------+---------+---------+
| label | cacheKey | count(*) | min(id) | max(id) |
+----------------------+------------------------------------------------------+----------+---------+---------+
| Total records in | civicrm search a8ed1e2039241c41457a88f65aa8a8ee_7845 | 6 | 787 | 792 |
| Selected records in | civicrm search a8ed1e2039241c41457a88f65aa8a8ee_7845 | 1 | 789 | 789 |
+----------------------+------------------------------------------------------+----------+---------+---------+
2 rows in set (0.01 sec)
```
bjendres
pushed a commit
that referenced
this pull request
Apr 29, 2019
…ct PSR-16 Suppose someone calls `ArrayDecorator::get()` or `FastArrayDecorator::get()` with an invalid key (e.g. passing `float` or an `array` instead of a `string`). This patch improves error-reporting for that scenario. This is primarily about fixing multiple test failures in `E2E_Cache_ArrayDecoratorTest` reported on `wp-demo`. These generally look like ``` 1) E2E_Cache_ArrayDecoratorTest::testGetInvalidKeys with data set #1 (true) array_key_exists(): The first argument should be either a string or an integer /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/CRM/Utils/Cache/ArrayDecorator.php:102 /Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/packages/Cache/IntegrationTests/LegacySimpleCacheTest.php:409 /Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar:598 ``` Before ------ The ArrayDecorator first checks its front cache (`array_key_exists`) to see if the key is defined. In the `wp-demo` environment, this produces a warning and causes the test to fail. The condition *is* erroneous, but PSR-16 specifies that the error should be reported as exception. After ----- The condition is reported as the expected exception. The test passes on `wp-demo`. Comment ------- This brings the code in `(Fast)ArrayDecorator.php` into alignment with most of the other `CRM/Utils/Cache/*.php` drivers; in most drivers, it is typical to validate the key at the start of most functions.
bjendres
pushed a commit
that referenced
this pull request
Oct 6, 2019
This avoids fails when the expectations were calculated right at the end of a day but then are compared with the new day's values. Results in this case look like CRM_Report_FormTest::testGetFromTo with data set #1 ('20190606000000', '20190606235959', 'this.day', '', '') fail on data set [ this.day , , ]. Local php time is 2019-06-07 00:00:13 and mysql time is 2019-06-07 00:00:13 Failed asserting that two arrays are equal. --- Expected +++ Actual @@ @@ Array ( - 0 => '20190606000000' - 1 => '20190606235959' + 0 => '20190607000000' + 1 => '20190607235959'
bjendres
pushed a commit
that referenced
this pull request
Oct 6, 2019
dev/core#1186 add in unit test to lock in fix from dmeritcowboy in #1…
bjendres
pushed a commit
that referenced
this pull request
Dec 11, 2019
Updating Fork with Commits on Base
bjendres
pushed a commit
that referenced
this pull request
Dec 11, 2019
Proposed updates to Gitlab issue template
bjendres
pushed a commit
that referenced
this pull request
Mar 16, 2020
Overview -------- This fixes a recent regression in which `xml/GenCode.php` fails to execute in certain configurations. Initial report: civicrm/civicrm-buildkit#503 Before ------ * If the folder `l10n` exists in its traditional location, and if you run `./bin/setup.sh -g`, then it correctly executes. * If the folder `l10n` does not exist in its traditional locacation, and if you run `./bin/setup.sh -g`, then it fails with an error like this: ``` + php -d mysql.default_host=127.0.0.1:3306 -d mysql.default_user=dmasterciv_abcde -d mysql.default_password=abcd1234 GenCode.php schema/Schema.xml '' Drupal civicrm_domain.version := 5.23.alpha1 Parsing schema description schema/Schema.xml Extracting database information Extracting table information PHP Fatal error: Uncaught RuntimeException: Invalid configuration: the [cms.root] path must be an absolute URL in /home/me/.../Civi/Core/Paths.php:174 Stack trace: #0 /home/me/.../Civi/Core/Paths.php(151): Civi\Core\Paths->toAbsoluteUrl('/', 'cms.root') #1 /home/me/.../Civi/Core/Paths.php(176): Civi\Core\Paths->getVariable('cms.root', 'url') civicrm#2 /home/me/.../Civi/Core/Paths.php(151): Civi\Core\Paths->toAbsoluteUrl('/', 'civicrm.root') civicrm#3 /home/me/.../Civi/Core/Paths.php(224): Civi\Core\Paths->getVariable('civicrm.root', 'path') civicrm#4 /home/me/.../Civi/Core/Paths.php(84): Civi\Core\Paths->getPath('l10n') civicrm#5 [internal function]: Civi\Core\Paths->Civi\Core\{closure}() civicrm#6 /home/jo in /home/me/.../Civi/Core/Paths.php on line 174 ``` After ----- You can run `./bin/setup.sh -g` with or without the `l10n` folder. Comments -------- There's an argument to be made that this shouldn't be necessary: when running `GenCode`, it should only create PHP files, and none of the output should depend on the CMS URL - because that's liable to change when you deploy the PHP code. If someone did try to generate URL at such an early stage, it's arguably good to generate an error. In point of fact, `GenCode` is not actually using the CMS URL. The oddity stems from the contract of `CRM_Utils_System_*` (specifically, `getCiviSourceStorage()` and `getDefaultFileStorage()`) which do double-duty providing both path and URL. To avoid duplicate work, `Civi\Core\Paths` uses the same grain of information - it tracks the pairs of path+URL. A more aggressive fix might be to split `getCiviSourceStorage()` and `getDefaultFileStorage()` so that it's possible to get the path and URL separately; then revise `Civi\Core\Paths` to take advantage of the finer-grained contract. However, splitting those things would be a more invasive patch, and we're currently in RC.
jofranz
pushed a commit
that referenced
this pull request
Jan 11, 2023
TypeError: call_user_func(): Argument #1 ($callback) must be a valid callback, non-static method CRM_Case_Page_AJAX::addClient() cannot be called statically
jensschuppe
pushed a commit
that referenced
this pull request
May 12, 2023
The `$sqlFile` may or may not be needed, which produces a warning: ``` Deprecated: file_exists(): Passing null to parameter #1 ($filename) of type string is deprecated in /.../tools/bin/scripts/set-version.php on line 105 ```
jensschuppe
pushed a commit
that referenced
this pull request
Jun 6, 2024
Gives inline accordion the light utility class
jensschuppe
pushed a commit
that referenced
this pull request
Jun 6, 2024
After running regen, toward the end of the process, there are some warnings like: ``` Generating Membership Generating MembershipPayment Generating MembershipLog Deprecated: strtr(): Passing null to parameter #1 ($string) of type string is deprecated in /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/CodeGen/GenerateData.php on line 1563 Call Stack: 0.0035 423192 1. {main}() /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/sql/GenerateData.php:0 1.0962 56966984 2. CRM_Core_CodeGen_GenerateData->generateAll() /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/sql/GenerateData.php:86 2.6564 102240512 3. CRM_Core_CodeGen_GenerateData->generate($itemName = 'MembershipLog') /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/CodeGen/GenerateData.php:84 2.6564 102240560 4. CRM_Core_CodeGen_GenerateData->addMembershipLog() /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/CodeGen/GenerateData.php:113 2.6592 102261720 5. CRM_Core_CodeGen_GenerateData::repairDate($date = NULL) /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/CodeGen/GenerateData.php:1580 30.7495 102266384 6. strtr($string = NULL, $from = ['-' => '', ':' => '', ' ' => '']) /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/CodeGen/GenerateData.php:1563 Deprecated: strtr(): Passing null to parameter #1 ($string) of type string is deprecated in /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/CodeGen/GenerateData.php on line 1563 Call Stack: 0.0035 423192 1. {main}() /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/sql/GenerateData.php:0 1.0962 56966984 2. CRM_Core_CodeGen_GenerateData->generateAll() /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/sql/GenerateData.php:86 2.6564 102240512 3. CRM_Core_CodeGen_GenerateData->generate($itemName = 'MembershipLog') /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/CodeGen/GenerateData.php:84 2.6564 102240560 4. CRM_Core_CodeGen_GenerateData->addMembershipLog() /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/CodeGen/GenerateData.php:113 30.7583 102263704 5. CRM_Core_CodeGen_GenerateData::repairDate($date = NULL) /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/CodeGen/GenerateData.php:1580 30.7583 102263704 6. strtr($string = NULL, $from = ['-' => '', ':' => '', ' ' => '']) /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/CodeGen/GenerateData.php:1563 Generating PCP Generating SoftContribution Generating Pledge Generating PledgePayment ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TEST-PR