Page MenuHomePhabricator

Generalize Legalpad validation logic for "Require Signature"
ClosedPublic

Authored by epriestley on Mar 22 2019, 4:27 PM.
Tags
None
Referenced Files
F15468422: D20311.id48465.diff
Fri, Apr 4, 12:31 AM
F15456659: D20311.id48484.diff
Sun, Mar 30, 11:08 AM
F15453567: D20311.id.diff
Sat, Mar 29, 12:41 PM
F15434636: D20311.diff
Tue, Mar 25, 4:45 AM
F15376887: D20311.diff
Thu, Mar 13, 6:25 AM
Unknown Object (File)
Mar 3 2025, 8:08 AM
Unknown Object (File)
Feb 20 2025, 2:14 AM
Unknown Object (File)
Feb 14 2025, 8:04 PM
Subscribers
None

Details

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.

Diff Detail

Repository
rP Phabricator
Branch
legalpad1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 22329
Build 30550: Run Core Tests
Build 30549: arc lint + arc unit