Uploaded image for project: 'CiviCRM'
  1. CiviCRM
  2. CRM-19967

Contact query inline docs improvements from CRM-4287

    Details

    • Type: Improvement
    • Status: Done/Fixed
    • Priority: Trivial
    • Resolution: Fixed/Completed
    • Affects Version/s: 4.7.15
    • Fix Version/s: None
    • Component/s: None
    • Labels:
    • Versioning Impact:
      Patch (backwards-compatible bug fixes)
    • Documentation Required?:
      None
    • Funding Source:
      Contributed Code
    • Verified?:
      No

      Description

      A couple of questions / follow-on from CRM-4287. Attempting to improve docs as some of this file is not clear to me.

      CRM_Contact_BAO_Query::fromClause() has a parameter "$primaryLocation" also - could we document what it does, to clarify that against the new search setting?

      Trying to better understand the parameters which don't have function docs in CRM_Contact_BAO_Query::fromClause()

      $ grep -Eri '(\:fromClause|>fromClause)\(' .
      ./CRM/Contact/BAO/GroupContact.php:    $from = CRM_Contact_BAO_Query::fromClause($tables);
      ./CRM/Contact/BAO/SavedSearch.php:      $from = CRM_Contact_BAO_Query::fromClause($whereTables);
      ./CRM/Contact/BAO/SavedSearch.php:        $from = CRM_Contact_BAO_Query::fromClause($whereTables);
      ./CRM/Contact/BAO/Query.php:    $this->_fromClause = self::fromClause($this->_tables, NULL, NULL, $this->_primaryLocation, $this->_mode, $apiEntity);
      ./CRM/Contact/BAO/Query.php:    $this->_simpleFromClause = self::fromClause($this->_whereTables, NULL, NULL, $this->_primaryLocation, $this->_mode);
      ./CRM/Contact/BAO/Query.php:        $this->_fromClause = self::fromClause($this->_tables, NULL, NULL, $this->_primaryLocation, $this->_mode);
      ./CRM/Contact/BAO/Query.php:        $this->_simpleFromClause = self::fromClause($this->_whereTables, NULL, NULL, $this->_primaryLocation, $this->_mode);
      ./CRM/Contact/BAO/Query.php:      $this->_fromClause = self::fromClause($this->_tables, NULL, NULL, $this->_primaryLocation, $this->_mode);
      ./CRM/Contact/BAO/Query.php:      $this->_simpleFromClause = self::fromClause($this->_whereTables, NULL, NULL, $this->_primaryLocation, $this->_mode);
      ./CRM/Contact/BAO/Contact/Permission.php:    $from = CRM_Contact_BAO_Query::fromClause($whereTables);
      ./CRM/Contact/BAO/Contact/Permission.php:    $from = CRM_Contact_BAO_Query::fromClause($whereTables);
      • For most references, only $tables is set.
      • $inner and $right don't seem to be used any more at all?
      • $primaryLocation is only ever set to $this->_primaryLocation or not used, so the parameter could be redundant.
      • $mode is used in a similar fashion - unset or $this->_mode.
      • $mode is passed through to CRM_Activity_BAO_Query::from() where it is not used.
      • $mode is passed through to CRM_Grant_BAO_Query::from() where it is used.
      • $mode is passed through to CRM_Core_Component::from() where it is not used.
      • $mode is passed into CRM_Contact_BAO_Hook::buildSearchFrom() where it is used.
      • $apiEntity has a comment about how it duplicates $mode in a confusing way (function docs for `::initialize()`, `::addSpecialFields()`, `::selectClause()`, see @TODO in `::selectClause()`).
      • The only place I see $_primaryLocation set to false is when CRM_Contact_BAO_Query::locationType() is called with an array in the second entry of $values.
      • All other cases this seems to be false. Variable docs say "Should we only search on primary location." Suspect this is from before "primary location" and "primary detail" were decoupled, feels a bit like what we're doing here (except it's primary location type, not primary detail).

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              xurizaemon Chris Burgess
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: