Skip to content
Jump to bottom

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
+61 −6
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions app/resources/locales/en_US/field.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

msgstr "Target Person"

msgid "ExternalIdentitySources.hash_source_record"
msgstr "Hash Source Records"

Expand Down
4 changes: 3 additions & 1 deletion app/src/Controller/ExternalIdentitiesController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

use Cake\Event\EventInterface;
use Cake\Http\Response;
use \App\Lib\Util\StringUtilities;
use Cake\ORM\TableRegistry;

// Use extend MVEAController for breadcrumb rendering. ExternalIdentities is
Expand Down Expand Up @@ -87,6 +88,7 @@ public function beforeRender(EventInterface $event) {
// Pull the Person name for breadcrumb rendering

$link = $this->getPrimaryLink(true);
$this->populateAutoViewVars();

if(!empty($link->value)) {
$this->set('vv_bc_parent_obj', $this->ExternalIdentities->People->get($link->value));
Expand All @@ -112,7 +114,7 @@ public function relink(string $id) {
try {
$Pipelines = TableRegistry::getTableLocator()->get('Pipelines');

$Pipelines->relink((int)$id, (int)$reqData['target_person_id']);
$Pipelines->relink((int)$id, StringUtilities::extractPeoplePickerPersonId($reqData['target_person_id']));

$this->Flash->success(__d('result', 'ExternalIdentities.relinked', [$id, $reqData['target_person_id']]));

Expand Down
16 changes: 16 additions & 0 deletions app/src/Lib/Util/StringUtilities.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,22 @@ public static function entityAndActionToTitle($entity,
return [$title, $supertitle, $subtitle];
}

/**
* Extract and return the Person ID from a People Picker string
*
* @since COmanage Registry v5.2.0
* @param string $s Person string from the People Picker widget, e.g. "Jane+Doe+(ID:+2125)"
* @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?

// Regular expression for handling People Picker input
$re = '/^.*\(ID: (\d+)\)$/m';
// Extract the People Picker Person ID
preg_match_all($re, $s, $matchesTarget, PREG_SET_ORDER, 0);
return (int)$matchesTarget[0][1];
}

/**
* Determine the class name from a foreign key (eg: report_id -> Reports).
*
Expand Down
10 changes: 10 additions & 0 deletions app/src/Model/Table/ExternalIdentitiesTable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,16 @@ public function initialize(array $config): void {
'type' => 'enum',
// XXX maybe this (and EIRoles) should be SuspendableStatusEnum?
'class' => 'StatusEnum'
],
// Required for peoplePicker (for re-linking)
'cosettings' => [
'type' => 'auxiliary',
'model' => 'CoSettings'
],
// Required for peoplePicker (for re-linking)
'types' => [
'type' => 'auxiliary',
'model' => 'Types'
]
]);

Expand Down
32 changes: 28 additions & 4 deletions app/templates/ExternalIdentities/relink.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,33 @@
]);

print $this->Form->hidden('external_identity_id', ['default' => $vv_external_identity->id]);
// This label isn't localized, but it should go away when the UX is fixed
print $this->Form->control('target_person_id', ['type' => 'integer', 'label' => 'Target Person ID']);

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

@Ioannis Ioannis May 26, 2025 •
edited
Loading

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 •
edited
Loading

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',
    ...
}

$vv_autocomplete_arguments = [
'fieldName' => 'target_person_id',
'fieldLabel' => __d('field', 'ExternalIdentitySources.target_person_id'),
'autocomplete' => [
'configuration' => [
'action' => 'GET',
'for' => 'co'
]
]
];
print $this->element('form/listItem', ['arguments' => $vv_autocomplete_arguments]);

?>
<li class="fields-submit">
<div class="field">
<div class="field-name">
</div>
<div class="field-info">
<?= $this->Form->submit() ?>
</div>
</div>
</li>
</ul>

<?php
print $this->Form->end();
2 changes: 1 addition & 1 deletion app/templates/element/form/listItem.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
declare(strict_types = 1);

// Make the element configuration available downstream
// XXX Unfortunately CAKEPHP doe not create a viewvar space for the element
// XXX Unfortunately CAKEPHP does not create a viewvar space for the element
// parameters. As a result we do not know which one is which unless we:
// - add a prefix and create a namespace
// - wrap them in an array.
Expand Down