(dev/core#217) PrevNext - Sanitize the `markSelection()` contract
authorTim Otten <totten@civicrm.org>
Mon, 2 Jul 2018 19:31:10 +0000 (12:31 -0700)
committerTim Otten <totten@civicrm.org>
Wed, 25 Jul 2018 00:31:11 +0000 (17:31 -0700)
The contract feels quirky -- e.g.

* Who would guess that `markSelection()` defaults to `$action == 'unselect'`?
* What's the point of accepting `$entity_table` if it's never used?

Fortunately, this function is only called from `CRM_Contact_Page_AJAX::selectUnselectContacts`,
so it's fairly easy to audit and see that:

* The `$action` is always passed in -- it never relies on the default value.
* The `$entity_table` is never specified explicitly -- it always relies on the default value.

CRM/Core/PrevNextCache/Interface.php
CRM/Core/PrevNextCache/Sql.php

index 161120ca00b8b82802f3c3942ab492deae9df81e..212482fbd62569349514d853b8ba2d4dbc76d067 100644 (file)
@@ -64,13 +64,11 @@ interface CRM_Core_PrevNextCache_Interface {
    * @param string $cacheKey
    * @param string $action
    *   Ex: 'select', 'unselect'.
-   * @param array|int|NULL $cIds
+   * @param array|int|NULL $ids
    *   A list of contact IDs to (un)select.
    *   To unselect all contact IDs, use NULL.
-   * @param string $entity_table
-   *   Ex: 'civicrm_contact'.
    */
-  public function markSelection($cacheKey, $action = 'unselect', $cIds = NULL, $entity_table = 'civicrm_contact');
+  public function markSelection($cacheKey, $action, $ids = NULL);
 
   /**
    * Get the selections.
index 2bf5242fc37db56f76536bcbd70b0167e449c457..9f7d99dc4e64344ff324c71f70f0afe78a597686 100644 (file)
@@ -83,10 +83,10 @@ INSERT INTO civicrm_prevnext_cache ( entity_table, entity_id1, entity_id2, cache
    * @param array|int|NULL $cIds
    *   A list of contact IDs to (un)select.
    *   To unselect all contact IDs, use NULL.
-   * @param string $entity_table
-   *   Ex: 'civicrm_contact'.
    */
-  public function markSelection($cacheKey, $action = 'unselect', $cIds = NULL, $entity_table = 'civicrm_contact') {
+  public function markSelection($cacheKey, $action, $cIds = NULL) {
+    $entity_table = 'civicrm_contact';
+
     if (!$cacheKey) {
       return;
     }