fix(optional-policies): resolve null property warnings and improve UX (…

…#61) * fix(optional-policies): resolve `Attempt to read property "is_accepted" on null` warnings * Apply fixes from StyleCI * fix(optional-policies): calm down some PHPStan errors * Apply fixes from StyleCI * fix(optional-policies): make user settings label translatable * feat(optional-policies): place optional policies settings above `flarum/gdpr` * style(optional-policies): remove unnecessary space * refactor(optional-policies): convert UpdateAlert and Policy model to TS * a11y(optional-policies): add aria label to close button * refactor(optional-policies): convert AcceptPoliciesModal to JSX * feat(optional-policies): add a separate alert message for optional policy updates --------- Co-authored-by: rafaucau <[email protected]>
FriendsOfFlarum · Dec 9, 2024 · cb5652f · cb5652f
1 parent 3f79f8b
commit cb5652f
Show file tree

Hide file tree

Showing 9 changed files with 192 additions and 181 deletions.
diff --git a/extend.php b/extend.php
@@ -47,12 +47,9 @@
         ->add(RegisterMiddleware::class),
 
     (new Extend\Model(User::class))
-        ->relationship('fofTermsPolicies', function (AbstractModel $user): BelongsToMany {
-            return $user->belongsToMany(Policy::class, 'fof_terms_policy_user')->withPivot('accepted_at');
-        })
-        ->relationship('fofTermsPoliciesState', function (AbstractModel $user): BelongsToMany {
-            return $user->belongsToMany(Policy::class, 'fof_terms_policy_user')->withPivot('is_accepted');
-        }),
+        ->relationship('fofTermsPolicies', fn (AbstractModel $user): BelongsToMany => $user
+                ->belongsToMany(Policy::class, 'fof_terms_policy_user')
+                ->withPivot(['accepted_at', 'is_accepted'])),
 
     (new Extend\User())
         ->permissionGroups(function ($actor, $groupIds) {

diff --git a/js/src/@types/Model/User.d.ts b/js/src/@types/Model/User.d.ts
@@ -0,0 +1,17 @@
+export * from 'flarum/common/models/User';
+
+declare module 'flarum/common/models/User' {
+  export default interface User {
+    fofTermsPoliciesHasUpdate(): boolean;
+    fofTermsPoliciesMustAccept(): boolean;
+    fofTermsPoliciesState(): Record<
+      number,
+      {
+        accepted_at: string;
+        has_update: boolean;
+        is_accepted: boolean;
+        must_accept: boolean;
+      }
+    >;
+  }
+}
diff --git a/js/src/common/models/Policy.js → js/src/common/models/Policy.ts b/js/src/common/models/Policy.js → js/src/common/models/Policy.ts
@@ -2,12 +2,12 @@ import Model from 'flarum/common/Model';
 import computed from 'flarum/common/utils/computed';
 
 export default class Policy extends Model {
-  sort = Model.attribute('sort');
-  name = Model.attribute('name');
-  url = Model.attribute('url');
-  update_message = Model.attribute('update_message');
-  terms_updated_at = Model.attribute('terms_updated_at');
-  optional = Model.attribute('optional');
+  sort = Model.attribute<string>('sort');
+  name = Model.attribute<string>('name');
+  url = Model.attribute<string>('url');
+  update_message = Model.attribute<string>('update_message');
+  terms_updated_at = Model.attribute<string>('terms_updated_at');
+  optional = Model.attribute<boolean>('optional');
   additional_info = Model.attribute('additional_info');
   form_key = computed('id', (id) => 'fof_terms_policy_' + id);
 

diff --git a/js/src/forum/components/AcceptPoliciesModal.js b/js/src/forum/components/AcceptPoliciesModal.js
@@ -10,7 +10,9 @@ export default class AcceptPoliciesModal extends Modal {
     super.oninit(vnode);
 
     app.store.all('fof-terms-policies').forEach((policy) => {
-      this[policy.form_key()] = false;
+      const state = app.session.user.fofTermsPoliciesState()[policy.id()];
+      // For optional policies, maintain current acceptance status
+      this[policy.form_key()] = policy.optional() ? state?.is_accepted || false : false;
     });
   }
 
@@ -23,7 +25,7 @@ export default class AcceptPoliciesModal extends Modal {
   }
 
   content() {
-    return m('.Modal-body', this.body());
+    return <div className="Modal-body">{this.body()}</div>;
   }
 
   body() {
@@ -36,91 +38,83 @@ export default class AcceptPoliciesModal extends Modal {
     );
 
     if (policies.length === 0) {
-      return Button.component(
-        {
-          className: 'Button',
-          onclick() {
+      return (
+        <Button
+          className="Button"
+          onclick={() => {
             app.modal.close();
-          },
-        },
-        app.translator.trans('fof-terms.forum.accept-modal.close')
+          }}
+        >
+          {app.translator.trans('fof-terms.forum.accept-modal.close')}
+        </Button>
       );
     }
 
-    return policies.map((policy) =>
-      m('div', [
-        m('h2', policy.name()),
-        app.forum.attribute('fof-terms.hide-updated-at')
-          ? null
-          : m(
-              'p',
-              policy.terms_updated_at()
-                ? app.translator.trans('fof-terms.forum.accept-modal.updated-at', {
-                    date: dayjs(policy.terms_updated_at()).format(app.forum.attribute('fof-terms.date-format')),
-                  })
-                : app.translator.trans('fof-terms.forum.accept-modal.updated-recently')
-            ),
-        policy.update_message() ? m('p', policy.update_message()) : null,
-        m(
-          '.Form-group',
-          m(
-            '.FoF-Terms-Check.FoF-Terms-Check--login',
-            m('label.checkbox', [
-              m('input', {
-                type: 'checkbox',
-                checked: this[policy.form_key()],
-                onchange: () => {
+    return policies.map((policy) => (
+      <div>
+        <h2>{policy.name()}</h2>
+        {app.forum.attribute('fof-terms.hide-updated-at') ? null : (
+          <p>
+            {policy.terms_updated_at()
+              ? app.translator.trans('fof-terms.forum.accept-modal.updated-at', {
+                  date: dayjs(policy.terms_updated_at()).format(app.forum.attribute('fof-terms.date-format')),
+                })
+              : app.translator.trans('fof-terms.forum.accept-modal.updated-recently')}
+          </p>
+        )}
+        {policy.update_message() ? <p>{policy.update_message()}</p> : null}
+        <div className="Form-group">
+          <div className="FoF-Terms-Check FoF-Terms-Check--login">
+            <label className="checkbox">
+              <input
+                type="checkbox"
+                checked={this[policy.form_key()]}
+                onchange={() => {
                   this[policy.form_key()] = !this[policy.form_key()];
-                },
-              }),
-              app.translator.trans('fof-terms.forum.accept-modal.i-accept', {
+                }}
+              />
+              {app.translator.trans('fof-terms.forum.accept-modal.i-accept', {
                 policy: policy.name(),
-                a: policy.url()
-                  ? m('a', {
-                      href: policy.url(),
-                      target: '_blank',
-                    })
-                  : m('span'),
-              }),
-            ])
-          )
-        ),
-        Button.component(
-          {
-            className: 'Button Button--primary',
-            disabled: !this[policy.form_key()] && !policy.optional(),
-            onclick: () => {
-              // We need to save the "must accept" property before performing the request
-              // Because an updated user serializer will be returned
-              const hadToAcceptToInteract = app.session.user.fofTermsPoliciesMustAccept();
+                a: policy.url() ? <a href={policy.url()} target="_blank" /> : <span />,
+              })}
+            </label>
+          </div>
+        </div>
+        <Button
+          className="Button Button--primary"
+          disabled={!this[policy.form_key()] && !policy.optional()}
+          onclick={() => {
+            // We need to save the "must accept" property before performing the request
+            // Because an updated user serializer will be returned
+            const hadToAcceptToInteract = app.session.user.fofTermsPoliciesMustAccept();
 
-              app
-                .request({
-                  url: app.forum.attribute('apiUrl') + policy.apiEndpoint() + (this[policy.form_key()] ? '/accept' : '/decline'),
-                  method: 'POST',
-                  errorHandler: this.onerror.bind(this),
-                })
-                .then((updated) => {
-                  app.store.pushPayload(updated);
+            app
+              .request({
+                url: app.forum.attribute('apiUrl') + policy.apiEndpoint() + (this[policy.form_key()] ? '/accept' : '/decline'),
+                method: 'POST',
+                errorHandler: this.onerror.bind(this),
+              })
+              .then((updated) => {
+                app.store.pushPayload(updated);
 
-                  // If this was the last policy to accept, close the modal
-                  if (policies.length === 1) {
-                    if (hadToAcceptToInteract) {
-                      // If the user was previously not allowed to interact with the forum,
-                      // we refresh to get updated permissions in the frontend
-                      window.location.reload();
-                    } else {
-                      app.modal.close();
-                    }
+                // If this was the last policy to accept, close the modal
+                if (policies.length === 1) {
+                  if (hadToAcceptToInteract) {
+                    // If the user was previously not allowed to interact with the forum,
+                    // we refresh to get updated permissions in the frontend
+                    window.location.reload();
+                  } else {
+                    app.modal.close();
                   }
+                }
 
-                  m.redraw();
-                });
-            },
-          },
-          app.translator.trans('fof-terms.forum.accept-modal.accept')
-        ),
-      ])
-    );
+                m.redraw();
+              });
+          }}
+        >
+          {app.translator.trans('fof-terms.forum.accept-modal.accept')}
+        </Button>
+      </div>
+    ));
   }
 }
diff --git a/js/src/forum/components/UpdateAlert.js b/js/src/forum/components/UpdateAlert.js
diff --git a/js/src/forum/components/UpdateAlert.tsx b/js/src/forum/components/UpdateAlert.tsx
@@ -0,0 +1,75 @@
+import app from 'flarum/forum/app';
+import Button from 'flarum/common/components/Button';
+import listItems from 'flarum/common/helpers/listItems';
+import AcceptPoliciesModal from './AcceptPoliciesModal';
+
+let temporarilyHidden = false;
+
+/**
+ * Renders similarly to Flarum's Alert, but with an additional .container inside
+ */
+export default class UpdateAlert {
+  shouldShowAlert() {
+    if (temporarilyHidden) {
+      return false;
+    }
+
+    const { user } = app.session;
+
+    return user && user.fofTermsPoliciesHasUpdate();
+  }
+
+  hasOnlyOptionalUpdates() {
+    const { user } = app.session;
+    return user && !user.fofTermsPoliciesMustAccept() && user.fofTermsPoliciesHasUpdate();
+  }
+
+  view() {
+    const { user } = app.session;
+
+    if (!this.shouldShowAlert() || !user) {
+      return null;
+    }
+
+    const controls = [
+      <Button
+        className="Button Button--link"
+        onclick={() => {
+          app.modal.show(AcceptPoliciesModal);
+        }}
+      >
+        {app.translator.trans('fof-terms.forum.update-alert.review')}
+      </Button>,
+    ];
+
+    const dismissControl = [];
+
+    if (!user.fofTermsPoliciesMustAccept()) {
+      dismissControl.push(
+        <Button
+          icon="fas fa-times"
+          className="Button Button--link Button--icon Alert-dismiss"
+          onclick={() => {
+            temporarilyHidden = true;
+          }}
+          aria-label={app.translator.trans('fof-terms.forum.update-alert.close')}
+        />
+      );
+    }
+
+    return (
+      <div className="Alert Alert-info">
+        <div className="container">
+          <span className="Alert-body">
+            {this.hasOnlyOptionalUpdates()
+              ? app.translator.trans('fof-terms.forum.update-alert.can-accept-optional-message')
+              : user.fofTermsPoliciesMustAccept()
+                ? app.translator.trans('fof-terms.forum.update-alert.must-accept-message')
+                : app.translator.trans('fof-terms.forum.update-alert.can-accept-message')}
+          </span>
+          <ul className="Alert-controls">{listItems(controls.concat(dismissControl))}</ul>
+        </div>
+      </div>
+    );
+  }
+}