Ref T3116. This creates a policy rule where you can require a signature on a given legalpad document.
NOTE: signatures must be for the *latest* document version.
Details
Details
- Reviewers
epriestley - Maniphest Tasks
- Restricted Maniphest Task
- Commits
- Restricted Diffusion Commit
rPd740374cca7a: Legalpad - add policy rule for legalpad document signatures
made a task have a custom policy requiring a legalpad signature. verified non-signers were locked out.
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
| src/applications/policy/rule/PhabricatorPolicyRuleLegalpadSignature.php | ||
|---|---|---|
| 25 | there's a few keys on this table, but the one with document phid is currently document phid, document version, signer phid I guess I could (and probably should) re-jigger this so document version is the 3rd value. I think queries on document phid and signer phid (with no version) will be helpful so we can say things lke "you signed a previous version of this document and must sign again" while still getting the really fast 3 column query. | |
Comment Actions
Neat!
| src/applications/policy/rule/PhabricatorPolicyRuleLegalpadSignature.php | ||
|---|---|---|
| 21–22 | Maybe we should bundle a needViewerSignatures() into this, seems like we must be doing similar queries elsewhere (or could benefit from doing them)? | |
| 25 | If this was part of the DocumentQuery it could at least add a ... AND documentPHID IN (%Ls) clause to get most of the benefit of the key. | |
Comment Actions
Oh, one thought: this rule/logic should probably be AND, not OR. That is, it seems reasonable to want to say "You have to sign the Invention Assignment Agreement and the Contracting Agreement".
Comment Actions
- re-jigger the key
- add "withSignerPHIDs" to query class. this filters out documents that don't have the full set of signer phids from the result set
- use "withSignerPHIDs" in the policy rule object
- update a query on the "sign" controller to specify key columns in proper order
| src/applications/legalpad/query/LegalpadDocumentQuery.php | ||
|---|---|---|
| 90 | gotta add something to check for versions matching, whoops | |
Comment Actions
fix the document version thing and this generally not actually working (I got "lucky" on my re-factor at first and my quick tests were passing and they should not have been)
re-tested extensively by toggling the custom policy between various legalpad documents (1 and N) and verifying proper access or lack of access