Skip to content

Improve Relink UI and add People Picker (CFM-125) #316

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

arlen
Copy link
Contributor

@arlen arlen commented May 23, 2025

No description provided.

@@ -471,6 +471,9 @@ msgstr "Redirect on Duplicate"
msgid "EnrollmentFlows.redirect_on_finalize"
msgstr "Redirect on Finalize"

msgid "ExternalIdentitySources.target_person_id"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be ExternalIdentities instead of ExternalIdentitySources

?>
<ul id="edit_ei-relink" class="fields form-list">
<?php
$this->Form->unlockField('target_person_id');
Copy link
Contributor

@Ioannis Ioannis May 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the field does not have to be unlocked. The reason why this is needed is because the backend does not know this field name. We can use the person_id field name which is known to the backend. If we do that we need to parse the id of the user friendly people picker string either before we load the RegistryAuth module - in the AppController.php file - or in the module itself.

$personId = StringUtilities::extractPeoplePickerPersonId($reqData['person_id'])

If we do not mind having the unlocked field then it works well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow the comment about the backend not knowing the field name, what do you mean by "the backend"?

This is a custom form that is generated by the relink view to pass a variable that is parsed by ExternalIdentitiesController::relink() and then passed to PipelinesTable::relink() as a parameter (and is validated there). The name target_person_id is arbitrary and only used for this passing. Since this worked prior to the introduction of the PeoplePicker, I would assume the PeoplePicker code is doing something that causes Cake's form tampering to complain on the submit. (Give this specific use case, we could unlock the field, though I'd still like to understand what exactly is happening to require it.)

Copy link
Contributor

@Ioannis Ioannis May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benno, @arlen

I'm not sure I follow the comment about the backend not knowing the field name, what do you mean by "the backend"?

What I meant is this: person_id is a standard field in the model's schema, whereas target_person_id is not part of the schema by default.

Currently, we’re including a Vue input element in the form, which gets submitted alongside the form data. However, since this input field isn’t rendered using the Form Helper, CakePHP detects a "form tampered" error when the field is submitted.

After some research, I discovered that one way to prevent an input field from being submitted is to remove its name attribute. I wasn’t aware of this previously, but after testing, I found it works as expected. If we remove the name from the Vue input and add a corresponding hidden field with the correct name, only the hidden field will be submitted. The important thing is to ensure the hidden field’s value updates whenever the Vue input changes.

[!important] The hidden field should be visually hidden and not type hidden or display none. For the type hidden use case CAKEPHP will throw a tampered form error. For the display none the field will not be submitted. The visually hidden fields take DOM space, which do not make them the best solution.

  $vv_autocomplete_arguments = [
    'fieldName' => target_person_id',
    'fieldLabel' => __d('field', 'ExternalIdentitySources.target_person_id'),
    'autocomplete' => [
      'configuration' => [
        'action' => 'GET',
        'for' => 'co'
      ]
    ]
  ];

then inside the peopleAutocomplete.php

  print $this->Form->control($fieldName, [
          'id' => $fieldName,
          'value' => '',
          'type' => 'text',
          'class' => 'visually-hidden',
          'label' => false,
      ]
  );

 autocompleteOptions: {
     inputProps: {
            //name: '<?php //= $fieldName ?>//',
            // This is not translated to data-personid but to datapersonid.
            dataPersonid: '<?= $inputValue ?>'
          },
 ...
}

and then inside the people picker element

    setPerson() {
     ...
      document.getElementById(this.options.hiddenFieldId).setAttribute('value', this.person.value);
     ...
   }

hide the field:

.visually-hidden {
  position: absolute;
  width: 1px;
  height: 1px;
  padding: 0;
  margin: -1px;
  border: 0;
  overflow: hidden;
  clip: rect(0, 0, 0, 0);
  white-space: nowrap; /* Prevent text wrapping */
}

in FieldHelper.php

public function constructSPAField(string $element, string $vueElementName): string {
    ...
     'htmlId' => $matchesId[0][1] . '_picker',
    ...
}

* @return int extracted Person ID (converted to int from the extracted string)
*/

public static function extractPeoplePickerPersonId(string $s): int {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will the function do if the string does not match?

@benno
Copy link
Contributor

benno commented Aug 6, 2025

Arlen's original comments (from Slack):

  1. To get this to work, I had to unlock the field in the view. I could not determine how to get around that - possibly I haven’t correctly made the field output look exactly like it would be from Cake. (See: https://github.internet2.edu/COmanage/registry/pull/316/files#diff-19b82a055c6b965b53fd7cca04e59480542c74f9c6f79a63af9b00528ab1c01e )
  2. The people picker (working against the API) does not appear to be finding Person information as I expect it to do. I re-linked an external Identity from one person to another in my set, but when I try to link it back, the search only finds the new person. It’s as if the ApiV2Controller::pick() action only is finding the underlying MVEAs from the External Identities…
  3. I left the current implementation of the people picker mostly alone - so I kept the new StringUtilities function. The autocomplete widget from PrimeVue can be set up within a javascript context to return a specific field - but I did not pursue that yet (because the first two items above need to be examined more). I also did not change the Manager and Sponsor pickers to use the new StringUtilities function (yet).

Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants