@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Generalize Legalpad validation logic for "Require Signature"

Summary:
See downstream <https://phabricator.wikimedia.org/T208254>.

I can't actually reproduce any issue here (we only show this field when creating a document, and only if the viewer is an administrator), so maybe this relied on some changes or was originally reported against older code.

Regardless, the validation isn't quite right: it requires administrator privileges to apply this transaction at all, but should only require administrator privileges to change the value.

Test Plan:
Edited Legalpad documents as an administrator and non-administrator before and after the change, with and without signatures being required.

Couldn't reproduce the original issue, but this version is generally more correct/robust.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20311

+15 -4
+15 -4
src/applications/legalpad/xaction/LegalpadDocumentRequireSignatureTransaction.php
··· 55 55 public function validateTransactions($object, array $xactions) { 56 56 $errors = array(); 57 57 58 - $is_admin = $this->getActor()->getIsAdmin(); 58 + $old = (bool)$object->getRequireSignature(); 59 + foreach ($xactions as $xaction) { 60 + $new = (bool)$xaction->getNewValue(); 59 61 60 - if (!$is_admin) { 61 - $errors[] = $this->newInvalidError( 62 - pht('Only admins may require signature.')); 62 + if ($old === $new) { 63 + continue; 64 + } 65 + 66 + $is_admin = $this->getActor()->getIsAdmin(); 67 + if (!$is_admin) { 68 + $errors[] = $this->newInvalidError( 69 + pht( 70 + 'Only administrators may change whether a document '. 71 + 'requires a signature.'), 72 + $xaction); 73 + } 63 74 } 64 75 65 76 return $errors;