(dev/core#217) PrevNext - Use more consistent cache-keys while adjusting filters

[totten]

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.

Providing more consistent cache-keys makes it easier to define a straight-forward contract for swappable PrevNextCache implementations (dev/core#217).

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 ways the old behavior is desireable -- one might 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 changed the names for no real reason at all.

To observe the behavior empirically, I would run a search, twiddle the UI widgets, and concurrently inspect the content of the PrevNext 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)

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

I confirmed that where the cacheKey is passed to this function it is always qfKey from the url - which is either the qfKey or 'civicrm search ' + $('input[name=qfKey]'

[eileenmcnaughton]

I went through the code & it makes sense but I'm struggling to r-run it because markSelection is not working for me on master - with or without this patch. (I switched back from Redis to ArrayCache & same deal - I am seeing some js errors & I'm not seeing any entries in the prevnext after a basic contact search with no params)

[eileenmcnaughton]

Ok I rebuild my dmaster & applied this & markSelection is working now in Redis and in ArrayCache mode. I did a basic search & export & also checked CiviRules custom search still works (which was an issue on one attempt to rationalise this code)