From 98f489a910cf21cd86e41ae6d1219ae6ddb5cece Mon Sep 17 00:00:00 2001 From: Arlen Johnson Date: Mon, 15 Jun 2026 18:40:23 -0400 Subject: [PATCH 01/13] Improve UI/UX for Terms and Conditions Review (CFM-501) --- .../AgreementCollectors/dispatch.inc | 220 +++--------------- .../templates/element/agreeDialog.php | 4 +- .../TermsAndConditionsController.php | 69 +++++- .../Model/Table/TermsAndConditionsTable.php | 20 +- app/templates/TermsAndConditions/review.php | 132 ++++------- app/templates/element/javascript.php | 4 +- app/webroot/css/co-base.css | 12 + 7 files changed, 155 insertions(+), 306 deletions(-) diff --git a/app/plugins/TermsAgreer/templates/AgreementCollectors/dispatch.inc b/app/plugins/TermsAgreer/templates/AgreementCollectors/dispatch.inc index 11026edd3..12c5412d4 100644 --- a/app/plugins/TermsAgreer/templates/AgreementCollectors/dispatch.inc +++ b/app/plugins/TermsAgreer/templates/AgreementCollectors/dispatch.inc @@ -1,193 +1,29 @@ element('flash', []); - -// Make the Form fields editable -$this->Field->enableFormEditMode(); -?> - -

- -Form->create(null, [ - 'id' => 'agreement-form', - 'type' => 'post' -]); - - -?> - - - - - - - - - - - - - - - - -
- - - - url)): // We have a URL based T&C ?> - - - element('TermsAgreer.agreeDialog', ['vv_tc' => $tc]); ?> - - -
- Form->checkbox( - 'tc'.$tc['id'], - ['id' => 'tc'.$tc['id'], 'class' => 'form-check-input tc-agree-checkbox'] - ) . - $this->Form->label( - 'tc'.$tc['id'], - __d('operation','agree'), - ['class' => 'form-check-label'] - ) - ?> -
-
- - - \ No newline at end of file + /** + * COmanage Registry T&C Agreement Collectors Petition Fields + * + * Portions licensed to the University Corporation for Advanced Internet + * Development, Inc. ("UCAID") under one or more contributor license agreements. + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * UCAID licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * @link https://www.internet2.edu/comanage COmanage Project + * @package registry + * @since COmanage Registry v5.2.0 + * @license Apache License, Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0) + */ + +// This dispatch uses the same user interface as the core TermsAndConditions/tandcs.inc template. + include ROOT . DS . 'templates' . DS . 'TermsAndConditions' . DS . 'tandcs.inc'; \ No newline at end of file diff --git a/app/plugins/TermsAgreer/templates/element/agreeDialog.php b/app/plugins/TermsAgreer/templates/element/agreeDialog.php index 0d710126d..49b25c244 100644 --- a/app/plugins/TermsAgreer/templates/element/agreeDialog.php +++ b/app/plugins/TermsAgreer/templates/element/agreeDialog.php @@ -57,9 +57,9 @@
-

mostly_static_page->title ?>

+

mostly_static_page->title ?? '' ?>

- sanitize($vv_tc->mostly_static_page->body) ?> + sanitize($vv_tc->mostly_static_page->body ?? '') ?>
diff --git a/app/src/Controller/TermsAndConditionsController.php b/app/src/Controller/TermsAndConditionsController.php index d30814b4d..f0060dc43 100644 --- a/app/src/Controller/TermsAndConditionsController.php +++ b/app/src/Controller/TermsAndConditionsController.php @@ -35,6 +35,8 @@ use Cake\Utility\Hash; use \App\Lib\Enum\ProvisioningContextEnum; use \App\Lib\Enum\TAndCStatusEnum; +use TermsAgreer\TermsAgreerPlugin; +use TermsAgreer\Lib\Enum\TAndCEnrollmentModeEnum; class TermsAndConditionsController extends StandardController { public array $paginate = [ @@ -148,8 +150,8 @@ public function review() { // Before we get started, see if a return parameter was requested, and if so if it is permitted. // If so, we'll override any current return URL. We check the return URL here rather than when // we're done because URLs stored by AppController are not subject to the allow list check. - - $returnUrl = $this->request->getQuery('return'); + $request = $this->getRequest(); + $returnUrl = $request->getQuery('return'); if(!empty($returnUrl)) { $returnUrl = base64_decode($returnUrl); @@ -162,7 +164,7 @@ public function review() { if(preg_match($u, $returnUrl)) { // The requested URL is permitted, so store it in the session, potentially overriding // the original return URL - $this->request->getSession()->write('TAndC.return', $returnUrl); + $request->getSession()->write('TAndC.return', $returnUrl); break; } } @@ -183,23 +185,66 @@ public function review() { $status = $this->TermsAndConditions->status((int)$personId); - // If there is nothing left to do, redirect to the original request - $done = true; + if($request->is('post')) { + // Walk the set of $status['tandc']s and look for an agreement in the POST data. + $data = $request->getData(); + + $ok = true; + + foreach($status as $s) { + // The post data is keyed on the string "tc" appended to the T&C id, + // and the expected value is "1". + + $key = "tc".$s['tandc']->id; + + if(!isset($data[$key]) || $data[$key] != "1") { + $ok = false; - foreach($status as $s) { - if($s['status'] != TAndCStatusEnum::Agreed) { - $done = false; - break; + $this->Flash->error(__d('terms_agreer','error.TAndCAgreement.missing', [$s['tandc']->description, $tc->id])); + } } - } - if($done) { - return $this->redirect($this->request->getSession()->read('TAndC.return')); + if($ok) { + foreach($status as $s) { + $this->log("TCiD: " . $s['tandc']->id); + try { + $this->TermsAndConditions->TAndCAgreements->record( + termsAndConditionsId: (int)$s['tandc']->id, + personId: $personId, + actorPersonId: $personId, + identifier: $this->getRequest()->getSession()->read('Auth.external.user') + ); + + // Request provisioning on success + + $this->llog('trace', "Requesting provisioning after T&C Agreement for Person " . $personId); + + $People = TableRegistry::getTableLocator()->get('People'); + + $People->requestProvisioning( + id: (int)$personId, + context: ProvisioningContextEnum::Automatic + ); + } catch(\Exception $e) { + $this->Flash->error($e->getMessage()); + return $this->generateRedirect(null); + } + } + + // Set the flash message + $this->Flash->success(__d('result', 'TermsAndConditions.recorded')); + + // redirect to the original request + return $this->redirect($request->getSession()->read('TAndC.return')); + } } $this->set('vv_tandc_statuses', $status); $this->set('vv_person_id', (int)$personId); + // The default mode for CO and platform wide T&Cs is "Explicit Consent" + $this->set('vv_tandc_mode', TAndCEnrollmentModeEnum::ExplicitConsent); + $this->set('vv_title', __d('controller', 'TermsAndConditions', 99)); } diff --git a/app/src/Model/Table/TermsAndConditionsTable.php b/app/src/Model/Table/TermsAndConditionsTable.php index c4a97ce7e..367a89791 100644 --- a/app/src/Model/Table/TermsAndConditionsTable.php +++ b/app/src/Model/Table/TermsAndConditionsTable.php @@ -88,6 +88,7 @@ public function initialize(array $config): void { $this->setAllowLookupPrimaryLink(['agree', 'proxy', 'recordTAndC', 'revoke']); $this->setAllowLookupRelatedPrimaryLink(['status' => ['person_id']]); $this->setAllowUnkeyedPrimaryLink(['review']); + $this->setAllowEmptyPrimaryLink(['review']); $this->setIndexContains([ 'MostlyStaticPages' @@ -194,25 +195,26 @@ public function status(int $personId): array { // Pull active T&C for the CO/COU $copersonid is a member of. This will NOT include // any outdated T&C, we'll pull those separately, below. - + $whereClause = [ - 'co_id' => $person->co_id, - 'status' => SuspendableStatusEnum::Active + 'TermsAndConditions.co_id' => $person->co_id, + 'TermsAndConditions.status' => SuspendableStatusEnum::Active ]; if(!empty($couIds)) { $whereClause['OR'] = [ - 'cou_id IS NULL', - 'cou_id IN' => array_values($couIds) + 'TermsAndConditions.cou_id IS NULL', + 'TermsAndConditions.cou_id IN' => array_values($couIds) ]; } else { - $whereClause[] = 'cou_id IS NULL'; + $whereClause[] = 'TermsAndConditions.cou_id IS NULL'; } $tandc = $this->find() - ->where($whereClause) - ->order('ordr ASC') - ->all(); + ->contain('MostlyStaticPages') + ->where($whereClause) + ->orderBy(['TermsAndConditions.ordr' => 'ASC']) + ->all(); if(!empty($tandc)) { // Pull all the Agreements for $personId at once, we'll walk through them as needed. diff --git a/app/templates/TermsAndConditions/review.php b/app/templates/TermsAndConditions/review.php index 49e5c255d..1fa254c15 100644 --- a/app/templates/TermsAndConditions/review.php +++ b/app/templates/TermsAndConditions/review.php @@ -1,32 +1,33 @@
@@ -35,68 +36,21 @@
-element('flash') // Flash messages ?> - - +element('flash') // Flash messages ?> + +Form->create(null); // We don't need an object for this form ?>
- - - - - - - - - - - - - - - - - - - - -
getName()]); ?>
-
-
- id; - $action_args['vv_actions'] = []; - - if($t['status'] != TAndCStatusEnum::Agreed) { - // T&C that are not current can be agreed to - - $action_args['vv_actions'][] = [ - 'order' => $this->Menu->getMenuOrder('Default'), - 'icon' => 'signature', - 'url' => [ - 'controller' => 'terms_and_conditions', - 'action' => 'agree', - $t['tandc']->id - ], - 'label' => __d('operation', 'agree'), - 'confirm' => [ - 'dg_body_txt' => __d('operation', 'TermsAndConditions.agree.confirm'), - 'dg_confirm_btn' => __d('operation', 'confirm') - ] - ]; - } + +
- if(!empty($action_args['vv_actions'])) { - print $this->element('menuAction', $action_args); - } - ?> -
- description; ?> - - 		identifier ?? "" ?>created) - ? $this->Time->nice($t['agreement']->created, $vv_tz) - : ""; - ?>
+
+
+Form->end(); ?> \ No newline at end of file diff --git a/app/templates/element/javascript.php b/app/templates/element/javascript.php index f37219223..abb256c72 100644 --- a/app/templates/element/javascript.php +++ b/app/templates/element/javascript.php @@ -339,8 +339,8 @@ } }); - // Add a .nospin class to all on-page cake error and warning links - $(".cake-error a").addClass('nospin'); + // Add a .nospin class to all on-page cake error and warning links (and debugging accordions) + $(".cake-error a, a.cake-debug-collapse").addClass('nospin'); // Add loading animation when a form is submitted, when any item with a "spin" class is clicked, // or on any anchor tag lacking the .nospin class. We do not automatically add this to buttons diff --git a/app/webroot/css/co-base.css b/app/webroot/css/co-base.css index 3b59ef4ad..7e5ca1bb7 100644 --- a/app/webroot/css/co-base.css +++ b/app/webroot/css/co-base.css @@ -2555,6 +2555,18 @@ body.termsandconditions ul.form-list li.fields-url { .tc-agree-dialog .modal-body h6 { margin-bottom: 0.5em; } +/* Terms and Conditions Review hides most interface elements. + XXX If we find there is a general need to hide navigation elements outside of + enrollment flows and T&Cs, we'll do this more generically */ +body.termsandconditions.review #top-bar, +body.termsandconditions.review #breadcrumbs, +body.termsandconditions.review #navigation-drawer { + display: none; +} +body.termsandconditions.review #tandc-review-submit { + text-align: right; + margin: 1.5em 0 0; +} /* MOSTLY STATIC PAGES */ body.pages.logged-out #top-bar, body.pages.logged-out #breadcrumbs { From b8d117d2584951c3603218419dde66ca9f460f08 Mon Sep 17 00:00:00 2001 From: Arlen Johnson Date: Thu, 18 Jun 2026 17:58:02 -0400 Subject: [PATCH 02/13] Move common code and language strings from TermsAgreer plugin to core (CFM-501) --- .../resources/locales/en_US/terms_agreer.po | 9 - .../AgreementCollectors/dispatch.inc | 52 ++--- app/resources/locales/en_US/information.po | 9 + app/templates/TermsAndConditions/review.php | 5 +- app/templates/TermsAndConditions/tandcs.inc | 210 ++++++++++++++++++ .../element/tandcAgreeDialog.php} | 2 +- 6 files changed, 249 insertions(+), 38 deletions(-) create mode 100644 app/templates/TermsAndConditions/tandcs.inc rename app/{plugins/TermsAgreer/templates/element/agreeDialog.php => templates/element/tandcAgreeDialog.php} (97%) diff --git a/app/plugins/TermsAgreer/resources/locales/en_US/terms_agreer.po b/app/plugins/TermsAgreer/resources/locales/en_US/terms_agreer.po index b2c8b8cda..35da383df 100644 --- a/app/plugins/TermsAgreer/resources/locales/en_US/terms_agreer.po +++ b/app/plugins/TermsAgreer/resources/locales/en_US/terms_agreer.po @@ -43,15 +43,6 @@ msgstr "Did not receive agreement for \"{0}\" (T&C {1})" msgid "field.AgreementCollectors.t_and_c_mode" msgstr "Terms and Conditions Mode" -msgid "information.AgreementCollectors.external" -msgstr "These Terms and Conditions will be loaded in an external browser window. After review, you must return to this window and click \"Agree\" to continue." - -msgid "information.AgreementCollectors.review" -msgstr "You must review and agree to these Terms and Conditions before continuing." - -msgid "information.AgreementCollectors.review.tc" -msgstr "Review Terms & Conditions" - msgid "result.AgreementCollectors.ignored" msgstr "Terms and Conditions collection disabled" diff --git a/app/plugins/TermsAgreer/templates/AgreementCollectors/dispatch.inc b/app/plugins/TermsAgreer/templates/AgreementCollectors/dispatch.inc index 12c5412d4..1e8cf7b2d 100644 --- a/app/plugins/TermsAgreer/templates/AgreementCollectors/dispatch.inc +++ b/app/plugins/TermsAgreer/templates/AgreementCollectors/dispatch.inc @@ -1,29 +1,29 @@ Form->create(null); // We don't need an object for this form ?>
diff --git a/app/templates/TermsAndConditions/tandcs.inc b/app/templates/TermsAndConditions/tandcs.inc new file mode 100644 index 000000000..ed9bc8dfc --- /dev/null +++ b/app/templates/TermsAndConditions/tandcs.inc @@ -0,0 +1,210 @@ +element('flash', []); + + // Make the Form fields editable + $this->Field->enableFormEditMode(); +?> + +

+ +Form->create(null, [ + 'id' => 'agreement-form', + 'type' => 'post' + ]); + + +?> + + + + + + + + + + + + + + $tc): ?> + + + + + + + + + + + + +
getName()]); ?>
+ + identifier ?? "" ?>created) + ? $this->Time->nice($vv_tandc_statuses[$i]['agreement']->created, $vv_tz) + : ""; + ?> + + url)): // We have a URL based T&C ?> + + + element('tandcAgreeDialog', ['vv_tc' => $tc]); ?> + + +
+ Form->checkbox( + 'tc'.$tc['id'], + [ + 'id' => 'tc'.$tc['id'], + 'class' => 'form-check-input tc-agree-checkbox', + 'checked' => !empty($vv_tandc_statuses) && $vv_tandc_statuses[$i]['status'] == 'Y' ? true : false, + ] + ) . + $this->Form->label( + 'tc'.$tc['id'], + __d('operation','agree'), + [ + 'class' => 'form-check-label' + ] + ) + ?> +
+
+ + \ No newline at end of file diff --git a/app/plugins/TermsAgreer/templates/element/agreeDialog.php b/app/templates/element/tandcAgreeDialog.php similarity index 97% rename from app/plugins/TermsAgreer/templates/element/agreeDialog.php rename to app/templates/element/tandcAgreeDialog.php index 49b25c244..63e384a90 100644 --- a/app/plugins/TermsAgreer/templates/element/agreeDialog.php +++ b/app/templates/element/tandcAgreeDialog.php @@ -52,7 +52,7 @@
From 76436fb35031e23c13b11cf671defcd247ee6727 Mon Sep 17 00:00:00 2001 From: Arlen Johnson Date: Fri, 26 Jun 2026 17:03:36 -0400 Subject: [PATCH 03/13] Further cleanup to Terms and Conditions (CFM-501) --- .../resources/locales/en_US/terms_agreer.po | 3 --- .../src/Controller/AgreementCollectorsController.php | 2 +- app/resources/locales/en_US/error.po | 3 +++ app/src/Controller/TermsAndConditionsController.php | 10 ++++------ 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/app/plugins/TermsAgreer/resources/locales/en_US/terms_agreer.po b/app/plugins/TermsAgreer/resources/locales/en_US/terms_agreer.po index 35da383df..0cb592900 100644 --- a/app/plugins/TermsAgreer/resources/locales/en_US/terms_agreer.po +++ b/app/plugins/TermsAgreer/resources/locales/en_US/terms_agreer.po @@ -37,9 +37,6 @@ msgstr "Ignore" msgid "enumeration.TAndCEnrollmentModeEnum.IC" msgstr "Implied Consent" -msgid "error.TAndCAgreement.missing" -msgstr "Did not receive agreement for \"{0}\" (T&C {1})" - msgid "field.AgreementCollectors.t_and_c_mode" msgstr "Terms and Conditions Mode" diff --git a/app/plugins/TermsAgreer/src/Controller/AgreementCollectorsController.php b/app/plugins/TermsAgreer/src/Controller/AgreementCollectorsController.php index 80f4cd977..4d9e25d6d 100644 --- a/app/plugins/TermsAgreer/src/Controller/AgreementCollectorsController.php +++ b/app/plugins/TermsAgreer/src/Controller/AgreementCollectorsController.php @@ -119,7 +119,7 @@ public function dispatch(string $id) { if(!isset($data[$key]) || $data[$key] != "1") { $ok = false; - $this->Flash->error(__d('terms_agreer','error.TAndCAgreement.missing', [$tc->description, $tc->id])); + $this->Flash->error(__d('error','TermsAndConditions.agreement.missing', [$tc->description, $tc->id])); } } diff --git a/app/resources/locales/en_US/error.po b/app/resources/locales/en_US/error.po index 2ed785f12..a27e407c1 100644 --- a/app/resources/locales/en_US/error.po +++ b/app/resources/locales/en_US/error.po @@ -451,6 +451,9 @@ msgstr "No outgoing SMTP Server configuration found" msgid "TermsAndConditions.document.one" msgstr "Exactly one of Mostly Static Page or URL must be specified" +msgid "TermsAndConditions.agreement.missing" +msgstr "Did not receive agreement for \"{0}\" (T&C {1})" + msgid "TermsAndConditions.revoke.none" msgstr "No Agreements available to revoke" diff --git a/app/src/Controller/TermsAndConditionsController.php b/app/src/Controller/TermsAndConditionsController.php index f0060dc43..2a0495771 100644 --- a/app/src/Controller/TermsAndConditionsController.php +++ b/app/src/Controller/TermsAndConditionsController.php @@ -35,8 +35,6 @@ use Cake\Utility\Hash; use \App\Lib\Enum\ProvisioningContextEnum; use \App\Lib\Enum\TAndCStatusEnum; -use TermsAgreer\TermsAgreerPlugin; -use TermsAgreer\Lib\Enum\TAndCEnrollmentModeEnum; class TermsAndConditionsController extends StandardController { public array $paginate = [ @@ -200,13 +198,12 @@ public function review() { if(!isset($data[$key]) || $data[$key] != "1") { $ok = false; - $this->Flash->error(__d('terms_agreer','error.TAndCAgreement.missing', [$s['tandc']->description, $tc->id])); + $this->Flash->error(__d('error','TermsAndConditions.agreement.missing', [$s['tandc']->description, $key])); } } if($ok) { foreach($status as $s) { - $this->log("TCiD: " . $s['tandc']->id); try { $this->TermsAndConditions->TAndCAgreements->record( termsAndConditionsId: (int)$s['tandc']->id, @@ -242,8 +239,9 @@ public function review() { $this->set('vv_tandc_statuses', $status); $this->set('vv_person_id', (int)$personId); - // The default mode for CO and platform wide T&Cs is "Explicit Consent" - $this->set('vv_tandc_mode', TAndCEnrollmentModeEnum::ExplicitConsent); + // The default mode for CO and platform wide T&Cs is "Explicit Consent". + // There is no enumeration for this in core, so set the value explicitly to 'EC'. + $this->set('vv_tandc_mode', 'EC'); $this->set('vv_title', __d('controller', 'TermsAndConditions', 99)); } From d247605bad5f3c6d178d03d310b470a096ed89da Mon Sep 17 00:00:00 2001 From: Arlen Johnson Date: Fri, 26 Jun 2026 17:21:44 -0400 Subject: [PATCH 04/13] Fix preview link to TandC Mostly Static Page (CFM-501) --- app/templates/TermsAndConditions/columns.inc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/templates/TermsAndConditions/columns.inc b/app/templates/TermsAndConditions/columns.inc index f1248e8fd..e769c39b9 100644 --- a/app/templates/TermsAndConditions/columns.inc +++ b/app/templates/TermsAndConditions/columns.inc @@ -49,6 +49,13 @@ $rowActions = [ [ 'icon' => 'arrow_outward', 'label' => __d('operation', 'visit.tandc'), - 'callbackUrl' => static fn($entity) => $entity->url + 'callbackUrl' => function($entity) use ($vv_base_url) { + // There should always be either a Mostly Static Page or a URL to reference. + if(!empty($entity->mostly_static_page->name)) { + // The T&C is a mostly static page. Provide a link to it. + return $vv_base_url . '/pages/' . $entity->mostly_static_page->name; + } + return $entity->url; + } ] ]; From 10b8ee5196aaf31e2a7f99aec9cb4d1124742735 Mon Sep 17 00:00:00 2001 From: Arlen Johnson Date: Sat, 27 Jun 2026 22:11:05 -0400 Subject: [PATCH 05/13] Add JavaScript nonce to TandC script tag (CFM-501) --- app/templates/TermsAndConditions/tandcs.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/TermsAndConditions/tandcs.inc b/app/templates/TermsAndConditions/tandcs.inc index ed9bc8dfc..06ab2459e 100644 --- a/app/templates/TermsAndConditions/tandcs.inc +++ b/app/templates/TermsAndConditions/tandcs.inc @@ -118,7 +118,7 @@ - diff --git a/app/templates/TermsAndConditions/tandcs.inc b/app/templates/TermsAndConditions/tandcs.inc index 06ab2459e..19468cbac 100644 --- a/app/templates/TermsAndConditions/tandcs.inc +++ b/app/templates/TermsAndConditions/tandcs.inc @@ -100,7 +100,10 @@ [ 'id' => 'tc'.$tc['id'], 'class' => 'form-check-input tc-agree-checkbox', - 'checked' => !empty($vv_tandc_statuses) && $vv_tandc_statuses[$i]['status'] == 'Y' ? true : false, + 'checked' => !empty($vv_tandc_statuses) && $vv_tandc_statuses[$i]['status'] === 'Y' ? true : false, + 'data-id' => $tc['id'], + 'data-status' => !empty($vv_tandc_statuses) && $vv_tandc_statuses[$i]['status'] === 'Y' ? 'Y' : 'N', + 'data-coid' => !empty($vv_cur_co) ? $vv_cur_co?->id : -1 ] ) . $this->Form->label( @@ -135,9 +138,17 @@ // Enable / disable the "Continue" button. if(allAgreed) { - $('.submit input[type="submit"]').prop('disabled', false); + + $('button#co-tc-continue').prop('disabled', false); + + $('.submit input[type="submit"]').prop('disabled', false); + } else { - $('.submit input[type="submit"]').prop('disabled', true); + + $('button#co-tc-continue').prop('disabled', true); + + $('.submit input[type="submit"]').prop('disabled', true); + } } @@ -155,7 +166,7 @@ jsConfirmGeneric( '', '', - $(this).attr('data-tc-id')+'-window-launcher', + $(this).data('tc-id')+'-window-launcher', '', '', '' @@ -175,36 +186,36 @@ // Launch the T&C window with the external URL let tandcExternalWindow = window.open( - $(this).attr('data-tc-url'), + $(this).data('tc-url'), 'tandcExternalWindow', 'width=' + winWidth + ',height=' + winHeight + ',left=' + left + ',top=' + top + ',resizable=yes,scrollbars=yes,toolbar=no,menubar=no,location=no,status=no' ); // Enable the T&C checkbox - $('#'+$(this).attr('data-tc-id')).prop('disabled', false); + $('#'+$(this).data('tc-id')).prop('disabled', false); }); // Handle "Review" button for Mostly Static Page-based terms and conditions. // Clicking this will reveal the T&C agree dialog. $('tr.tc-msp button.tc-review-button').click(function(e) { e.preventDefault(); - $('#'+$(this).attr('data-tc-id')+'-agree-dialog').modal('show'); + $('#'+$(this).data('tc-id')+'-agree-dialog').modal('show'); }); // Handle "Agree" button click from MSP Dialog $('.tc-agree-button').click(function(e) { // Enable and check the T&C checkbox - $('#'+$(this).attr('data-tc-id')) + $('#'+$(this).data('tc-id')) .prop('checked', true) .prop('disabled', false) .trigger('change'); }); - // Check agreements when a checkbox is clicked. - // If all are checked, we'll enable "Continue". + // Check agreements when a checkbox is clicked. + // If all agreements are checked, we'll enable "Continue". $('.tc-agree-checkbox').change(function() { checkAgreements(tcmode); }); }); - \ No newline at end of file + diff --git a/app/templates/element/form/nameDiv.php b/app/templates/element/form/nameDiv.php index 375436646..fd005759d 100644 --- a/app/templates/element/form/nameDiv.php +++ b/app/templates/element/form/nameDiv.php @@ -90,9 +90,9 @@ ): ?> Form->label($fn, $label) ?> - - + Date: Mon, 10 Aug 2026 13:58:40 -0400 Subject: [PATCH 07/13] Simplify TermsAndConditionsController::review() now that ajax is used to record agreements on login (CFM-501) --- .../TermsAndConditionsController.php | 80 ------------------- 1 file changed, 80 deletions(-) diff --git a/app/src/Controller/TermsAndConditionsController.php b/app/src/Controller/TermsAndConditionsController.php index 2a0495771..688c2e006 100644 --- a/app/src/Controller/TermsAndConditionsController.php +++ b/app/src/Controller/TermsAndConditionsController.php @@ -145,34 +145,7 @@ public function proxy(string $id) { */ public function review() { - // Before we get started, see if a return parameter was requested, and if so if it is permitted. - // If so, we'll override any current return URL. We check the return URL here rather than when - // we're done because URLs stored by AppController are not subject to the allow list check. - $request = $this->getRequest(); - $returnUrl = $request->getQuery('return'); - - if(!empty($returnUrl)) { - $returnUrl = base64_decode($returnUrl); - - $CoSettings = TableRegistry::getTableLocator()->get('CoSettings'); - $settings = $CoSettings->find()->where(['co_id' => $this->getCOID()])->firstOrFail(); - - if(!empty($settings->tc_return_url_allowlist)) { - foreach(preg_split('/\R/', $settings->tc_return_url_allowlist) as $u) { - if(preg_match($u, $returnUrl)) { - // The requested URL is permitted, so store it in the session, potentially overriding - // the original return URL - $request->getSession()->write('TAndC.return', $returnUrl); - break; - } - } - } else { - // No allowed URLs, so ignore redirect - } - } - // We get the current user from RegistryAuthComponent and then pass their T&C status to the view. - $personId = $this->RegistryAuth->getPersonID($this->getCOID()); if(empty($personId)) { @@ -183,59 +156,6 @@ public function review() { $status = $this->TermsAndConditions->status((int)$personId); - if($request->is('post')) { - // Walk the set of $status['tandc']s and look for an agreement in the POST data. - $data = $request->getData(); - - $ok = true; - - foreach($status as $s) { - // The post data is keyed on the string "tc" appended to the T&C id, - // and the expected value is "1". - - $key = "tc".$s['tandc']->id; - - if(!isset($data[$key]) || $data[$key] != "1") { - $ok = false; - - $this->Flash->error(__d('error','TermsAndConditions.agreement.missing', [$s['tandc']->description, $key])); - } - } - - if($ok) { - foreach($status as $s) { - try { - $this->TermsAndConditions->TAndCAgreements->record( - termsAndConditionsId: (int)$s['tandc']->id, - personId: $personId, - actorPersonId: $personId, - identifier: $this->getRequest()->getSession()->read('Auth.external.user') - ); - - // Request provisioning on success - - $this->llog('trace', "Requesting provisioning after T&C Agreement for Person " . $personId); - - $People = TableRegistry::getTableLocator()->get('People'); - - $People->requestProvisioning( - id: (int)$personId, - context: ProvisioningContextEnum::Automatic - ); - } catch(\Exception $e) { - $this->Flash->error($e->getMessage()); - return $this->generateRedirect(null); - } - } - - // Set the flash message - $this->Flash->success(__d('result', 'TermsAndConditions.recorded')); - - // redirect to the original request - return $this->redirect($request->getSession()->read('TAndC.return')); - } - } - $this->set('vv_tandc_statuses', $status); $this->set('vv_person_id', (int)$personId); From 37f7d66d15ed46d6882251f9f869bea841c060bf Mon Sep 17 00:00:00 2001 From: Arlen Johnson Date: Thu, 20 Aug 2026 09:55:13 -0400 Subject: [PATCH 08/13] Move ajax request check from maybeEnforceTAndCs() to beforeFilter() (CFM-501) --- app/src/Controller/AppController.php | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/app/src/Controller/AppController.php b/app/src/Controller/AppController.php index a5b7e955a..bb8f0f4f9 100644 --- a/app/src/Controller/AppController.php +++ b/app/src/Controller/AppController.php @@ -207,6 +207,7 @@ public function beforeFilter(\Cake\Event\EventInterface $event) { $this->RegistryAuth->isAuthenticatedUser() && !$this->RegistryAuth->isApiUser() && $this->getCOID() !== null + && !$this->request->is('ajax') ) { $this->set('vv_person_id', $this->RegistryAuth->getPersonId($this->getCOID())); @@ -726,13 +727,7 @@ protected function getTheme() { */ protected function maybeEnforceTAndCs() { - // First, skip this on ajax calls - - if($this->request->is('ajax')) { - return; - } - - // Second, if we are processing a request for certain controllers + // First, if we are processing a request for certain controllers // and actions, skip the check and return $modelName = $this->request->getParam('controller'); From 15e99bd78337c94374be3f5a9afd82963730917c Mon Sep 17 00:00:00 2001 From: Arlen Johnson Date: Thu, 27 Aug 2026 14:12:31 -0400 Subject: [PATCH 09/13] Add inline-alert feature (CFM-501) --- app/resources/locales/en_US/information.po | 2 +- app/templates/TermsAndConditions/status.php | 8 ++++++-- app/webroot/css/co-base.css | 8 ++++++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/app/resources/locales/en_US/information.po b/app/resources/locales/en_US/information.po index 74759c4b9..d77f9d14b 100644 --- a/app/resources/locales/en_US/information.po +++ b/app/resources/locales/en_US/information.po @@ -235,7 +235,7 @@ msgid "table.list" msgstr "{0} List" msgid "TermsAndConditions.desc.old" -msgstr "{0} (Agreement is to an older definition)" +msgstr "Agreement is to an older definition" msgid "tandc.review" msgstr "You must review and agree to these Terms and Conditions before continuing." diff --git a/app/templates/TermsAndConditions/status.php b/app/templates/TermsAndConditions/status.php index 5d166a1ff..a2165c7b1 100644 --- a/app/templates/TermsAndConditions/status.php +++ b/app/templates/TermsAndConditions/status.php @@ -114,11 +114,11 @@ if(!empty($t['oldtandc'])) { // If this Agreement is to an older definition of the T&C, use that // older definition and provide an advisory. - + print '
'; print $this->Html->link( // There might be a more elegant way to render this than appending // it to the URL label... - __d('information', 'TermsAndConditions.desc.old', [$t['oldtandc']->description]), + $t['oldtandc']->description, [ 'controller' => 'terms_and_conditions', // We can only view archive records, not edit @@ -126,6 +126,10 @@ $t['oldtandc']->id ] ); + print $this->element('notify/alert', [ + 'message' => __d('information', 'TermsAndConditions.desc.old') + ]); + print '
'; } else { print $this->Html->link( $t['tandc']->description, diff --git a/app/webroot/css/co-base.css b/app/webroot/css/co-base.css index 8e21cdf78..721eb1b0c 100644 --- a/app/webroot/css/co-base.css +++ b/app/webroot/css/co-base.css @@ -484,6 +484,14 @@ body.cos.select #top-bar { color: var(--cmg-color-link-darker); text-decoration: underline; } +.co-inline-alert-container { + display: flex; + gap: 1rem; + align-items: center; +} +.co-inline-alert-container .co-alert { + padding: 0.2rem 0.4rem; +} /* Alerts in the add-edit form: */ ul.form-list li.alert-banner { display: block; From faffd007d509c55c550e31aec411473c3c502796 Mon Sep 17 00:00:00 2001 From: Arlen Johnson Date: Sun, 30 Aug 2026 19:49:50 -0400 Subject: [PATCH 10/13] Improve T&C (and all) mobile rendering (CFM-501) --- .../templates/element/emailVerifiers/list.php | 23 +- .../OrcidSource/templates/element/preview.php | 31 ++- app/templates/Error/error400.php | 2 +- app/templates/Error/error500.php | 2 +- .../ExternalIdentitySources/search.php | 29 ++- app/templates/Petitions/resume.php | 104 ++++++--- app/templates/Standard/index.php | 15 +- app/templates/TermsAndConditions/status.php | 207 ++++++++++-------- app/templates/TermsAndConditions/tandcs.inc | 174 ++++++++------- app/templates/element/javascript.php | 15 ++ app/webroot/css/co-base.css | 112 ++++++++-- app/webroot/css/co-responsive.css | 102 ++++++++- 12 files changed, 559 insertions(+), 257 deletions(-) diff --git a/app/plugins/CoreEnroller/templates/element/emailVerifiers/list.php b/app/plugins/CoreEnroller/templates/element/emailVerifiers/list.php index f4a44b24b..5bdc3ae5a 100644 --- a/app/plugins/CoreEnroller/templates/element/emailVerifiers/list.php +++ b/app/plugins/CoreEnroller/templates/element/emailVerifiers/list.php @@ -101,14 +101,23 @@ ?> - - - - - - - +
+ +
+ + +
+ +
+
+ + + + + + +
diff --git a/app/plugins/OrcidSource/templates/element/preview.php b/app/plugins/OrcidSource/templates/element/preview.php index c5ad8ab0b..f2efffd99 100644 --- a/app/plugins/OrcidSource/templates/element/preview.php +++ b/app/plugins/OrcidSource/templates/element/preview.php @@ -33,18 +33,25 @@ ?> - - - - - - + + + + + + - - - - - - + + + + + +
+
+ +
+ 		+
+
+
diff --git a/app/templates/Error/error400.php b/app/templates/Error/error400.php index 809690169..5de690ef4 100644 --- a/app/templates/Error/error400.php +++ b/app/templates/Error/error400.php @@ -31,7 +31,7 @@ $this->end(); endif; ?> -

+

: '{$url}'") ?> diff --git a/app/templates/Error/error500.php b/app/templates/Error/error500.php index 4ade4dd94..b3971e69a 100644 --- a/app/templates/Error/error500.php +++ b/app/templates/Error/error500.php @@ -35,7 +35,7 @@ $this->end(); endif; ?> -

+

: diff --git a/app/templates/ExternalIdentitySources/search.php b/app/templates/ExternalIdentitySources/search.php index 79858177c..279a034b2 100644 --- a/app/templates/ExternalIdentitySources/search.php +++ b/app/templates/ExternalIdentitySources/search.php @@ -137,6 +137,7 @@ $r): ?> +

Html->link( // Cast to string in case we have a numeric source key (string)$source_key, @@ -149,10 +150,32 @@ 'class' => 'row-link row-link-retrieve' ] ); ?> +
+ + +
+ +
+
+ +
+ + +
+ +
+
+ +
+ + +
+ +
+
+ +
- - - diff --git a/app/templates/Petitions/resume.php b/app/templates/Petitions/resume.php index 9c4f9557c..8fc08f9d7 100644 --- a/app/templates/Petitions/resume.php +++ b/app/templates/Petitions/resume.php @@ -76,39 +76,75 @@
- - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + +
ordr ?? "" ?>description ?>petition_step_results[0]->comment ?? "" ?>actor_type) ?>id ])) { - print $this->Html->link( - ($step->id == $vv_next_step_id) ? - __d('operation', 'continue') . ' arrow_forward' : - __d('operation', 'Petitions.rerun') . ' restart_alt', - $vv_dispatch_urls[ $step->id ], - [ - 'class' => 'btn btn-sm ' . (($step->id == $vv_next_step_id) ? 'btn-tertiary ef-continue-step' : 'btn-default ef-rerun-step'), - 'escape' => false - ] - ); - - if($step->id == $vv_next_step_id) { - $seenNextStep = true; - } - } - ?>
+
+ description ?> +
+ 		+
+ +
+
+ ordr ?? "" ?> +
+ 		+
+ +
+
+ petition_step_results[0]->comment ?? "" ?> +
+ 		+
+ +
+
+ actor_type) ?> +
+ 		+
+ +
+
+ id ])) { + print $this->Html->link( + ($step->id == $vv_next_step_id) ? + __d('operation', 'continue') . ' arrow_forward' : + __d('operation', 'Petitions.rerun') . ' restart_alt', + $vv_dispatch_urls[ $step->id ], + [ + 'class' => 'btn btn-sm ' . (($step->id == $vv_next_step_id) ? 'btn-tertiary ef-continue-step' : 'btn-default ef-rerun-step'), + 'escape' => false + ] + ); + + if($step->id == $vv_next_step_id) { + $seenNextStep = true; + } + } + ?> +
+
diff --git a/app/templates/Standard/index.php b/app/templates/Standard/index.php index dfaff9d5e..50b77a8c8 100644 --- a/app/templates/Standard/index.php +++ b/app/templates/Standard/index.php @@ -272,7 +272,7 @@ - + > '; } + if(!$isFirstLink) { + $label = !empty($cfg['label']) ? $cfg['label'] : \App\Lib\Util\StringUtilities::columnKey($modelsName, $col, $vv_tz); + print '
' . $label . '
'; + print '
'; + } else { + print '
'; + } + switch($cfg['type']) { case 'boolean': if(!empty($entity->$col) && $entity->$col) { @@ -663,6 +671,7 @@ } break; } + print "
"; // cell-content (or cell-first-content) if($isFirstLink && !empty($rowActions)) { print '
'; // field-actions-container @@ -674,7 +683,9 @@ - + + + diff --git a/app/templates/TermsAndConditions/status.php b/app/templates/TermsAndConditions/status.php index a2165c7b1..e3bf45040 100644 --- a/app/templates/TermsAndConditions/status.php +++ b/app/templates/TermsAndConditions/status.php @@ -55,101 +55,124 @@
- id; - $action_args['vv_actions'] = []; - - if($t['status'] != TAndCStatusEnum::Agreed) { - // T&C that are not current can be agreed to on behalf by an admin - - $action_args['vv_actions'][] = [ - 'order' => $this->Menu->getMenuOrder('Default'), - 'icon' => 'signature', - 'url' => [ - 'controller' => 'terms_and_conditions', - 'action' => 'proxy', - $t['tandc']->id, - '?' => [ - 'person_id' => $vv_person_id - ] - ], - 'label' => __d('operation', 'TermsAndConditions.proxy'), - 'confirm' => [ - 'dg_body_txt' => __d('operation', 'TermsAndConditions.proxy.confirm'), - 'dg_confirm_btn' => __d('operation', 'confirm') - ] - ]; - } - - if($t['status'] != TAndCStatusEnum::NotAgreed) { - // T&C that are Agreed, Expired, or Outdated can be revoked - - $action_args['vv_actions'][] = [ - 'order' => $this->Menu->getMenuOrder('Default'), - 'icon' => 'contract_delete', - 'url' => [ - 'controller' => 'terms_and_conditions', - 'action' => 'revoke', - $t['tandc']->id, - '?' => [ - 'person_id' => $vv_person_id - ] - ], - 'label' => __d('operation', 'TermsAndConditions.revoke'), - 'confirm' => [ - 'dg_body_txt' => __d('operation', 'TermsAndConditions.revoke.confirm'), - 'dg_confirm_btn' => __d('operation', 'confirm') - ] - ]; - } - - if(!empty($action_args['vv_actions'])) { - print $this->element('menuAction', $action_args); - } - ?> + id; + $action_args['vv_actions'] = []; + + if($t['status'] != TAndCStatusEnum::Agreed) { + // T&C that are not current can be agreed to on behalf by an admin + + $action_args['vv_actions'][] = [ + 'order' => $this->Menu->getMenuOrder('Default'), + 'icon' => 'signature', + 'url' => [ + 'controller' => 'terms_and_conditions', + 'action' => 'proxy', + $t['tandc']->id, + '?' => [ + 'person_id' => $vv_person_id + ] + ], + 'label' => __d('operation', 'TermsAndConditions.proxy'), + 'confirm' => [ + 'dg_body_txt' => __d('operation', 'TermsAndConditions.proxy.confirm'), + 'dg_confirm_btn' => __d('operation', 'confirm') + ] + ]; + } + + if($t['status'] != TAndCStatusEnum::NotAgreed) { + // T&C that are Agreed, Expired, or Outdated can be revoked + + $action_args['vv_actions'][] = [ + 'order' => $this->Menu->getMenuOrder('Default'), + 'icon' => 'contract_delete', + 'url' => [ + 'controller' => 'terms_and_conditions', + 'action' => 'revoke', + $t['tandc']->id, + '?' => [ + 'person_id' => $vv_person_id + ] + ], + 'label' => __d('operation', 'TermsAndConditions.revoke'), + 'confirm' => [ + 'dg_body_txt' => __d('operation', 'TermsAndConditions.revoke.confirm'), + 'dg_confirm_btn' => __d('operation', 'confirm') + ] + ]; + } + + if(!empty($action_args['vv_actions'])) { + print $this->element('menuAction', $action_args); + } + ?> +
+
+ '; + print $this->Html->link( + // There might be a more elegant way to render this than appending + // it to the URL label... + $t['oldtandc']->description, + [ + 'controller' => 'terms_and_conditions', + // We can only view archive records, not edit + 'action' => 'view', + $t['oldtandc']->id + ] + ); + print $this->element('notify/alert', [ + 'message' => __d('information', 'TermsAndConditions.desc.old') + ]); + print '
'; + } else { + print $this->Html->link( + $t['tandc']->description, + [ + 'controller' => 'terms_and_conditions', + 'action' => 'edit', + $t['tandc']->id + ] + ); + } + ?> +
+
+ + +
+
- '; - print $this->Html->link( - // There might be a more elegant way to render this than appending - // it to the URL label... - $t['oldtandc']->description, - [ - 'controller' => 'terms_and_conditions', - // We can only view archive records, not edit - 'action' => 'view', - $t['oldtandc']->id - ] - ); - print $this->element('notify/alert', [ - 'message' => __d('information', 'TermsAndConditions.desc.old') - ]); - print '
'; - } else { - print $this->Html->link( - $t['tandc']->description, - [ - 'controller' => 'terms_and_conditions', - 'action' => 'edit', - $t['tandc']->id - ] - ); - } +
+ +
+ + +
+ +
+
+ identifier ?? "" ?> +
+ + +
+ getName()]); ?> +
+
+ created) + ? $this->Time->nice($t['agreement']->created, $vv_tz) + : ""; ?> -
+ - - identifier ?? "" ?> - created) - ? $this->Time->nice($t['agreement']->created, $vv_tz) - : ""; - ?> diff --git a/app/templates/TermsAndConditions/tandcs.inc b/app/templates/TermsAndConditions/tandcs.inc index 19468cbac..00cf2f585 100644 --- a/app/templates/TermsAndConditions/tandcs.inc +++ b/app/templates/TermsAndConditions/tandcs.inc @@ -45,81 +45,107 @@ ?> - - - - - - - - - - - - - - $tc): ?> - - - - - - - - + + + + +
getName()]); ?>
- - identifier ?? "" ?>created) - ? $this->Time->nice($vv_tandc_statuses[$i]['agreement']->created, $vv_tz) - : ""; - ?> - - url)): // We have a URL based T&C ?> - - - element('tandcAgreeDialog', ['vv_tc' => $tc]); ?> +
+ + + + + + + + + + + + + + + $tc): ?> + + + + + + - - - - - -
getName()]); ?>
+
+ +
+ 		+
+
+ +
+ 		+
+
+ identifier ?? "" ?> +
+ 		+
getName()]); ?>
+
+ created) + ? $this->Time->nice($vv_tandc_statuses[$i]['agreement']->created, $vv_tz) + : ""; + ?> +
+ 		-
- Form->checkbox( - 'tc'.$tc['id'], - [ - 'id' => 'tc'.$tc['id'], - 'class' => 'form-check-input tc-agree-checkbox', - 'checked' => !empty($vv_tandc_statuses) && $vv_tandc_statuses[$i]['status'] === 'Y' ? true : false, - 'data-id' => $tc['id'], - 'data-status' => !empty($vv_tandc_statuses) && $vv_tandc_statuses[$i]['status'] === 'Y' ? 'Y' : 'N', - 'data-coid' => !empty($vv_cur_co) ? $vv_cur_co?->id : -1 - ] - ) . - $this->Form->label( - 'tc'.$tc['id'], - __d('operation','agree'), - [ - 'class' => 'form-check-label' - ] - ) - ?> -
-
+
+
+
+ + url)): // We have a URL based T&C ?> + + + element('tandcAgreeDialog', ['vv_tc' => $tc]); ?> + +
+ 		+
+
+
+ Form->checkbox( + 'tc'.$tc['id'], + [ + 'id' => 'tc'.$tc['id'], + 'class' => 'form-check-input tc-agree-checkbox', + 'checked' => !empty($vv_tandc_statuses) && $vv_tandc_statuses[$i]['status'] === 'Y' ? true : false, + 'data-id' => $tc['id'], + 'data-status' => !empty($vv_tandc_statuses) && $vv_tandc_statuses[$i]['status'] === 'Y' ? 'Y' : 'N', + 'data-coid' => !empty($vv_cur_co) ? $vv_cur_co?->id : -1 + ] + ) . + $this->Form->label( + 'tc'.$tc['id'], + __d('operation','agree'), + [ + 'class' => 'form-check-label' + ] + ) + ?> +
+
+
+