From 4fb3b27f1d2aae902f4ca50be63227c50e21fbf0 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Wed, 10 Jul 2013 11:46:39 -0700 Subject: [PATCH] Legalpad - add signature page Summary: Fixes T3481. Sort of - this thing be very ugly. Also it assumes that you'll "always" want to sign terms. I was thinking in a future diff that should be optional as well as configurable, though it was unclear to me if either was worth pursuing... Generally very hideous as the three main elements (PHUIDocument, AphrontErrorView, and AphrontForm with an AphrontFormInset) have never really played together before. Test Plan: agreed to some test terms. noted UI displayed nicely. reloaded and noted UI told me I had signed it already. Went to different terms and filled them out wrong and got sensical errors. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T3481 Differential Revision: https://secure.phabricator.com/D6399 --- .../patches/20130709.legalpadsignature.sql | 2 + src/__phutil_library_map__.php | 2 + .../PhabricatorApplicationLegalpad.php | 2 +- .../LegalpadDocumentListController.php | 4 +- .../LegalpadDocumentSignController.php | 236 ++++++++++++++++++ .../LegalpadDocumentViewController.php | 3 - .../editor/LegalpadDocumentEditor.php | 2 +- .../legalpad/storage/LegalpadDocumentBody.php | 1 - .../storage/LegalpadDocumentSignature.php | 13 +- .../patch/PhabricatorBuiltinPatchList.php | 4 + 10 files changed, 260 insertions(+), 9 deletions(-) create mode 100644 resources/sql/patches/20130709.legalpadsignature.sql create mode 100644 src/applications/legalpad/controller/LegalpadDocumentSignController.php diff --git a/resources/sql/patches/20130709.legalpadsignature.sql b/resources/sql/patches/20130709.legalpadsignature.sql new file mode 100644 index 0000000000..5f6380e4d8 --- /dev/null +++ b/resources/sql/patches/20130709.legalpadsignature.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_legalpad.legalpad_documentsignature + ADD signatureData LONGTEXT NOT NULL COLLATE utf8_bin AFTER signerPHID; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 641dc4fa29..47ecf785b8 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -644,6 +644,7 @@ phutil_register_library_map(array( 'LegalpadDocumentPreviewController' => 'applications/legalpad/controller/LegalpadDocumentPreviewController.php', 'LegalpadDocumentQuery' => 'applications/legalpad/query/LegalpadDocumentQuery.php', 'LegalpadDocumentSearchEngine' => 'applications/legalpad/query/LegalpadDocumentSearchEngine.php', + 'LegalpadDocumentSignController' => 'applications/legalpad/controller/LegalpadDocumentSignController.php', 'LegalpadDocumentSignature' => 'applications/legalpad/storage/LegalpadDocumentSignature.php', 'LegalpadDocumentViewController' => 'applications/legalpad/controller/LegalpadDocumentViewController.php', 'LegalpadMockMailReceiver' => 'applications/legalpad/mail/LegalpadMockMailReceiver.php', @@ -2573,6 +2574,7 @@ phutil_register_library_map(array( 'LegalpadDocumentPreviewController' => 'LegalpadController', 'LegalpadDocumentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'LegalpadDocumentSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'LegalpadDocumentSignController' => 'LegalpadController', 'LegalpadDocumentSignature' => 'LegalpadDAO', 'LegalpadDocumentViewController' => 'LegalpadController', 'LegalpadMockMailReceiver' => 'PhabricatorObjectMailReceiver', diff --git a/src/applications/legalpad/application/PhabricatorApplicationLegalpad.php b/src/applications/legalpad/application/PhabricatorApplicationLegalpad.php index 10044127e4..913d5d2d26 100644 --- a/src/applications/legalpad/application/PhabricatorApplicationLegalpad.php +++ b/src/applications/legalpad/application/PhabricatorApplicationLegalpad.php @@ -40,7 +40,7 @@ final class PhabricatorApplicationLegalpad extends PhabricatorApplication { public function getRoutes() { return array( - '/L(?P\d+)/' => 'LegalpadDocumentViewController', + '/L(?P\d+)' => 'LegalpadDocumentSignController', '/legalpad/' => array( '' => 'LegalpadDocumentListController', '(query/(?P[^/]+)/)?' => 'LegalpadDocumentListController', diff --git a/src/applications/legalpad/controller/LegalpadDocumentListController.php b/src/applications/legalpad/controller/LegalpadDocumentListController.php index 47d4f63c47..4d0f6ea43a 100644 --- a/src/applications/legalpad/controller/LegalpadDocumentListController.php +++ b/src/applications/legalpad/controller/LegalpadDocumentListController.php @@ -37,8 +37,8 @@ final class LegalpadDocumentListController extends LegalpadController $list->setUser($user); foreach ($documents as $document) { $last_updated = phabricator_date($document->getDateModified(), $user); - $updater = $this->getHandle( - reset($document->getRecentContributorPHIDs()))->renderLink(); + $recent_contributors = $document->getRecentContributorPHIDs(); + $updater = $this->getHandle(reset($recent_contributors))->renderLink(); $title = $document->getTitle(); diff --git a/src/applications/legalpad/controller/LegalpadDocumentSignController.php b/src/applications/legalpad/controller/LegalpadDocumentSignController.php new file mode 100644 index 0000000000..5841adebfe --- /dev/null +++ b/src/applications/legalpad/controller/LegalpadDocumentSignController.php @@ -0,0 +1,236 @@ +id = $data['id']; + } + + public function processRequest() { + $request = $this->getRequest(); + $user = $request->getUser(); + + $document = id(new LegalpadDocumentQuery()) + ->setViewer($user) + ->withIDs(array($this->id)) + ->needDocumentBodies(true) + ->executeOne(); + + if (!$document) { + return new Aphront404Response(); + } + + $signature = id(new LegalpadDocumentSignature()) + ->loadOneWhere( + 'documentPHID = %s AND documentVersion = %d AND signerPHID = %s', + $document->getPHID(), + $document->getVersions(), + $user->getPHID()); + + if (!$signature) { + $has_signed = false; + $error_view = null; + $signature = id(new LegalpadDocumentSignature()) + ->setSignerPHID($user->getPHID()) + ->setDocumentPHID($document->getPHID()) + ->setDocumentVersion($document->getVersions()); + $data = array( + 'name' => $user->getRealName(), + 'email' => $user->loadPrimaryEmailAddress()); + $signature->setSignatureData($data); + } else { + $has_signed = true; + $error_view = id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) + ->setTitle(pht('You have already agreed to these terms.')); + $data = $signature->getSignatureData(); + } + + $e_name = true; + $e_email = true; + $e_address_1 = true; + $errors = array(); + if ($request->isFormPost()) { + $name = $request->getStr('name'); + $email = $request->getStr('email'); + $address_1 = $request->getStr('address_1'); + $address_2 = $request->getStr('address_2'); + $phone = $request->getStr('phone'); + $agree = $request->getExists('agree'); + + if (!$name) { + $e_name = pht('Required'); + $errors[] = pht('Name field is required.'); + } + $data['name'] = $name; + + if (!$email) { + $e_email = pht('Required'); + $errors[] = pht('Email field is required.'); + } else { + $addr_obj = new PhutilEmailAddress($email); + $domain = $addr_obj->getDomainName(); + if (!$domain) { + $e_email = pht('Invalid'); + $errors[] = pht('A valid email is required.'); + } + } + $data['email'] = $email; + + if (!$address_1) { + $e_address_1 = pht('Required'); + $errors[] = pht('Address line 1 field is required.'); + } + $data['address_1'] = $address_1; + $data['address_2'] = $address_2; + $data['phone'] = $phone; + $signature->setSignatureData($data); + + if (!$agree) { + $errors[] = pht( + 'You must check "I agree to the terms laid forth above."'); + } + + if (!$errors) { + $signature->save(); + $has_signed = true; + $error_view = id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) + ->setTitle(pht('Signature successful. Thank you.')); + } else { + $error_view = id(new AphrontErrorView()) + ->setTitle(pht('Error in submission.')) + ->setErrors($errors); + } + } + + $document_body = $document->getDocumentBody(); + $engine = id(new PhabricatorMarkupEngine()) + ->setViewer($user); + $engine->addObject( + $document_body, + LegalpadDocumentBody::MARKUP_FIELD_TEXT); + $engine->process(); + + $title = $document_body->getTitle(); + + $header = id(new PhabricatorHeaderView()) + ->setHeader($title); + + $content = array( + id(new PHUIDocumentView()) + ->setHeader($header) + ->appendChild($this->buildDocument($engine, $document_body)), + $error_view, + $this->buildSignatureForm( + $document_body, + $signature, + $has_signed, + $e_name, + $e_email, + $e_address_1)); + + return $this->buildApplicationPage( + $content, + array( + 'title' => $title, + 'device' => true, + 'dust' => true, + 'pageObjects' => array($document->getPHID()), + )); + } + + private function buildDocument( + PhabricatorMarkupEngine + $engine, LegalpadDocumentBody $body) { + + require_celerity_resource('legalpad-documentbody-css'); + + return phutil_tag( + 'div', + array( + 'class' => 'legalpad-documentbody' + ), + $engine->getOutput($body, LegalpadDocumentBody::MARKUP_FIELD_TEXT)); + + } + + private function buildSignatureForm( + LegalpadDocumentBody $body, + LegalpadDocumentSignature $signature, + $has_signed = false, + $e_name = true, + $e_email = true, + $e_address_1 = true) { + + $user = $this->getRequest()->getUser(); + if ($has_signed) { + $instructions = pht('Thank you for signing and agreeing.'); + } else { + $instructions = pht('Please enter the following information.'); + } + + $data = $signature->getSignatureData(); + $form = id(new AphrontFormView()) + ->setUser($user) + ->setFlexible(true) + ->appendChild( + id(new AphrontFormInsetView()) + ->setTitle(pht('Sign and Agree')) + ->setDescription($instructions) + ->setContent(phutil_tag('br', array())) + ->appendChild( + id(new AphrontFormTextControl()) + ->setLabel(pht('Name')) + ->setValue(idx($data, 'name', '')) + ->setName('name') + ->setError($e_name) + ->setDisabled($has_signed)) + ->appendChild( + id(new AphrontFormTextControl()) + ->setLabel(pht('Email')) + ->setValue(idx($data, 'email', '')) + ->setName('email') + ->setError($e_email) + ->setDisabled($has_signed)) + ->appendChild( + id(new AphrontFormTextControl()) + ->setLabel(pht('Address line 1')) + ->setValue(idx($data, 'address_1', '')) + ->setName('address_1') + ->setError($e_address_1) + ->setDisabled($has_signed)) + ->appendChild( + id(new AphrontFormTextControl()) + ->setLabel(pht('Address line 2')) + ->setValue(idx($data, 'address_2', '')) + ->setName('address_2') + ->setDisabled($has_signed)) + ->appendChild( + id(new AphrontFormTextControl()) + ->setLabel(pht('Phone')) + ->setValue(idx($data, 'phone', '')) + ->setName('phone') + ->setDisabled($has_signed)) + ->appendChild( + id(new AphrontFormCheckboxControl()) + ->addCheckbox( + 'agree', + 'agree', + pht('I agree to the terms laid forth above.'), + $has_signed) + ->setDisabled($has_signed)) + ->appendChild( + id(new AphrontFormSubmitControl()) + ->setValue(pht('Sign and Agree')) + ->setDisabled($has_signed))); + + return $form; + } + +} diff --git a/src/applications/legalpad/controller/LegalpadDocumentViewController.php b/src/applications/legalpad/controller/LegalpadDocumentViewController.php index 6ac821505a..5e49dad15f 100644 --- a/src/applications/legalpad/controller/LegalpadDocumentViewController.php +++ b/src/applications/legalpad/controller/LegalpadDocumentViewController.php @@ -47,9 +47,6 @@ final class LegalpadDocumentViewController extends LegalpadController { $engine = id(new PhabricatorMarkupEngine()) ->setViewer($user); - $engine->addObject( - $document_body, - LegalpadDocumentBody::MARKUP_FIELD_TITLE); $engine->addObject( $document_body, LegalpadDocumentBody::MARKUP_FIELD_TEXT); diff --git a/src/applications/legalpad/editor/LegalpadDocumentEditor.php b/src/applications/legalpad/editor/LegalpadDocumentEditor.php index 4f4fb40f1b..c8070a0b9e 100644 --- a/src/applications/legalpad/editor/LegalpadDocumentEditor.php +++ b/src/applications/legalpad/editor/LegalpadDocumentEditor.php @@ -169,7 +169,7 @@ final class LegalpadDocumentEditor $body->addTextSection( pht('DOCUMENT DETAIL'), - PhabricatorEnv::getProductionURI('/L'.$object->getID())); + PhabricatorEnv::getProductionURI('/legalpad/view/'.$object->getID().'/')); return $body; } diff --git a/src/applications/legalpad/storage/LegalpadDocumentBody.php b/src/applications/legalpad/storage/LegalpadDocumentBody.php index b2060d3c5f..7c658595c4 100644 --- a/src/applications/legalpad/storage/LegalpadDocumentBody.php +++ b/src/applications/legalpad/storage/LegalpadDocumentBody.php @@ -7,7 +7,6 @@ final class LegalpadDocumentBody extends LegalpadDAO implements PhabricatorMarkupInterface { - const MARKUP_FIELD_TITLE = 'markup:title'; const MARKUP_FIELD_TEXT = 'markup:text '; protected $phid; diff --git a/src/applications/legalpad/storage/LegalpadDocumentSignature.php b/src/applications/legalpad/storage/LegalpadDocumentSignature.php index b1d6fff997..bf04ce7307 100644 --- a/src/applications/legalpad/storage/LegalpadDocumentSignature.php +++ b/src/applications/legalpad/storage/LegalpadDocumentSignature.php @@ -6,7 +6,18 @@ final class LegalpadDocumentSignature extends LegalpadDAO { protected $documentPHID; - protected $documentversion; + protected $documentVersion; protected $signerPHID; + protected $signatureData = array(); + + public function getConfiguration() { + return array( + self::CONFIG_SERIALIZATION => array( + 'signatureData' => self::SERIALIZATION_JSON, + ), + ) + parent::getConfiguration(); + } + + } diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index 050595b620..db8daf6016 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1430,6 +1430,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'php', 'name' => $this->getPatchPath('20130703.legalpaddocdenorm.php'), ), + '20130709.legalpadsignature.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('20130709.legalpadsignature.sql'), + ), '20130709.droptimeline.sql' => array( 'type' => 'sql', 'name' => $this->getPatchPath('20130709.droptimeline.sql'), -- 2.51.2