Worked on HELP-476, CiviCRM settings override issue #4
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
sunilpawar
pushed a commit
that referenced
this pull request
Apr 21, 2017
* CRM-20345 - CRM_Utils_Array::crmArraySortByField - Add test. Allow multiple fields.
* CRM-20345 - CRM_Utils_SQL_Select::orderBy - Use more deterministic ordering
The technique of computing default `$weight = count($this->orderBys)`
addresses a valid point: we need to preserve ordering for existing callers
who don't specify weights -- while also allowing weights.
However, it feels weird in my gut. Not sure why -- maybe it's something like this:
```php
// A1: Non-deterministic ordering
$select->orderBy('alpha', 1);
$select->orderBy('beta');
$select->orderBy('delta', 2);
$select->orderBy('gamma', 3);
// A2: Deterministic ordering
$select->orderBy('alpha', 10);
$select->orderBy('beta');
$select->orderBy('delta', 20);
$select->orderBy('gamma', 30);
// B1: Deterministic ordering
$select->orderBy('alpha');
$select->orderBy('beta');
$select->orderBy('delta');
$select->orderBy('gamma');
// B2: Non-deterministic ordering
$select->orderBy('alpha', 1);
$select->orderBy('beta', 1);
$select->orderBy('delta', 1);
$select->orderBy('gamma', 1);
```
As a reader, I would expect A1/A2 to be the same, and I would expect B1/B2
to be the same. But they're not. If there's a collision in the `weight`s,
the ordering becomes non-deterministic (depending on obscure details or
happenstance of the PHP runtime).
Of course, there's no right answer: in A1/A2, you can plausibly put `beta`
before `alpha` or after `alpha` or after `gamma`. But it should be
determinstic so that it always winds up in the same place.
sunilpawar
pushed a commit
that referenced
this pull request
Jun 29, 2017
CRM-20561 - Remove example files. Cleanup code style.
sunilpawar
pushed a commit
that referenced
this pull request
Oct 5, 2017
There appears to be some application logic which follows a process like this: 1. Read the case 2. Tweak the data 3. Save updated case The problem is comes if step #4 resaves a timestamp loaded in step #1, which is fairly likely to happen if you read+save the same record. This was specifically observed on the "Manage Case" screen when editing activities, but the data-flow is pretty common, so make a general fix to the BAO.
sunilpawar
pushed a commit
that referenced
this pull request
Jul 2, 2018
sunilpawar
pushed a commit
that referenced
this pull request
Jul 2, 2018
Accessibility #4: Make alerts accessible, Added new setting under misc.
sunilpawar
pushed a commit
that referenced
this pull request
Nov 2, 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 (`#2`), the content in `civicrm_prevnext_cache` remains the same (for the given qfKey).
* If you change the sort column (`#3`) or alphabetic filter (`#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`/`#2`),
but they're reset if you use sort or alphabet options (`#3`/`#4`).
Before
------
* The content of `civicrm_prevnext_cache.cacheKey` takes one of two forms, depending on whether you're using an alphabetic filter (`#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 `#3`/`#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)
```
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.
No description provided.