CFM-411 - Autocomplete Updates (#212)

* Improve People Picker URL structure * Remove extended query parameters from picker URL * Remove viewConfigParameter:for. Left join both group members and non group members. * add middle name as a filtering option * Add middle name po entry * Fix CoSettings picker configuration * Allow picker email/identifier all or 1 type * UI updates to People Picker (CFM-411) * update autocomplete library. Fix pagination. Improve isMember code changes. * disable li autocomplete option * push .disabled css class * fix keyboard accessibility for pagination * Picker mode just not require Identifiers or EmailAddresses to be linked to a Person * class for the last option element * Further UI updates to People Picker (CFM-411) * Accessibility contrast improvement for People Picker (CFM-411) --------- Co-authored-by: Ioannis Igoumenos <ioigoume@gmail.com>
COmanage · Aug 29, 2024 · ea72b9f · ea72b9f
1 parent 8fb585d
commit ea72b9f
Show file tree

Hide file tree

Showing 25 changed files with 393 additions and 128 deletions.
diff --git a/app/config/routes.php b/app/config/routes.php
@@ -114,6 +114,9 @@ function (RouteBuilder $builder) {
     ['controller' => 'ApiV2', 'action' => 'generateApiKey', 'model' => 'api_users'])
     ->setPass(['id'])
     ->setPatterns(['id' => '[0-9]+']);
+  $builder->get(
+    '/people/pick',
+    ['controller' => 'ApiV2', 'action' => 'pick', 'model' => 'people']);
   // These establish the usual CRUD options on all models:
   $builder->delete(
     '/{model}/{id}', ['controller' => 'ApiV2', 'action' => 'delete'])

diff --git a/app/config/schema/schema.json b/app/config/schema/schema.json
@@ -127,8 +127,8 @@
         "required_fields_name": { "type": "string", "size": 160 },
         "search_global_limit": { "type": "integer" },
         "search_global_limited_models": { "type": "boolean" },
-        "person_picker_email_type": { "type": "integer" },
-        "person_picker_identifier_type": { "type": "integer" },
+        "person_picker_email_address_type_id": { "type": "integer" },
+        "person_picker_identifier_type_id": { "type": "integer" },
         "person_picker_display_types": { "type": "boolean" }
       },
       "indexes": {

diff --git a/app/resources/locales/en_US/field.po b/app/resources/locales/en_US/field.po
@@ -125,6 +125,9 @@ msgstr "Item"
 msgid "family"
 msgstr "Family Name"
 
+msgid "middle"
+msgstr "Middle Name"
+
 msgid "given"
 msgstr "Given Name"
 

diff --git a/app/resources/locales/en_US/operation.po b/app/resources/locales/en_US/operation.po
@@ -66,6 +66,9 @@ msgstr "Apply Database Schema"
 msgid "assign"
 msgstr "Assign"
 
+msgid "all"
+msgstr "All"
+
 msgid "any"
 msgstr "Any"
 

diff --git a/app/src/Controller/ApiV2Controller.php b/app/src/Controller/ApiV2Controller.php
@@ -136,6 +136,25 @@ public function beforeRender(\Cake\Event\EventInterface $event) {
 
     return parent::beforeRender($event);
   }
+
+  /**
+   * Calculate the CO ID associated with the request.
+   *
+   * @since  COmanage Registry v5.0.0
+   * @return int      CO ID, or null if no CO contextwas found
+   */
+
+  public function calculateRequestedCOID(): ?int {
+    if($this->request->getQuery('group_id') !== null) {
+      $groupId = $this->request->getQuery('group_id');
+      $Group = TableRegistry::getTableLocator()->get('Groups');
+
+      $groupRecord = $Group->get($groupId);
+      return $groupRecord->co_id;
+    }
+
+    return null;
+  }
 
   /**
    * Handle a delete action for a Standard object.
@@ -181,7 +200,42 @@ public function delete($id) {
       throw new BadRequestException($this->exceptionToError($e));
     }
   }
-
+
+  protected function dispatchIndex(string $mode = 'default') {
+    // There are use cases where we will pass co_id and another model_id as a query parameter. The co_id might be
+    // required for the primary link calculations while the foreign key for filtering. Since we are using the
+    // most constrained identifier to calculate the co_id, we then check if the two parameters match. If not,
+    // the request should fail, so as to prevent any security holes.
+    if($this->request->getQuery('co_id') !== null
+      && $this->getCOID() !== null
+      && (int)$this->getCOID() !== (int)$this->request->getQuery('co_id')) {
+      $this->llog('error', 'CO Id calculated from Group ID does not match CO Id query parameter');
+      // Mask this with a generic UnauthorizedException
+      throw new UnauthorizedException(__d('error', 'perm'));
+    }
+
+    // $modelsName = Models
+    $modelsName = $this->name;
+    // $table = the actual table object
+    $table = $this->$modelsName;
+
+    $reqParameters = [...$this->request->getQuery()];
+    $pickerMode = ($mode === 'picker');
+
+    // Construct the Query
+    $query = $this->getIndexQuery($pickerMode, $reqParameters);
+
+    if(method_exists($table, 'findIndexed')) {
+      $query = $table->findIndexed($query);
+    }
+    // This magically makes REST calls paginated... can use eg direction=,
+    // sort=, limit=, page=
+    $this->set($this->tableName, $this->paginate($query));
+
+    // Let the view render
+    $this->render('/Standard/api/v2/json/index');
+  }
+
   /**
    * Handle an edit action for a Standard object.
    *
@@ -300,33 +354,7 @@ public function generateApiKey(string $id) {
    */
 
   public function index() {
-    // $modelsName = Models
-    $modelsName = $this->name;
-    // $table = the actual table object
-    $table = $this->$modelsName;
-
-    $reqParameters = [];
-    $pickerMode = false;
-    if($this->request->is('ajax')) {
-      $reqParameters = [...$this->request->getQuery()];
-      if($this->request->getQuery('picker') !== null) {
-        $pickerMode = filter_var($this->request->getQuery('picker'), FILTER_VALIDATE_BOOLEAN);
-      }
-    }
-
-
-    // Construct the Query
-    $query = $this->getIndexQuery($pickerMode, $reqParameters);
-
-    if(method_exists($table, 'findIndexed')) {
-      $query = $table->findIndexed($query);
-    }
-      // This magically makes REST calls paginated... can use eg direction=,
-    // sort=, limit=, page= 
-    $this->set($this->tableName, $this->paginate($query));
-
-    // Let the view render
-    $this->render('/Standard/api/v2/json/index');
+    $this->dispatchIndex();
   }
 
   /**
@@ -354,4 +382,14 @@ public function view($id = null) {
     // Let the view render
     $this->render('/Standard/api/v2/json/index');
   }
+
+  /**
+   * Pick a set of Standard Objects.
+   *
+   * @since  COmanage Registry v5.0.0
+   */
+
+  public function pick() {
+    $this->dispatchIndex(mode: 'picker');
+  }
 }
diff --git a/app/src/Controller/AppController.php b/app/src/Controller/AppController.php
@@ -346,7 +346,8 @@ public function getPrimaryLink(bool $lookup=false) {
           }
         }
 
-        if(empty($this->cur_pl->value) && !$this->$modelsName->allowEmptyPrimaryLink()) {
+        if(empty($this->cur_pl->value)
+           && !$this->$modelsName->allowEmptyPrimaryLink($this->request->getParam('action'))) {
           throw new \RuntimeException(__d('error', 'primary_link'));
         }
       }

diff --git a/app/src/Lib/Traits/IndexQueryTrait.php b/app/src/Lib/Traits/IndexQueryTrait.php
@@ -57,13 +57,38 @@ public function constructGetIndexContains(Query $query): object {
       $containClause = $table->getIndexContains();
     }
 
-    if($this->request->is('restful')|| $this->request->is('ajax')) {
+    if($this->request->is('restful') || $this->request->is('ajax')) {
       $containClause = $this->containClauseFromQueryParams();
     }
 
     return empty($containClause) ? $query : $query->contain($containClause);
   }
 
+  /**
+   * Construct the Picker Contain array
+   *
+   * @param   Query  $query
+   *
+   * @return object Cake ORM Query object
+   * @since  COmanage Registry v5.0.0
+   */
+  public function constructGetPickerContains(Query $query): object {
+    // $this->name = Models
+    $modelsName = $this->name;
+    // $table = the actual table object
+    $table = $this->$modelsName;
+    // Initialize the containClause
+    $containClause = [];
+
+    // Get whatever the table configuration has
+    if(method_exists($table, 'getPickerContains')
+      && $table->getPickerContains()) {
+      $containClause = $table->getPickerContains();
+    }
+
+    return empty($containClause) ? $query : $query->contain($containClause);
+  }
+
 
   /**
    * Construct the Contain Clause from the query parameters of an AJAX or REST call
@@ -151,7 +176,7 @@ public function getIndexQuery(bool $pickerMode = false, array $requestParams = [
     }
 
     // Get Associated Model Data
-    $query = $this->constructGetIndexContains($query);
+    $query = $pickerMode ? $this->constructGetPickerContains($query) : $this->constructGetIndexContains($query);
 
     // Attributes to search for
     if(method_exists($table, 'getSearchableAttributes')) {
@@ -170,8 +195,25 @@ public function getIndexQuery(bool $pickerMode = false, array $requestParams = [
 
         // Here we iterate over the attributes, and we add a new where clause for each one
         foreach($searchableAttributes as $attribute => $options) {
+          $jointype = 'INNER';
+          if (
+            $pickerMode
+            && !empty($options['model'])
+            && \in_array($options['model'], ['Identifiers', 'EmailAddresses'], true)
+          ) {
+            // XXX People picker is different than people filtering. A people picker has the following requirements:
+            //     - Name is required
+            //     - Identifiers, EmailAddresses are optional
+            //     Having that said, we LEFT JOIN the Identifiers and EmailAddresses models instead of INNER JOIN them.
+            $jointype = 'LEFT';
+          }
           // Add the Join Clauses
-          $query = $table->addJoins($query, $attribute, $this->request);
+          $query = $table->addJoins(
+              $query,
+              $attribute,
+              $this->request,
+              $jointype
+          );
 
           // Construct and apply the where Clause
           if(!empty($this->request->getQuery($attribute))) {
@@ -199,11 +241,13 @@ public function getIndexQuery(bool $pickerMode = false, array $requestParams = [
       $query = $query->where(fn(QueryExpression $exp, Query $query) => $exp->in($table->getAlias().'.status', [StatusEnum::Active, StatusEnum::GracePeriod]));
 
       // Specific expressions per view
-      $query = match($requestParams['for'] ?? '') {
+      $query = match(true) {
         // GroupMembers Add view: We need to filter the active members
-        'GroupMembers' => $query->leftJoinWith('GroupMembers', fn($q) => $q->where(['GroupMembers.group_id' => (int)($requestParams['groupid'] ?? -1)]))
-                                ->where($this->getTableLocator()->get('GroupMembers')->checkValidity($query))
-                                ->where(fn(QueryExpression $exp, Query $query) => $exp->isNull('GroupMembers.' . StringUtilities::classNameToForeignKey($table->getAlias()))),
+        (isset($requestParams['group_id']) && $modelsName === 'People') => $query
+          ->leftJoinWith('GroupMembers', fn($q) => $q->where(['GroupMembers.group_id' => (int)($requestParams['group_id'] ?? -1)])),
+//   XXX We want to get both members and not members. The frontend will handle the rest
+//        ->where($this->getTableLocator()->get('GroupMembers')->checkValidity($query))
+//        ->where(fn(QueryExpression $exp, Query $query) => $exp->isNull('GroupMembers.' . StringUtilities::classNameToForeignKey($table->getAlias()))),
         // Just return the query
         default => $query
       };

diff --git a/app/src/Lib/Traits/QueryModificationTrait.php b/app/src/Lib/Traits/QueryModificationTrait.php
@@ -52,6 +52,9 @@ trait QueryModificationTrait {
   // Array of associated models to pull during a view
   private $viewContains = false;
 
+  // Array of associated models to pull during a pick action
+  private $pickerContains = false;
+
 
   /**
    * Construct the checkValidity for the fields valid_from and valid_through
@@ -121,7 +124,18 @@ public function getIndexContains() {
   public function getPatchAssociated() {
     return $this->patchAssociated;
   }
-
+
+  /**
+   * Obtain the set of associated models to pull during a pick.
+   *
+   * @since  COmanage Registry v5.0.0
+   * @return array Array of associated models
+   */
+
+  public function getPickerContains() {
+    return $this->pickerContains;
+  }
+
   /**
    * Obtain the set of associated models to pull during a view.
    *
@@ -187,7 +201,18 @@ public function setIndexFilter(array|\Closure $filter) {
   public function setPatchAssociated(array $a) {
     $this->patchAssociated = $a;
   }
-
+
+  /**
+   * Set the associated models to pull during a pick.
+   *
+   * @since  COmanage Registry v5.0.0
+   * @param  array $c Array of associated models
+   */
+
+  public function setPickerContains(array $c) {
+    $this->pickerContains = $c;
+  }
+
   /**
    * Set the associated models to pull during a view.
    *

diff --git a/app/src/Lib/Traits/SearchFilterTrait.php b/app/src/Lib/Traits/SearchFilterTrait.php
@@ -124,7 +124,7 @@ public function addJoins(Query $query, string $attribute, ServerRequest $request
 
 
     return $query->join($joinAssociations);
-    // XXX We can not use the inenerJoinWith since it applies EagerLoading and includes all the fields which
+    // XXX We can not use the innerJoinWith since it applies EagerLoading and includes all the fields which
     //     causes problems
 //    return $query->innerJoinWith($this->searchFilters[$attribute]['model']);
   }

diff --git a/app/src/Model/Table/CoSettingsTable.php b/app/src/Model/Table/CoSettingsTable.php
@@ -222,25 +222,25 @@ public function addDefaults(int $coId): int {
     // Default values for each setting
 
     $defaultSettings = [
-      'co_id'                             => $coId,
-      'default_address_type_id'           => null,
-      'default_email_address_type_id'     => null,
-      'default_identifier_type_id'        => null,
-      'default_name_type_id'              => null,
-      'default_pronoun_type_id'           => null,
-      'default_telephone_number_type_id'  => null,
-      'default_url_type_id'               => null,
-      'email_smtp_server_id'              => null,
-      'email_delivery_address_type_id'    => null,
-      'permitted_fields_name'             => PermittedNameFieldsEnum::HGMFS,
-      'permitted_fields_telephone_number' => PermittedTelephoneNumberFieldsEnum::CANE,
-      'person_picker_email_type'          => null,
-      'person_picker_identifier_type'     => null,
-      'person_picker_display_types'       => true,
-      'required_fields_address'           => RequiredAddressFieldsEnum::Street,
-      'required_fields_name'              => RequiredNameFieldsEnum::Given,
-      'search_global_limit'               => DEF_GLOBAL_SEARCH_LIMIT,
-      'search_limited_models'             => false
+      'co_id'                                => $coId,
+      'default_address_type_id'              => null,
+      'default_email_address_type_id'        => null,
+      'default_identifier_type_id'           => null,
+      'default_name_type_id'                 => null,
+      'default_pronoun_type_id'              => null,
+      'default_telephone_number_type_id'     => null,
+      'default_url_type_id'                  => null,
+      'email_smtp_server_id'                 => null,
+      'email_delivery_address_type_id'       => null,
+      'permitted_fields_name'                => PermittedNameFieldsEnum::HGMFS,
+      'permitted_fields_telephone_number'    => PermittedTelephoneNumberFieldsEnum::CANE,
+      'person_picker_email_address_type_id'  => null,
+      'person_picker_identifier_type_id'     => null,
+      'person_picker_display_types'          => true,
+      'required_fields_address'              => RequiredAddressFieldsEnum::Street,
+      'required_fields_name'                 => RequiredNameFieldsEnum::Given,
+      'search_global_limit'                  => DEF_GLOBAL_SEARCH_LIMIT,
+      'search_limited_models'                => false
 // XXX to add new settings, set a default here, then add a validation rule below
 //     also update data model documentation
       // 'disable_expiration'         => false,
@@ -416,15 +416,15 @@ public function validationDefault(Validator $validator): Validator {
     ]);
     $validator->allowEmptyString('person_picker_display_types');
 
-    $validator->add('person_picker_email_type', [
+    $validator->add('person_picker_email_address_type_id', [
       'content' => ['rule' => 'isInteger']
     ]);
-    $validator->allowEmptyString('person_picker_email_type');
+    $validator->allowEmptyString('person_picker_email_address_type_id');
 
-    $validator->add('person_picker_identifier_type', [
+    $validator->add('person_picker_identifier_type_id', [
       'content' => ['rule' => 'isInteger']
     ]);
-    $validator->allowEmptyString('person_picker_identifier_type');
+    $validator->allowEmptyString('person_picker_identifier_type_id');
 
     $validator->add('required_fields_address', [
       'content' => ['rule' => ['inList', RequiredAddressFieldsEnum::getConstValues()]]

diff --git a/app/src/Model/Table/GroupsTable.php b/app/src/Model/Table/GroupsTable.php
@@ -176,7 +176,7 @@ public function initialize(array $config): void {
             // For the Groups Filtering block we want to
             // pick/GET from the entire CO pool of people
             'action' => 'GET',
-            // The co configuration will fall throught the default configuration
+            // The co configuration will fall through the default configuration
             'for' => 'co'
           ]
         ]