Improve T&C (and all) mobile rendering (CFM-501) #447

arlen · 2026-08-30T23:55:20Z

No description provided.

arlen · 2026-08-31T00:01:56Z

This PR includes all commits found in #419. This PR adds only one commit, but it was large enough that I didn't want to commingle them.

Ioannis

LGTM

benno · 2026-09-15T11:31:15Z

app/src/Controller/ApiV2Controller.php

-      personId: $json['personId'],
-      actorPersonId: $json['actorPersonId'],
+      personId: (int)$json['personId'],
+      actorPersonId: (int)$json['actorPersonId'],


I realize this is existing code, but what's to stop somebody from sending a random person ID as either the subject or the actor? The original intent of the /record API was for API Users, not AJAX requests, and API Users are already trusted to make calls on behalf of other users.

We probably need to split recordTAndC into two: one for the "regular" API and one for the AJAX API. The second call can set the Actor Person ID based on the currently authenticated user. Person ID would either (1) be the currently authenticated user, or (2) if the user is an Admin, allow Person ID to be asserted if it is any Person the Actor can manage.

I agree with this concern. Can we break this issue out into a different ticket independent of this PR since this one doesn't change the existing code (other than to ensure the (int) type is properly cast)?

This PR adds the record route to the AJAX API, so we can't merge it until this is addressed since doing so will introduce a security issue. I've opened CFM-553 for the API change.

Commit 9edae67 included in this PR addresses this (based on the patch provided by @benno .

benno · 2026-09-15T11:58:41Z

app/src/Controller/TermsAndConditionsController.php

+    // 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 is core code referencing a Plugin concept, so we need to fix this.

Presumably the same concept could apply during Review at Login: as a CO Administrator, I might want to set this to Implied Consent to match the behavior of the Enroller Plugin.

Minimally, it seems like TermsAgreer\Lib\Enum\TAndCEnrollmentModeEnum could become a Core Enumeration, perhaps renamed to something like TAndCAgreementModeEnum. The question is whether we also merge it with TAndCLoginModeEnum. While at first glance it seems like we could, the (unknowable) question is what future enhancements might be requested for the Login Review behavior.

We could, for example, imagine a future request for "Require at Login, but not for Administrators", or "Require at Login, but allow the user to Skip twice". So it probably makes sense to keep the two enums as separate concepts, which unfortunately means we probably also need a second CO Setting to control the Explicit / Implied Consent decision.

Ok - I will move TermsAgreer\Lib\Enum\TAndCEnrollmentModeEnum to Core.

The TermsAgreer\Lib\Enum\TAndCEnrollmentModeEnum has been moved to Core TAndCAgreementModeEnum.

arlen · 2026-09-16T19:01:09Z

This PR has been rebased against the latest develop.

arlen · 2026-09-28T20:06:31Z

Rebased against the latest develop.

…erve database column value

… (CFM-501)

… to record agreements on login (CFM-501)

…CFM-501)

arlen · 2026-09-29T21:04:58Z

This has been rebased against develop, and I believe all issues above have been addressed.

benno · 2026-09-29T23:33:50Z

app/src/Model/Table/TermsAndConditionsTable.php

@@ -123,7 +123,7 @@ public function initialize(array $config): void {
        // able to record the actor foreign key for audit purposes.
        'proxy' =>        ['coAdmin'],
        // 'recordTAndC' is used by ApiV2Controller
-        'recordTAndC' =>  ['platformAdmin', 'coAdmin'],
+        'recordTAndC' =>  ['platformAdmin', 'coAdmin', 'coMember'],


This is going to be a problem because it's going to open the Model Specific API to any API user, not just privileged ones. We'll need to split recordTAndC into two separate actions, one for the Model Specific API, and one for the AJAX API.

This has been addressed in 2c4fdd4

benno · 2026-09-29T23:45:35Z

app/templates/TermsAndConditions/review.php

+      const requests = [];
+      $('input.tc-agree-checkbox').each(function(){
+        const status = $(this).data('status');
+        if(status === 'N') {


Strictly speaking you shouldn't hardcode the enum value, but do something like

if(status === '<?= TAndCStatusEnum::NotAgreed ?>') {

good point - fixed in f0f5894

benno · 2026-09-29T23:47:12Z

app/templates/TermsAndConditions/tandcs.inc

+                    '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',


Similarly, preferable not to hard code the enum values.

also fixed in f0f5894

benno · 2026-09-29T23:47:39Z

app/templates/TermsAndConditions/tandcs.inc

+    $('.tc-agree-checkbox').each(function() {
+      if(!$(this).prop('checked')) {
+        allAgreed = false;
+        if(mode == 'EC') {


also also fixed in f0f5894

…ic API, and one for the AJAX API (CFM-501)

arlen · 2026-10-03T17:33:19Z

app/config/routes.php

@@ -145,19 +145,14 @@ function (RouteBuilder $builder) {
    ['controller' => 'ApiV2', 'action' => 'generateApiKey', 'model' => 'api_users'])
    ->setPass(['id'])
    ->setPatterns(['id' => '[0-9]+']);
- $builder->post(
-   '/terms_and_conditions/record/{id}',


This route was in here twice (which is why the change may look a little more confusing than it is). The only actual change here is changing the terms_and_conditions/record action from 'recordTAndC' to 'xRecordTAndC'.

arlen requested review from Ioannis and benno August 31, 2026 00:03

Ioannis approved these changes Aug 31, 2026

View reviewed changes

arlen mentioned this pull request Sep 3, 2026

Improve UI/UX for Terms and Conditions Review (CFM-501) #419

Closed

arlen force-pushed the feature-cfm501-mobileRendering branch from 6cc996c to b25dd03 Compare September 3, 2026 14:25

Ioannis approved these changes Sep 3, 2026

View reviewed changes

benno requested changes Sep 15, 2026

View reviewed changes

arlen force-pushed the feature-cfm501-mobileRendering branch from e07ad12 to 748c0c8 Compare September 16, 2026 18:59

arlen force-pushed the feature-cfm501-mobileRendering branch from 748c0c8 to a365af2 Compare September 28, 2026 20:06

Ioannis and others added 17 commits September 28, 2026 21:11

Rename TermsAndConditions entity URL accessor to document_url to pres…

d0dcd25

…erve database column value

Improve UI/UX for Terms and Conditions Review (CFM-501)

0729aef

Move common code and language strings from TermsAgreer plugin to core…

ce38fbc

… (CFM-501)

Further cleanup to Terms and Conditions (CFM-501)

f32d86b

Fix preview link to TandC Mostly Static Page (CFM-501)

5eb95ff

Add JavaScript nonce to TandC script tag (CFM-501)

00e4f4d

Implement ajax UI for CO-level Terms and Conditions agreements (CFM-501)

082342e

Simplify TermsAndConditionsController::review() now that ajax is used…

e41e2e0

… to record agreements on login (CFM-501)

Move ajax request check from maybeEnforceTAndCs() to beforeFilter() (…

20d8802

…CFM-501)

Add inline-alert feature (CFM-501)

7e6ad79

Improve T&C (and all) mobile rendering (CFM-501)

4805e90

Ensure elements under #main can overflow when necessary (CFM-501)

3b62d0e

Ensure hover state underlines ignore icons (CFM-501)

e0f62b8

Move TAndCEnrollmentModeEnum to core TAndCAgreementModeEnum (CFM-510)

f201720

Ensure TermsAndConditionsController includes vv_base_url (CFM-501)

49258d9

Create separate API for recording T&C (CFM-553)

9edae67

Fix Mostly Static Page link from T&C index view (CFM-501)

1c24f70

arlen force-pushed the feature-cfm501-mobileRendering branch from a365af2 to 1c24f70 Compare September 29, 2026 20:59

benno requested changes Sep 29, 2026

View reviewed changes

arlen added 2 commits October 1, 2026 14:26

Use enumerations rather than direct values in TandC templates (CFM-501)

f0f5894

Split recordTAndC into two separate actions, one for the Model Specif…

2c4fdd4

…ic API, and one for the AJAX API (CFM-501)

arlen commented Oct 3, 2026

View reviewed changes

Improve T&C (and all) mobile rendering (CFM-501) #447

Are you sure you want to change the base?

Improve T&C (and all) mobile rendering (CFM-501) #447

Conversation

arlen commented Aug 30, 2026

Uh oh!

arlen commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ioannis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arlen commented Sep 16, 2026

Uh oh!

arlen commented Sep 28, 2026

Uh oh!

arlen commented Sep 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arlen commented Aug 31, 2026 •

edited

Loading