From c3809b0d593d25a53d12f5502d53a78b82e16ce1 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Fri, 28 Oct 2016 13:55:55 -0700 Subject: [PATCH 01/41] Add a Merchant logo to Phortune Summary: Is a logo. For merchants. Test Plan: Set a new logo, remove it. See on list. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T7607 Differential Revision: https://secure.phabricator.com/D16751 --- resources/builtin/merchant.png | Bin 0 -> 5344 bytes .../20161025.phortune.merchant.image.1.sql | 2 + src/__phutil_library_map__.php | 2 + .../PhabricatorPhortuneApplication.php | 1 + .../PhortuneMerchantPictureController.php | 234 ++++++++++++++++++ .../PhortuneMerchantViewController.php | 11 +- .../editor/PhortuneMerchantEditor.php | 8 + .../phortune/query/PhortuneMerchantQuery.php | 35 +++ .../query/PhortuneMerchantSearchEngine.php | 5 +- .../phortune/storage/PhortuneMerchant.php | 20 ++ .../storage/PhortuneMerchantTransaction.php | 1 + 11 files changed, 316 insertions(+), 3 deletions(-) create mode 100644 resources/builtin/merchant.png create mode 100644 resources/sql/autopatches/20161025.phortune.merchant.image.1.sql create mode 100644 src/applications/phortune/controller/PhortuneMerchantPictureController.php diff --git a/resources/builtin/merchant.png b/resources/builtin/merchant.png new file mode 100644 index 0000000000000000000000000000000000000000..c8381eb00fd59eff93fb0a28242ad81f360331d4 GIT binary patch literal 5344 zcmaJ_c|4R|`=4QueJqu|N!B!G!7v&-qwEZ!vJ5jA%M4>=Y#D2WlI+S>90RW5+-5>AnL!?36h@K>}mh?(2(((2}-SGl!e|8xXxn zXF{n&T&RVud#H~)njo#C4Z+YgnGF1hG(3dv=SvRMq-#n4#aEL#{xuDghWtfB^U;$2 zTPX)~D~JJwN`$B>qoD2x6(mF*t&C7ZBQ;b{L6C3+3If}g*+`M-?Gf&Vy)=`t7{?+-&L!(o1YzwG*p zI*?{V{C_q6D|Mi4h(8f#Lky$@QQevG@Hp`s%=F#=7W9jdsg0&3mBb7R-q(=g9^^+P z)6N=dNi)AF6G#M2HzJXs;el3zGJAEXDvF>6bweQt%tjTduA-`rCc@o*^Z9Rh4Rw8W z1VSB-R#C+w5Jo5seUvIzMMG6jRm}jU0oVJDJxdOx;mPjA-?B+e*?(hE*#C*uG@ugk zGz!(0Lh=2b1y)`Z8YR$+;tw$}_!V_Eh=MuZokadMsrV~Uf7@+Hq>?TX30Nw{5Av6% zHA(+P3ltiORwbyRpl~F!QFX%;p=b>c1k~LfsiA=>PRTsjmeGa88j4)@KA@kxuf77cr`Ri1+Oj*W2y-Il|28EOuwg?8TxDZ zZ)q@J{w*#dnORX(X6X!Fxpe>lfLG5N>eOYs?&%0^@damSEKY6v+5AZz3 z?jjlZWc;9ObnKeS(7tnBNh3@Vj9JC&Ut_0n=ckdp_11(tS!mmVRSoqP>zO`$zu0whQn^pq{NzyvCehUi8}4E31O-(WPUjutSqcb{?H@ zk`(w3kT;pNm0EPJBI9yF&mg;|IWI=n`hs6Mn{ihK%d|!xuXms2g?JG`XP0uo8^E2L z6$*FjCcLat2v)2x-Haowm>;q}>8~muEe*fG;2MCD3%9wG58s^l;QHX&oWwg4!44;~ zSczA4FR~3yEF4DlkRFRiVuWep!@Z)B@gX%cS3vG0w&d-vn=XtmSE{{DVo zbI{TyB{+Pw+PS7ipsTE+B9J{56n5g}P^o3mpKBwA8)c8? ze|B9zb}TJ3bELGatoxYuy6bR5rNI@xrq<9+O&$8`6t3MfYz5xP@-S&|#77{@<)qEFP~OcwQk<43Bwy|Fz;`@UN2iTi_M$R>&*eNX z$M*1=2WUJyKn_yl)Ql>&t2nm^l4+iM;P=k?T|KeSN?Kj+vgZYOW~8RlqnVAjV*0~h z-#vWq@iO;tL3Z{|&gyO|Fzx4O^E^}po85Wd`6Crsts^XkW)owt^{Q73P=0T#PX{ma z*yb=cSXo%=3%k0*m#Q9e0lxis?U}x_yE`(o5E2ri+|<-mg@2(=o@`lU)iQOpu&|iA zDQ_7*oXoxdB9YRzzh$71#SM>jRGB^ju^8zVZ+y$m81{SrCyokqsAfQH z$Bblp@*%8RT-xGH>y#iJ_|O%3YZ;RR1|lPV?R=J`j48G5@s zCHQjM#{A0_1z^XmcEXmoVsg11H&<_Ry%IF>iQEmhrha5~AxrWN1%|#}LE`=9l5uw~ z9@2`HqoZPaR+c&{f!bMFRpsZ)<>*mGFWA|kS4tENL_bB!9fr7t%eva0gbEm6mui)G z3`-R?RdJ3%FbiySzT#Y!n53|9x`?1){hiv{*Kv$>lb;`o2a17;kvrIp~0ac zt&-c?ymwwSnjt-2WX}o92FSimc#Z=pg~b=)N-gg3OJQE;zzZknxy(9fG%_}ZZLO|$ zr%yI8CF)GHhK;`els_mcdHgoKy1F{V+S+=}v;Wp&awwcpta$Pyb!&ayq-61vBgUv! zPQ)vH|Hn4RQ1tOoV;dVAMpGCE2gkvoeCbPDVgq49f`U({Hn?yfM76_rypK3O(`KVB zo~_Us^92Eo1{`>go_Djh&JLPZ#dC>HVDhmd?z&7SET=>Px@hJKG$goTVq_FoOMd?- zDs%TrcXxMHGDFqEgieh&;p3^ zUOBCXOr%DU{j80TSj}SQ4(kp_M`386Uh{gl7}#P?oVXK_bOd@e1Slvb_Q>)S%W&vf z2Zu2%77b3v&wtqvgxo#<#G6YggW3sPDiT{XU=NoS7K<}Vp&48De*gZxs_^=m>5w!~ zOO;zsn(V-Qr*O*M!q-o(R=}Q@*CMij3MumE?{1Uf*|W*jKxLk!#KiCN=x1i;%tf&+ zGbcxMW^vIGhNcicFFG4II_7xHzsM=jkDDzG2~a-FKk!CZZ?!8rI{IN%osU+~=V93H z*4I4NwvW@!P_!&=VPPTbh?LZ2?i86odWV&bO>rD{6ZuVEOLVS`As8`HY?O4Zm?wop z-i(gT$#v8qPc&v=CyNm3eOq6~heq@J-4-`@BQr8HQS@p-7|_Z7o6X!y4QW5m=r#ih~z z$-(eF-20C+GsXV5O-rPLuJ9jQE;qcwED(~lr__TyObFGN6=H>Z7#JK3n+h1xj@bQ@ zWvg@~qo==rYjtz;eHW5XW+-BeJG(rsyrLqGjr&OQm8(}3k6v1y`2HnDT>YC#hw`Uz z#+9!Zot#!mN=ujGEVaKAhwUsx-MWPl6dT~+n!I?|#M2an zRy-tLxFwNS?VWkWZen60_Ic7#N=7qELu2REOZO31*NL%Eh2Zy0SL+agF?ODTppcrB z5of7qR@7oio^vLFmWPzy%!1>MyodfUuWB6w0u^WII`)~KXHxm3Fd{O+(66jTmId0+ zOXiyK=8tSdNd>&VkLTNFs7N_N1=1~eBeph!f=}=l)+N3p0 zH}k-Rk~n?oMbez4)1R6bq(706dG%l3q{dw8P|W84BMfWRYNQTyZ zyJLKvU7@O8hPdKgO{6$GOKF@vorj7q9%xt6CuM!t!eHw0^T8;!YhHKD%TFH1V8Yp| z_5i)mvIo)Wy_RAf`xnJK?)!{#0fE3I-&PuR_Z!LOD(lP6J~_uJMSMv~Ne%6D+upjV z@h6`L_Z-rCOwG)AY>Qbx_eLznf|nyE{+yd}JAeNCX@w1`GEx5Arkeu;1Bk$d{{4+) zQ56sU6OBRy1)GX4=9x&^uc(SQub_Ry6h1-^ITyHrEev) zRlBvNWx_6skL7c(`*+EQ+Rr#4(4R2YHvZ)1xEBp>s7ab}uJ9DgpfYapt@N)h4WLAEz_dhzS=qV^@3y7>3K&r=S-y$kQ_GP|;F+-D=3 zWB%mue~`g*y_BB=$bbBi^p!M3hRvbz1J9oObIe_h*OeJhufpjJHV{c<2?3efyd{0c z-uEt=C5nCT>P_DzhwDHfq@%F@aCmq)h(e*@`){FB?20AoqN6je<=>oX)?is@;2wLM z)9$jzjD3^cor>WX1Kvp5?2#F;x8d3opCYxKBWTR~hKHD$@@?l}X?KfE-1-Qn+!ZEi z6u~Cz!zF+M&T!`e&ncdSx6gGgi^aJ`&YGNL(>tzUonAQA`z4$YTY2OAp1fjyDcP0S#uBpViXZfmVxZ1vY7>I>+R<%0%J*hR1DP>{NA+$jq3la9^u zA42jLwzr1Otz&h!g$FDN9`%C~D%-7u>J~5P`;+XbHJ%kp;00k6C^+Pi5JJVq*uzHvk~c>*ER$SV?}duE(eE zj{1PuwY~kbo~lf`i^i%$ic4*AjKOU+z#abD!7HaL95EfajMwosT{0ZwN+~i*ug-Df z9spvG2%DuPeE}!O30E|hKdD7QSnmDd<$O6rq%ltt5T(?|p*UOZ@lx~wJ1BA9E|OhP z%739&(YgD&`%gRiV?8Wur-L)g7z=m+=cO4syVQ@tBQ8>3fk^Y)KdlK_D;iG7cju9$+mx{-(SuLgLE9BbRSP%jrB zCQ#X2NYAcqQ{$0gGxTKDRR-|O`B+C5?~w!7CD+12o`oS3qLy!LLf5`&F0;>krWm_Y zrk3033GeJn_;hYq<;w5z3oIKvPkG)2At|0#cDrzp{jk%6foc@m2n?7Iz>!scGyvcPkO2+$wkTRrN4!OssVeD^p0v{*LyT?jgFx$WG z$lwQB3hvxK7UtLMEcP^|ur)To9{iBJmLN~|ot_4bsMv{IUX2vW=g3YY;r)8o^XkSRIff&;F} WC2As(@YP@c9M2kA7*^}MM*koGgm{tw literal 0 HcmV?d00001 diff --git a/resources/sql/autopatches/20161025.phortune.merchant.image.1.sql b/resources/sql/autopatches/20161025.phortune.merchant.image.1.sql new file mode 100644 index 0000000000..995a4a8fd6 --- /dev/null +++ b/resources/sql/autopatches/20161025.phortune.merchant.image.1.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_phortune.phortune_merchant + ADD profileImagePHID VARBINARY(64); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index cb4a6c15df..dcb61c6342 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4185,6 +4185,7 @@ phutil_register_library_map(array( 'PhortuneMerchantInvoiceCreateController' => 'applications/phortune/controller/PhortuneMerchantInvoiceCreateController.php', 'PhortuneMerchantListController' => 'applications/phortune/controller/PhortuneMerchantListController.php', 'PhortuneMerchantPHIDType' => 'applications/phortune/phid/PhortuneMerchantPHIDType.php', + 'PhortuneMerchantPictureController' => 'applications/phortune/controller/PhortuneMerchantPictureController.php', 'PhortuneMerchantQuery' => 'applications/phortune/query/PhortuneMerchantQuery.php', 'PhortuneMerchantSearchEngine' => 'applications/phortune/query/PhortuneMerchantSearchEngine.php', 'PhortuneMerchantTransaction' => 'applications/phortune/storage/PhortuneMerchantTransaction.php', @@ -9434,6 +9435,7 @@ phutil_register_library_map(array( 'PhortuneMerchantInvoiceCreateController' => 'PhortuneMerchantController', 'PhortuneMerchantListController' => 'PhortuneMerchantController', 'PhortuneMerchantPHIDType' => 'PhabricatorPHIDType', + 'PhortuneMerchantPictureController' => 'PhortuneMerchantController', 'PhortuneMerchantQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhortuneMerchantSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhortuneMerchantTransaction' => 'PhabricatorApplicationTransaction', diff --git a/src/applications/phortune/application/PhabricatorPhortuneApplication.php b/src/applications/phortune/application/PhabricatorPhortuneApplication.php index 470ceb10ab..313328e41e 100644 --- a/src/applications/phortune/application/PhabricatorPhortuneApplication.php +++ b/src/applications/phortune/application/PhabricatorPhortuneApplication.php @@ -81,6 +81,7 @@ final class PhabricatorPhortuneApplication extends PhabricatorApplication { ), 'merchant/' => array( '(?:query/(?P[^/]+)/)?' => 'PhortuneMerchantListController', + 'picture/(?:(?P\d+)/)?' => 'PhortuneMerchantPictureController', 'edit/(?:(?P\d+)/)?' => 'PhortuneMerchantEditController', 'orders/(?P\d+)/(?:query/(?P[^/]+)/)?' => 'PhortuneCartListController', diff --git a/src/applications/phortune/controller/PhortuneMerchantPictureController.php b/src/applications/phortune/controller/PhortuneMerchantPictureController.php new file mode 100644 index 0000000000..74d7ea061c --- /dev/null +++ b/src/applications/phortune/controller/PhortuneMerchantPictureController.php @@ -0,0 +1,234 @@ +getViewer(); + $id = $request->getURIData('id'); + + $merchant = id(new PhortuneMerchantQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->needProfileImage(true) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->executeOne(); + if (!$merchant) { + return new Aphront404Response(); + } + + $uri = $merchant->getViewURI(); + + $supported_formats = PhabricatorFile::getTransformableImageFormats(); + $e_file = true; + $errors = array(); + + if ($request->isFormPost()) { + $phid = $request->getStr('phid'); + $is_default = false; + if ($phid == PhabricatorPHIDConstants::PHID_VOID) { + $phid = null; + $is_default = true; + } else if ($phid) { + $file = id(new PhabricatorFileQuery()) + ->setViewer($viewer) + ->withPHIDs(array($phid)) + ->executeOne(); + } else { + if ($request->getFileExists('picture')) { + $file = PhabricatorFile::newFromPHPUpload( + $_FILES['picture'], + array( + 'authorPHID' => $viewer->getPHID(), + 'canCDN' => true, + )); + } else { + $e_file = pht('Required'); + $errors[] = pht( + 'You must choose a file when uploading a merchant logo.'); + } + } + + if (!$errors && !$is_default) { + if (!$file->isTransformableImage()) { + $e_file = pht('Not Supported'); + $errors[] = pht( + 'This server only supports these image formats: %s.', + implode(', ', $supported_formats)); + } else { + $xform = PhabricatorFileTransform::getTransformByKey( + PhabricatorFileThumbnailTransform::TRANSFORM_PROFILE); + $xformed = $xform->executeTransform($file); + } + } + + if (!$errors) { + if ($is_default) { + $new_value = null; + } else { + $xformed->attachToObject($merchant->getPHID()); + $new_value = $xformed->getPHID(); + } + + $xactions = array(); + $xactions[] = id(new PhortuneMerchantTransaction()) + ->setTransactionType(PhortuneMerchantTransaction::TYPE_PICTURE) + ->setNewValue($new_value); + + $editor = id(new PhortuneMerchantEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true); + + $editor->applyTransactions($merchant, $xactions); + + return id(new AphrontRedirectResponse())->setURI($uri); + } + } + + $title = pht('Edit Merchant Picture'); + + $form = id(new PHUIFormLayoutView()) + ->setUser($viewer); + + $default_image = PhabricatorFile::loadBuiltin($viewer, 'merchant.png'); + + $images = array(); + + $current = $merchant->getProfileImagePHID(); + $has_current = false; + if ($current) { + $file = id(new PhabricatorFileQuery()) + ->setViewer($viewer) + ->withPHIDs(array($current)) + ->executeOne(); + if ($file) { + if ($file->isTransformableImage()) { + $has_current = true; + $images[$current] = array( + 'uri' => $file->getBestURI(), + 'tip' => pht('Current Picture'), + ); + } + } + } + + $images[PhabricatorPHIDConstants::PHID_VOID] = array( + 'uri' => $default_image->getBestURI(), + 'tip' => pht('Default Picture'), + ); + + require_celerity_resource('people-profile-css'); + Javelin::initBehavior('phabricator-tooltips', array()); + + $buttons = array(); + foreach ($images as $phid => $spec) { + $button = javelin_tag( + 'button', + array( + 'class' => 'grey profile-image-button', + 'sigil' => 'has-tooltip', + 'meta' => array( + 'tip' => $spec['tip'], + 'size' => 300, + ), + ), + phutil_tag( + 'img', + array( + 'height' => 50, + 'width' => 50, + 'src' => $spec['uri'], + ))); + + $button = array( + phutil_tag( + 'input', + array( + 'type' => 'hidden', + 'name' => 'phid', + 'value' => $phid, + )), + $button, + ); + + $button = phabricator_form( + $viewer, + array( + 'class' => 'profile-image-form', + 'method' => 'POST', + ), + $button); + + $buttons[] = $button; + } + + if ($has_current) { + $form->appendChild( + id(new AphrontFormMarkupControl()) + ->setLabel(pht('Current Logo')) + ->setValue(array_shift($buttons))); + } + + $form->appendChild( + id(new AphrontFormMarkupControl()) + ->setLabel(pht('Use Logo')) + ->setValue($buttons)); + + $form_box = id(new PHUIObjectBoxView()) + ->setHeaderText($title) + ->setFormErrors($errors) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->setForm($form); + + $upload_form = id(new AphrontFormView()) + ->setUser($viewer) + ->setEncType('multipart/form-data') + ->appendChild( + id(new AphrontFormFileControl()) + ->setName('picture') + ->setLabel(pht('Upload Logo')) + ->setError($e_file) + ->setCaption( + pht('Supported formats: %s', implode(', ', $supported_formats)))) + ->appendChild( + id(new AphrontFormSubmitControl()) + ->addCancelButton($uri) + ->setValue(pht('Upload Logo'))); + + $upload_box = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Upload New Logo')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->setForm($upload_form); + + $crumbs = $this->buildApplicationCrumbs(); + $crumbs->addTextCrumb($merchant->getName(), $uri); + $crumbs->addTextCrumb(pht('Merchant Logo')); + $crumbs->setBorder(true); + + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Edit Merchant Logo')) + ->setHeaderIcon('fa-camera'); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter(array( + $form_box, + $upload_box, + )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild( + array( + $view, + )); + + } +} diff --git a/src/applications/phortune/controller/PhortuneMerchantViewController.php b/src/applications/phortune/controller/PhortuneMerchantViewController.php index cc94da6f4f..6c74ce8a06 100644 --- a/src/applications/phortune/controller/PhortuneMerchantViewController.php +++ b/src/applications/phortune/controller/PhortuneMerchantViewController.php @@ -10,6 +10,7 @@ final class PhortuneMerchantViewController $merchant = id(new PhortuneMerchantQuery()) ->setViewer($viewer) ->withIDs(array($id)) + ->needProfileImage(true) ->executeOne(); if (!$merchant) { return new Aphront404Response(); @@ -28,7 +29,7 @@ final class PhortuneMerchantViewController ->setHeader($merchant->getName()) ->setUser($viewer) ->setPolicyObject($merchant) - ->setHeaderIcon('fa-bank'); + ->setImage($merchant->getProfileImageURI()); $providers = id(new PhortunePaymentProviderConfigQuery()) ->setViewer($viewer) @@ -171,6 +172,14 @@ final class PhortuneMerchantViewController ->setWorkflow(!$can_edit) ->setHref($this->getApplicationURI("merchant/edit/{$id}/"))); + $curtain->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Edit Logo')) + ->setIcon('fa-camera') + ->setDisabled(!$can_edit) + ->setWorkflow(!$can_edit) + ->setHref($this->getApplicationURI("merchant/picture/{$id}/"))); + $curtain->addAction( id(new PhabricatorActionView()) ->setName(pht('View Orders')) diff --git a/src/applications/phortune/editor/PhortuneMerchantEditor.php b/src/applications/phortune/editor/PhortuneMerchantEditor.php index 20f329b202..2d35ddabfd 100644 --- a/src/applications/phortune/editor/PhortuneMerchantEditor.php +++ b/src/applications/phortune/editor/PhortuneMerchantEditor.php @@ -17,6 +17,7 @@ final class PhortuneMerchantEditor $types[] = PhortuneMerchantTransaction::TYPE_NAME; $types[] = PhortuneMerchantTransaction::TYPE_DESCRIPTION; $types[] = PhortuneMerchantTransaction::TYPE_CONTACTINFO; + $types[] = PhortuneMerchantTransaction::TYPE_PICTURE; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDGE; @@ -33,6 +34,8 @@ final class PhortuneMerchantEditor return $object->getDescription(); case PhortuneMerchantTransaction::TYPE_CONTACTINFO: return $object->getContactInfo(); + case PhortuneMerchantTransaction::TYPE_PICTURE: + return $object->getProfileImagePHID(); } return parent::getCustomTransactionOldValue($object, $xaction); @@ -46,6 +49,7 @@ final class PhortuneMerchantEditor case PhortuneMerchantTransaction::TYPE_NAME: case PhortuneMerchantTransaction::TYPE_DESCRIPTION: case PhortuneMerchantTransaction::TYPE_CONTACTINFO: + case PhortuneMerchantTransaction::TYPE_PICTURE: return $xaction->getNewValue(); } @@ -66,6 +70,9 @@ final class PhortuneMerchantEditor case PhortuneMerchantTransaction::TYPE_CONTACTINFO: $object->setContactInfo($xaction->getNewValue()); return; + case PhortuneMerchantTransaction::TYPE_PICTURE: + $object->setProfileImagePHID($xaction->getNewValue()); + return; } return parent::applyCustomInternalTransaction($object, $xaction); @@ -79,6 +86,7 @@ final class PhortuneMerchantEditor case PhortuneMerchantTransaction::TYPE_NAME: case PhortuneMerchantTransaction::TYPE_DESCRIPTION: case PhortuneMerchantTransaction::TYPE_CONTACTINFO: + case PhortuneMerchantTransaction::TYPE_PICTURE: return; } diff --git a/src/applications/phortune/query/PhortuneMerchantQuery.php b/src/applications/phortune/query/PhortuneMerchantQuery.php index aa730393ff..c91267b73c 100644 --- a/src/applications/phortune/query/PhortuneMerchantQuery.php +++ b/src/applications/phortune/query/PhortuneMerchantQuery.php @@ -6,6 +6,7 @@ final class PhortuneMerchantQuery private $ids; private $phids; private $memberPHIDs; + private $needProfileImage; public function withIDs(array $ids) { $this->ids = $ids; @@ -22,6 +23,11 @@ final class PhortuneMerchantQuery return $this; } + public function needProfileImage($need) { + $this->needProfileImage = $need; + return $this; + } + protected function loadPage() { $table = new PhortuneMerchant(); $conn = $table->establishConnection('r'); @@ -50,6 +56,35 @@ final class PhortuneMerchantQuery $merchant->attachMemberPHIDs($member_phids); } + if ($this->needProfileImage) { + $default = null; + $file_phids = mpull($merchants, 'getProfileImagePHID'); + $file_phids = array_filter($file_phids); + if ($file_phids) { + $files = id(new PhabricatorFileQuery()) + ->setParentQuery($this) + ->setViewer($this->getViewer()) + ->withPHIDs($file_phids) + ->execute(); + $files = mpull($files, null, 'getPHID'); + } else { + $files = array(); + } + + foreach ($merchants as $merchant) { + $file = idx($files, $merchant->getProfileImagePHID()); + if (!$file) { + if (!$default) { + $default = PhabricatorFile::loadBuiltin( + $this->getViewer(), + 'merchant.png'); + } + $file = $default; + } + $merchant->attachProfileImageFile($file); + } + } + return $merchants; } diff --git a/src/applications/phortune/query/PhortuneMerchantSearchEngine.php b/src/applications/phortune/query/PhortuneMerchantSearchEngine.php index d37c1455b3..a818f37d28 100644 --- a/src/applications/phortune/query/PhortuneMerchantSearchEngine.php +++ b/src/applications/phortune/query/PhortuneMerchantSearchEngine.php @@ -18,7 +18,8 @@ final class PhortuneMerchantSearchEngine } public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { - $query = id(new PhortuneMerchantQuery()); + $query = id(new PhortuneMerchantQuery()) + ->needProfileImage(true); return $query; } @@ -74,7 +75,7 @@ final class PhortuneMerchantSearchEngine ->setHeader($merchant->getName()) ->setHref('/phortune/merchant/'.$merchant->getID().'/') ->setObject($merchant) - ->setImageIcon('fa-bank'); + ->setImageURI($merchant->getProfileImageURI()); $list->addItem($item); } diff --git a/src/applications/phortune/storage/PhortuneMerchant.php b/src/applications/phortune/storage/PhortuneMerchant.php index fef1728014..7d202eeaf0 100644 --- a/src/applications/phortune/storage/PhortuneMerchant.php +++ b/src/applications/phortune/storage/PhortuneMerchant.php @@ -9,8 +9,10 @@ final class PhortuneMerchant extends PhortuneDAO protected $viewPolicy; protected $description; protected $contactInfo; + protected $profileImagePHID; private $memberPHIDs = self::ATTACHABLE; + private $profileImageFile = self::ATTACHABLE; public static function initializeNewMerchant(PhabricatorUser $actor) { return id(new PhortuneMerchant()) @@ -25,6 +27,7 @@ final class PhortuneMerchant extends PhortuneDAO 'name' => 'text255', 'description' => 'text', 'contactInfo' => 'text', + 'profileImagePHID' => 'phid?', ), ) + parent::getConfiguration(); } @@ -43,6 +46,23 @@ final class PhortuneMerchant extends PhortuneDAO return $this; } + public function getViewURI() { + return '/phortune/merchant/'.$this->getID().'/'; + } + + public function getProfileImageURI() { + return $this->getProfileImageFile()->getBestURI(); + } + + public function attachProfileImageFile(PhabricatorFile $file) { + $this->profileImageFile = $file; + return $this; + } + + public function getProfileImageFile() { + return $this->assertAttached($this->profileImageFile); + } + /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/phortune/storage/PhortuneMerchantTransaction.php b/src/applications/phortune/storage/PhortuneMerchantTransaction.php index 9c284ca7fd..48895d85ec 100644 --- a/src/applications/phortune/storage/PhortuneMerchantTransaction.php +++ b/src/applications/phortune/storage/PhortuneMerchantTransaction.php @@ -6,6 +6,7 @@ final class PhortuneMerchantTransaction const TYPE_NAME = 'merchant:name'; const TYPE_DESCRIPTION = 'merchant:description'; const TYPE_CONTACTINFO = 'merchant:contactinfo'; + const TYPE_PICTURE = 'merchant:picture'; public function getApplicationName() { return 'phortune'; From 0c9ecf835166ec214c7ee2bacaca76707c5363ee Mon Sep 17 00:00:00 2001 From: Chad Little Date: Fri, 28 Oct 2016 14:13:20 -0700 Subject: [PATCH 02/41] Update Phortune Merchant to EditEngine Summary: Converts PhortuneMerchant to EditEngine. Test Plan: Edits existing merchants fine, same issue as Conpherence when making new ones with permissions. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16750 --- src/__phutil_library_map__.php | 2 + .../PhabricatorPhortuneApplication.php | 3 +- .../PhortuneMerchantEditController.php | 193 +----------------- .../editor/PhortuneMerchantEditEngine.php | 120 +++++++++++ .../phortune/storage/PhortuneMerchant.php | 1 - 5 files changed, 128 insertions(+), 191 deletions(-) create mode 100644 src/applications/phortune/editor/PhortuneMerchantEditEngine.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index dcb61c6342..c6b7b28774 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4180,6 +4180,7 @@ phutil_register_library_map(array( 'PhortuneMerchantCapability' => 'applications/phortune/capability/PhortuneMerchantCapability.php', 'PhortuneMerchantController' => 'applications/phortune/controller/PhortuneMerchantController.php', 'PhortuneMerchantEditController' => 'applications/phortune/controller/PhortuneMerchantEditController.php', + 'PhortuneMerchantEditEngine' => 'applications/phortune/editor/PhortuneMerchantEditEngine.php', 'PhortuneMerchantEditor' => 'applications/phortune/editor/PhortuneMerchantEditor.php', 'PhortuneMerchantHasMemberEdgeType' => 'applications/phortune/edge/PhortuneMerchantHasMemberEdgeType.php', 'PhortuneMerchantInvoiceCreateController' => 'applications/phortune/controller/PhortuneMerchantInvoiceCreateController.php', @@ -9430,6 +9431,7 @@ phutil_register_library_map(array( 'PhortuneMerchantCapability' => 'PhabricatorPolicyCapability', 'PhortuneMerchantController' => 'PhortuneController', 'PhortuneMerchantEditController' => 'PhortuneMerchantController', + 'PhortuneMerchantEditEngine' => 'PhabricatorEditEngine', 'PhortuneMerchantEditor' => 'PhabricatorApplicationTransactionEditor', 'PhortuneMerchantHasMemberEdgeType' => 'PhabricatorEdgeType', 'PhortuneMerchantInvoiceCreateController' => 'PhortuneMerchantController', diff --git a/src/applications/phortune/application/PhabricatorPhortuneApplication.php b/src/applications/phortune/application/PhabricatorPhortuneApplication.php index 313328e41e..434cc4119a 100644 --- a/src/applications/phortune/application/PhabricatorPhortuneApplication.php +++ b/src/applications/phortune/application/PhabricatorPhortuneApplication.php @@ -82,7 +82,8 @@ final class PhabricatorPhortuneApplication extends PhabricatorApplication { 'merchant/' => array( '(?:query/(?P[^/]+)/)?' => 'PhortuneMerchantListController', 'picture/(?:(?P\d+)/)?' => 'PhortuneMerchantPictureController', - 'edit/(?:(?P\d+)/)?' => 'PhortuneMerchantEditController', + $this->getEditRoutePattern('edit/') + => 'PhortuneMerchantEditController', 'orders/(?P\d+)/(?:query/(?P[^/]+)/)?' => 'PhortuneCartListController', '(?P\d+)/' => array( diff --git a/src/applications/phortune/controller/PhortuneMerchantEditController.php b/src/applications/phortune/controller/PhortuneMerchantEditController.php index 4def6edebf..07c743251c 100644 --- a/src/applications/phortune/controller/PhortuneMerchantEditController.php +++ b/src/applications/phortune/controller/PhortuneMerchantEditController.php @@ -4,193 +4,8 @@ final class PhortuneMerchantEditController extends PhortuneMerchantController { public function handleRequest(AphrontRequest $request) { - $viewer = $request->getViewer(); - $id = $request->getURIData('id'); - - if ($id) { - $merchant = id(new PhortuneMerchantQuery()) - ->setViewer($viewer) - ->withIDs(array($id)) - ->requireCapabilities( - array( - PhabricatorPolicyCapability::CAN_VIEW, - PhabricatorPolicyCapability::CAN_EDIT, - )) - ->executeOne(); - if (!$merchant) { - return new Aphront404Response(); - } - $is_new = false; - } else { - $this->requireApplicationCapability( - PhortuneMerchantCapability::CAPABILITY); - - $merchant = PhortuneMerchant::initializeNewMerchant($viewer); - $merchant->attachMemberPHIDs(array($viewer->getPHID())); - $is_new = true; - } - - if ($is_new) { - $title = pht('Create Merchant'); - $button_text = pht('Create Merchant'); - $cancel_uri = $this->getApplicationURI('merchant/'); - } else { - $title = pht( - 'Edit Merchant %d %s', - $merchant->getID(), - $merchant->getName()); - $button_text = pht('Save Changes'); - $cancel_uri = $this->getApplicationURI( - '/merchant/'.$merchant->getID().'/'); - } - - $e_name = true; - $v_name = $merchant->getName(); - $v_desc = $merchant->getDescription(); - $v_cont = $merchant->getContactInfo(); - $v_members = $merchant->getMemberPHIDs(); - $e_members = null; - - $validation_exception = null; - if ($request->isFormPost()) { - $v_name = $request->getStr('name'); - $v_desc = $request->getStr('desc'); - $v_cont = $request->getStr('cont'); - $v_view = $request->getStr('viewPolicy'); - $v_edit = $request->getStr('editPolicy'); - $v_members = $request->getArr('memberPHIDs'); - - $type_name = PhortuneMerchantTransaction::TYPE_NAME; - $type_desc = PhortuneMerchantTransaction::TYPE_DESCRIPTION; - $type_cont = PhortuneMerchantTransaction::TYPE_CONTACTINFO; - $type_edge = PhabricatorTransactions::TYPE_EDGE; - $type_view = PhabricatorTransactions::TYPE_VIEW_POLICY; - - $edge_members = PhortuneMerchantHasMemberEdgeType::EDGECONST; - - $xactions = array(); - - $xactions[] = id(new PhortuneMerchantTransaction()) - ->setTransactionType($type_name) - ->setNewValue($v_name); - - $xactions[] = id(new PhortuneMerchantTransaction()) - ->setTransactionType($type_desc) - ->setNewValue($v_desc); - - $xactions[] = id(new PhortuneMerchantTransaction()) - ->setTransactionType($type_cont) - ->setNewValue($v_cont); - - $xactions[] = id(new PhortuneMerchantTransaction()) - ->setTransactionType($type_view) - ->setNewValue($v_view); - - $xactions[] = id(new PhortuneMerchantTransaction()) - ->setTransactionType($type_edge) - ->setMetadataValue('edge:type', $edge_members) - ->setNewValue( - array( - '=' => array_fuse($v_members), - )); - - $editor = id(new PhortuneMerchantEditor()) - ->setActor($viewer) - ->setContentSourceFromRequest($request) - ->setContinueOnNoEffect(true); - - try { - $editor->applyTransactions($merchant, $xactions); - - $id = $merchant->getID(); - $merchant_uri = $this->getApplicationURI("merchant/{$id}/"); - return id(new AphrontRedirectResponse())->setURI($merchant_uri); - } catch (PhabricatorApplicationTransactionValidationException $ex) { - $validation_exception = $ex; - - $e_name = $ex->getShortMessage($type_name); - $e_mbmers = $ex->getShortMessage($type_edge); - - $merchant->setViewPolicy($v_view); - } - } - - $policies = id(new PhabricatorPolicyQuery()) - ->setViewer($viewer) - ->setObject($merchant) - ->execute(); - - $form = id(new AphrontFormView()) - ->setUser($viewer) - ->appendChild( - id(new AphrontFormTextControl()) - ->setName('name') - ->setLabel(pht('Name')) - ->setValue($v_name) - ->setError($e_name)) - ->appendChild( - id(new PhabricatorRemarkupControl()) - ->setUser($viewer) - ->setName('desc') - ->setLabel(pht('Description')) - ->setValue($v_desc)) - ->appendChild( - id(new PhabricatorRemarkupControl()) - ->setUser($viewer) - ->setName('cont') - ->setLabel(pht('Contact Info')) - ->setValue($v_cont)) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setDatasource(new PhabricatorPeopleDatasource()) - ->setLabel(pht('Members')) - ->setName('memberPHIDs') - ->setValue($v_members) - ->setError($e_members)) - ->appendChild( - id(new AphrontFormPolicyControl()) - ->setName('viewPolicy') - ->setPolicyObject($merchant) - ->setCapability(PhabricatorPolicyCapability::CAN_VIEW) - ->setPolicies($policies)) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->setValue($button_text) - ->addCancelButton($cancel_uri)); - - $header = id(new PHUIHeaderView()) - ->setHeader($title); - - $crumbs = $this->buildApplicationCrumbs(); - if ($is_new) { - $crumbs->addTextCrumb(pht('Create Merchant')); - $header->setHeaderIcon('fa-plus-square'); - } else { - $crumbs->addTextCrumb( - pht('Merchant %d', $merchant->getID()), - $this->getApplicationURI('/merchant/'.$merchant->getID().'/')); - $crumbs->addTextCrumb(pht('Edit')); - $header->setHeaderIcon('fa-pencil'); - } - $crumbs->setBorder(true); - - $box = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Merchant')) - ->setValidationException($validation_exception) - ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) - ->setForm($form); - - $view = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setFooter(array( - $box, - )); - - return $this->newPage() - ->setTitle($title) - ->setCrumbs($crumbs) - ->appendChild($view); - - } - + return id(new PhortuneMerchantEditEngine()) + ->setController($this) + ->buildResponse(); + } } diff --git a/src/applications/phortune/editor/PhortuneMerchantEditEngine.php b/src/applications/phortune/editor/PhortuneMerchantEditEngine.php new file mode 100644 index 0000000000..1c68fe67c9 --- /dev/null +++ b/src/applications/phortune/editor/PhortuneMerchantEditEngine.php @@ -0,0 +1,120 @@ +getViewer()); + } + + protected function newObjectQuery() { + return new PhortuneMerchantQuery(); + } + + protected function getObjectCreateTitleText($object) { + return pht('Create New Merchant'); + } + + protected function getObjectEditTitleText($object) { + return pht('Edit Merchant: %s', $object->getName()); + } + + protected function getObjectEditShortText($object) { + return $object->getName(); + } + + protected function getObjectCreateShortText() { + return pht('Create Merchant'); + } + + protected function getObjectName() { + return pht('Merchant'); + } + + protected function getObjectCreateCancelURI($object) { + return $this->getApplication()->getApplicationURI('/'); + } + + protected function getEditorURI() { + return $this->getApplication()->getApplicationURI('edit/'); + } + + protected function getObjectViewURI($object) { + return $object->getViewURI(); + } + + public function isEngineConfigurable() { + return false; + } + + protected function buildCustomEditFields($object) { + $viewer = $this->getViewer(); + + if ($this->getIsCreate()) { + $member_phids = array($viewer->getPHID()); + } else { + $member_phids = $object->getMemberPHIDs(); + } + + return array( + id(new PhabricatorTextEditField()) + ->setKey('name') + ->setLabel(pht('Name')) + ->setDescription(pht('Merchant name.')) + ->setConduitTypeDescription(pht('New Merchant name.')) + ->setIsRequired(true) + ->setTransactionType(PhortuneMerchantTransaction::TYPE_NAME) + ->setValue($object->getName()), + + id(new PhabricatorRemarkupEditField()) + ->setKey('description') + ->setLabel(pht('Description')) + ->setDescription(pht('Merchant description.')) + ->setConduitTypeDescription(pht('New merchant description.')) + ->setTransactionType(PhortuneMerchantTransaction::TYPE_DESCRIPTION) + ->setValue($object->getDescription()), + + id(new PhabricatorRemarkupEditField()) + ->setKey('contactInfo') + ->setLabel(pht('Contact Info')) + ->setDescription(pht('Merchant contact information.')) + ->setConduitTypeDescription(pht('Merchant contact information.')) + ->setTransactionType(PhortuneMerchantTransaction::TYPE_CONTACTINFO) + ->setValue($object->getContactInfo()), + + id(new PhabricatorUsersEditField()) + ->setKey('members') + ->setAliases(array('memberPHIDs')) + ->setLabel(pht('Members')) + ->setUseEdgeTransactions(true) + ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) + ->setMetadataValue( + 'edge:type', + PhortuneMerchantHasMemberEdgeType::EDGECONST) + ->setDescription(pht('Initial merchant members.')) + ->setConduitDescription(pht('Set merchant members.')) + ->setConduitTypeDescription(pht('New list of members.')) + ->setValue($member_phids), + + ); + } + +} diff --git a/src/applications/phortune/storage/PhortuneMerchant.php b/src/applications/phortune/storage/PhortuneMerchant.php index 7d202eeaf0..b2117a2662 100644 --- a/src/applications/phortune/storage/PhortuneMerchant.php +++ b/src/applications/phortune/storage/PhortuneMerchant.php @@ -63,7 +63,6 @@ final class PhortuneMerchant extends PhortuneDAO return $this->assertAttached($this->profileImageFile); } - /* -( PhabricatorApplicationTransactionInterface )------------------------- */ From a792faf78d82cf2c19a75436a9fbcba6774c2780 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 28 Oct 2016 14:03:02 -0700 Subject: [PATCH 03/41] Use "book" instead of "life ring" icon for global help menu Summary: This is more consistent with the icon we use for documentation elsewhere. Test Plan: Looked at the icon, had an easier time guessing it meant "documentation". Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D16765 --- .../help/extension/PhabricatorHelpMainMenuBarExtension.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/help/extension/PhabricatorHelpMainMenuBarExtension.php b/src/applications/help/extension/PhabricatorHelpMainMenuBarExtension.php index f12714b5ac..3d2969c035 100644 --- a/src/applications/help/extension/PhabricatorHelpMainMenuBarExtension.php +++ b/src/applications/help/extension/PhabricatorHelpMainMenuBarExtension.php @@ -36,7 +36,7 @@ final class PhabricatorHelpMainMenuBarExtension $help_name = pht('%s Help', $application->getName()); $help_item = id(new PHUIListItemView()) - ->setIcon('fa-life-ring') + ->setIcon('fa-book') ->addClass('core-menu-item') ->setID($help_id) ->setName($help_name) From 2bbddb8c0f2943f57793ed7ce95ea303d89c238e Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 28 Oct 2016 13:21:38 -0700 Subject: [PATCH 04/41] Improve some setInitialValue() behavior for PhortuneMerchants Summary: This fixes the permissions issue with D16750, which is actually not really a permissions issue, exactly. This is the only place anywhere that we use a tokenizer field //and// give it a default value which is not the same as the object value (when creating a merchant, we default it to the viewer). In other cases (like Maniphest) we avoid this because you can edit the form to have defaults, which would collide with whatever default we provide. Some disucssion in T10222. Since we aren't going to let you edit these forms for the forseeable future, this behavior is reasonable here though. However, it triggered a sort-of-bug related to conflict detection for these fields (see T4768). These fields actually have two values: a hidden "initial" value, and a visible edited value. When you submit the form, we compute your edit by comparing the edited value to the initial value, then applying adds/removes, instead of just saying "set value equal to new value". This prevents issues when two people edit at the same time and both make changes to the field. In this case, the initial value was being set to the display value, so the field would say "Value: [(alincoln x)]" but internally have that as the intitial value, too. When you submitted, it would see "you didn't change anything", and thus not add any members. So the viewer wouldn't actually be added as a member, then the policy check would correctly fail. Note that there are still some policy issues here (you can remove yourself from a Merchant and lock yourself out) but they fall into the realm of stuff discussed in D16677. Test Plan: Created a merchant account with D16750 applied. Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D16764 --- .../phortune/editor/PhortuneMerchantEditEngine.php | 1 + .../transactions/editfield/PhabricatorEditField.php | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/applications/phortune/editor/PhortuneMerchantEditEngine.php b/src/applications/phortune/editor/PhortuneMerchantEditEngine.php index 1c68fe67c9..8c2356e6fe 100644 --- a/src/applications/phortune/editor/PhortuneMerchantEditEngine.php +++ b/src/applications/phortune/editor/PhortuneMerchantEditEngine.php @@ -112,6 +112,7 @@ final class PhortuneMerchantEditEngine ->setDescription(pht('Initial merchant members.')) ->setConduitDescription(pht('Set merchant members.')) ->setConduitTypeDescription(pht('New list of members.')) + ->setInitialValue($object->getMemberPHIDs()) ->setValue($member_phids), ); diff --git a/src/applications/transactions/editfield/PhabricatorEditField.php b/src/applications/transactions/editfield/PhabricatorEditField.php index 6ff84da870..a0b63e2da1 100644 --- a/src/applications/transactions/editfield/PhabricatorEditField.php +++ b/src/applications/transactions/editfield/PhabricatorEditField.php @@ -401,8 +401,15 @@ abstract class PhabricatorEditField extends Phobject { public function setValue($value) { $this->hasValue = true; - $this->initialValue = $value; $this->value = $value; + + // If we don't have an initial value set yet, use the value as the + // initial value. + $initial_value = $this->getInitialValue(); + if ($initial_value === null) { + $this->initialValue = $value; + } + return $this; } From 1014a2771795f7a9062666c4ad6c8effe5cc435e Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 28 Oct 2016 14:53:46 -0700 Subject: [PATCH 05/41] Document Calendar imports Summary: Ref T10747. - Adds import documentation. - Adds import/export docs to the help menu. - Removes some weird/old/out-of-date information from the general user guide, which I'll rewrite later. Test Plan: Read documentation somewhat thoroughly. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10747 Differential Revision: https://secure.phabricator.com/D16766 --- .../PhabricatorCalendarApplication.php | 10 ++ src/docs/user/userguide/calendar.diviner | 74 +--------- .../user/userguide/calendar_imports.diviner | 132 ++++++++++++++++++ 3 files changed, 147 insertions(+), 69 deletions(-) create mode 100644 src/docs/user/userguide/calendar_imports.diviner diff --git a/src/applications/calendar/application/PhabricatorCalendarApplication.php b/src/applications/calendar/application/PhabricatorCalendarApplication.php index 1e1a125cde..8caf8c27ec 100644 --- a/src/applications/calendar/application/PhabricatorCalendarApplication.php +++ b/src/applications/calendar/application/PhabricatorCalendarApplication.php @@ -104,6 +104,16 @@ final class PhabricatorCalendarApplication extends PhabricatorApplication { 'name' => pht('Calendar User Guide'), 'href' => PhabricatorEnv::getDoclink('Calendar User Guide'), ), + array( + 'name' => pht('Importing Events'), + 'href' => PhabricatorEnv::getDoclink( + 'Calendar User Guide: Importing Events'), + ), + array( + 'name' => pht('Exporting Events'), + 'href' => PhabricatorEnv::getDoclink( + 'Calendar User Guide: Exporting Events'), + ), ); } diff --git a/src/docs/user/userguide/calendar.diviner b/src/docs/user/userguide/calendar.diviner index e6f9bd4b2c..3aefa84e17 100644 --- a/src/docs/user/userguide/calendar.diviner +++ b/src/docs/user/userguide/calendar.diviner @@ -9,75 +9,11 @@ Overview IMPORTANT: Calendar is a prototype application. See @{article:User Guide: Prototype Applications}. -The Calendar application is a tool that allows users to schedule group events -and share personal plans. -There are several kinds of events you can create: +Next Steps +========== -- Regular events such as a one-time meeting or a personal appointment. -- All day events such as a company-wide holiday or a vacation. -- Recurring events, which can be regular or all day, such as a weekly 1-1 or - a birthday. +Continue by: - -Editing Events -============== - -All fields of basic and all day events can be edited after the event has been -created. - -Every instance of a recurring event has an index that maintains its place in -the sequence order. Before an instance of a recurring event is edited, it is -considered a ghost event, or a placeholder. This means that there is no -database entry for that instance. Rather, when querying for events, if a -recurring series of events overlaps with the query range, instance -placeholders of that recurring event are generated and are displayed for -that range. If a placeholder instance of a recurring event is edited, a real -entry in the database is created and all changes are saved. When that -instance falls within a query range, the real instance event replaces the -old placeholder instance. - -To prevent disordering of the recurring sequence of events, parent recurring -events do not allow editing of date-related fields like recurrence frequency -and recurrence start and end dates. If all instances of the recurring event -need to be rescheduled, users are encouraged to cancel a recurring event and -create a new recurring event with the revised date and time. - - -Cancelling Events -================= - -Cancelling basic events will hide that event from most of the builtin Calendar -queries, unless the query specifies to display cancelled events. - -There are two ways to cancel an instance of a recurring event. - -- Cancel an instance of a recurring event. -- Cancel the entire series of a recurring event. - -Cancelling a placeholder instance of a recurring event will create a real -cancelled event that will replace the placeholder instance. Consequently, -the cancellation status of that instance of the recurring event will -persist if the parent event is cancelled and subsequently reinstated. - -When an entire series of a recurring event is cancelled, all the placeholder -and real instances are also cancelled. An entire series can similarly be -reinstated, but it is currently not possible to reinstate an instance of a -cancelled recurring event series. To reinstate that instance, the entire -series must be reinstated. If an instance of a recurring event has been -cancelled, then the entire recurring event series is also cancelled, -reinstating the series will not reinstate the previously cancelled instances -of that event. - - -Commenting On Recurring Events -============================== - -If a placeholder instance of a recurring event has not been converted to a -real instance of the series as a result of editing or cancelling, commenting on -that placeholder instance does not currently save a draft for that instance -only. The draft is saved for the recurring event parent, so the parent -recurring event and all placeholder instances will show that draft. When a -comment is actually added to a placeholder instance, the instance is converted -to a recurrence exception, and the comment will only appear on that instance -of the recurring event. + - importing events with @{article:Calendar User Guide: Importing Events}; or + - exporting events with @{article:Calendar User Guide: Exporting Events}. diff --git a/src/docs/user/userguide/calendar_imports.diviner b/src/docs/user/userguide/calendar_imports.diviner new file mode 100644 index 0000000000..a4163e9bcd --- /dev/null +++ b/src/docs/user/userguide/calendar_imports.diviner @@ -0,0 +1,132 @@ +@title Calendar User Guide: Importing Events +@group userguide + +Importing events from other calendars. + +Overview +======== + +IMPORTANT: Calendar is a prototype application. See +@{article:User Guide: Prototype Applications}. + +You can import events into Phabricator to other calendar applications or from +`.ics` files. This document will guide you through how to importe event data +into Phabricator. + +When you import events from another application, they can not be edited in +Phabricator. Importing events allows you to share events or keep track of +events from different sources, but does not let you edit events from other +applications in Phabricator. + + +Import Policies +=============== + +When you import events, you select a visibility policy for the import. By +default, imported events are only visible to you (the user importing them). + +To share imported events with other users, make the import **Visible To** +a wider set of users, like "All Users". + + +Importing `.ics` Files +====================== + +`.ics` files contain information about events, usually either about a single +event or an entire event calendar. + +If you have an event or calendar in `.ics` format, you can import it into +Phabricator in two ways: + + - Navigate to {nav Calendar > Imports > Import Events > Import .ics File}. + - Drag and drop the file onto a Calendar. + +This will create a copy of the event in Phabricator. + +If you want to update an imported event later, just repeat this process. The +event will be updated with the latest information. + +Many applications send `.ics` files as email attachments. You can import these +into Phabricator. + + +.ics Files: Google Calendar +=========================== + +In **Google Calendar**, you can generate a `.ics` file for a calendar by +clicking the dropdown menu next to the calendar and selecting +{nav Calendar Settings > Export Calendar > Export this calendar}. + + +.ics Files: Calendar.app +======================== + +In **Calendar.app**, you can generate an `.ics` file for a calendar by +selecting the calendar, then selecting {nav File > Export > Export...} and +saving the calendar as a `.ics` file. + +You can also convert an individual event into an `.ics` file by dragging it +from the calendar to your desktop (or any other folder). + +When you import an event using an `.ics` file, Phabricator can not +automatically keep the event up to date. You'll need to repeat the process if +there are changes to the event or calendar later, so Phabricator can learn +about the updates. + + +Importing .ics URIs +===================== + +If you have a calendar in another application that supports publishing a +`.ics` URI, you can subscribe to it in Phabricator. This will import the entire +calendar, and can be configured to automatically keep it up to date and in sync +with the external calendar. + +First, find the subscription URI for the calendar you want to import (see +below for some guidance on popular calendar applications). Then, browse to +{nav Calendar > Imports > Import Events > Import .ics URI}. + +When you import a URI, you can choose to enable automatic updates. If you do, +Phabricator will periodically update the events it imports from this source. +You can stop this later by turning off the automatic updates or disabling +the import. + +{icon lock} **Privacy Note**: When you import via URI, the URI often contains +sensitive information (like a username, password, or secret key) which allows +anyone who knows it to access private details about events. Anyone who can edit +the import will also be able to view and edit the URI, so make sure you don't +grant edit access to users who should not have access to the event details. + + +.ics URIs: Google Calendar +========================== + +In **Google Calendar**, you can get the subscription URI for a calendar +by selecting {nav Calendar Settings} from the dropdown next to the calendar, +then copying the URL from the {nav ICAL} link under **Private Address**. This +URI provides access to all event details, including private information. + +You may need to adjust the sharing and visibility settings for the calendar +before this option is available. + +Alternatively, you can use the URI from the {nav ICAL} link under +**Calendar Address** to access a more limited set of event details. You can +configure which details are available by configuring how the calendar is +shared. + + +.ics URIs: Calendar.app +======================= + +**Calendar.app** does not support subscriptions via `.ics` URIs. + +You can export a calendar as an `.ics` file by following the steps above, but +Phabricator can not automatically keep events imported in this way up to date. + + +Next Steps +========== + +Continue by: + + - returning to the @{article:Calendar User Guide}. From 1e488e92771a0700f0eda938805df469e40eba59 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 28 Oct 2016 15:59:26 -0700 Subject: [PATCH 06/41] When importing events, delete events which have been removed on the other end Summary: Ref T10747. If stuff has been deleted on the other calendar, delete it on ours. Test Plan: Imported with deletion, saw deletions: {F1889689} Reviewers: chad Reviewed By: chad Maniphest Tasks: T10747 Differential Revision: https://secure.phabricator.com/D16768 --- src/__phutil_library_map__.php | 2 ++ .../PhabricatorCalendarImportEngine.php | 26 +++++++++++--- ...PhabricatorCalendarImportDeleteLogType.php | 34 +++++++++++++++++++ 3 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 src/applications/calendar/importlog/PhabricatorCalendarImportDeleteLogType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index c6b7b28774..b8eb474a20 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2111,6 +2111,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarImport' => 'applications/calendar/storage/PhabricatorCalendarImport.php', 'PhabricatorCalendarImportDefaultLogType' => 'applications/calendar/importlog/PhabricatorCalendarImportDefaultLogType.php', 'PhabricatorCalendarImportDeleteController' => 'applications/calendar/controller/PhabricatorCalendarImportDeleteController.php', + 'PhabricatorCalendarImportDeleteLogType' => 'applications/calendar/importlog/PhabricatorCalendarImportDeleteLogType.php', 'PhabricatorCalendarImportDeleteTransaction' => 'applications/calendar/xaction/PhabricatorCalendarImportDeleteTransaction.php', 'PhabricatorCalendarImportDisableController' => 'applications/calendar/controller/PhabricatorCalendarImportDisableController.php', 'PhabricatorCalendarImportDisableTransaction' => 'applications/calendar/xaction/PhabricatorCalendarImportDisableTransaction.php', @@ -6965,6 +6966,7 @@ phutil_register_library_map(array( ), 'PhabricatorCalendarImportDefaultLogType' => 'PhabricatorCalendarImportLogType', 'PhabricatorCalendarImportDeleteController' => 'PhabricatorCalendarController', + 'PhabricatorCalendarImportDeleteLogType' => 'PhabricatorCalendarImportLogType', 'PhabricatorCalendarImportDeleteTransaction' => 'PhabricatorCalendarImportTransactionType', 'PhabricatorCalendarImportDisableController' => 'PhabricatorCalendarController', 'PhabricatorCalendarImportDisableTransaction' => 'PhabricatorCalendarImportTransactionType', diff --git a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php index 1eae386869..74d3866fb7 100644 --- a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php +++ b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php @@ -409,10 +409,28 @@ abstract class PhabricatorCalendarImportEngine array()); } - // TODO: When the source is a subscription-based ICS file or some other - // similar source, we should load all events from the source here and - // destroy the ones we didn't update. These are events that have been - // deleted. + // Delete any events which are no longer present in the source. + $updated_events = mpull($update_map, null, 'getPHID'); + $source_events = id(new PhabricatorCalendarEventQuery()) + ->setViewer($viewer) + ->withImportSourcePHIDs(array($import->getPHID())) + ->execute(); + + $engine = new PhabricatorDestructionEngine(); + foreach ($source_events as $source_event) { + if (isset($updated_events[$source_event->getPHID()])) { + // We imported and updated this event, so keep it around. + continue; + } + + $import->newLogMessage( + PhabricatorCalendarImportDeleteLogType::LOGTYPE, + array( + 'name' => $source_event->getName(), + )); + + $engine->destroyObject($source_event); + } } private function getFullNodeUID(PhutilCalendarEventNode $node) { diff --git a/src/applications/calendar/importlog/PhabricatorCalendarImportDeleteLogType.php b/src/applications/calendar/importlog/PhabricatorCalendarImportDeleteLogType.php new file mode 100644 index 0000000000..04801a4612 --- /dev/null +++ b/src/applications/calendar/importlog/PhabricatorCalendarImportDeleteLogType.php @@ -0,0 +1,34 @@ +getParameter('name')); + } + + public function getDisplayIcon( + PhabricatorUser $viewer, + PhabricatorCalendarImportLog $log) { + return 'fa-times'; + } + + public function getDisplayColor( + PhabricatorUser $viewer, + PhabricatorCalendarImportLog $log) { + return 'grey'; + } + +} From 199300565142da74f2292fff0114a7e160c0d558 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 29 Oct 2016 16:52:34 -0700 Subject: [PATCH 07/41] Fix a variant translation issue Summary: Fixes T11799. This string is varying on the first parameter, but should vary on the second parameter. Test Plan: Ran `bin/garbage set-policy ...`, saw proper translation. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11799 Differential Revision: https://secure.phabricator.com/D16769 --- .../translation/PhabricatorUSEnglishTranslation.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index b11f67ceb4..d89b5c8f71 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -1402,8 +1402,10 @@ final class PhabricatorUSEnglishTranslation ), 'Setting retention policy for "%s" to %s day(s).' => array( - 'Setting retention policy for "%s" to one day.', - 'Setting retention policy for "%s" to %s days.', + array( + 'Setting retention policy for "%s" to one day.', + 'Setting retention policy for "%s" to %s days.', + ), ), 'Waiting %s second(s) for lease to activate.' => array( From 5fd79479ecb46fd1c98268f9f1cae61408fa0171 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Sat, 29 Oct 2016 17:45:10 -0700 Subject: [PATCH 08/41] Add a basic invoice view for printing to Phortune Summary: Makes a more complete PDF looking invoice form for printing in Phortune. Test Plan: Make an invoice, click print view, print. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16762 --- resources/celerity/map.php | 2 + .../20161029.phortune.invoice.1.sql | 5 + src/__phutil_library_map__.php | 2 + .../PhabricatorPhortuneApplication.php | 1 + .../controller/PhortuneCartViewController.php | 95 ++++++++--- .../PhortuneMerchantViewController.php | 16 ++ .../editor/PhortuneMerchantEditEngine.php | 45 +++-- .../editor/PhortuneMerchantEditor.php | 39 +++++ .../phortune/storage/PhortuneMerchant.php | 4 + .../storage/PhortuneMerchantTransaction.php | 16 ++ .../phortune/view/PhortuneInvoiceView.php | 159 ++++++++++++++++++ .../application/phortune/phortune-invoice.css | 75 +++++++++ 12 files changed, 419 insertions(+), 40 deletions(-) create mode 100644 resources/sql/autopatches/20161029.phortune.invoice.1.sql create mode 100644 src/applications/phortune/view/PhortuneInvoiceView.php create mode 100644 webroot/rsrc/css/application/phortune/phortune-invoice.css diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 8c4ba2ef05..8725524a29 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -89,6 +89,7 @@ return array( 'rsrc/css/application/pholio/pholio-inline-comments.css' => '8e545e49', 'rsrc/css/application/pholio/pholio.css' => 'ca89d380', 'rsrc/css/application/phortune/phortune-credit-card-form.css' => '8391eb02', + 'rsrc/css/application/phortune/phortune-invoice.css' => '476055e2', 'rsrc/css/application/phortune/phortune.css' => '5b99dae0', 'rsrc/css/application/phrequent/phrequent.css' => 'ffc185ad', 'rsrc/css/application/phriction/phriction-document-css.css' => '4282e4ad', @@ -840,6 +841,7 @@ return array( 'phortune-credit-card-form' => '2290aeef', 'phortune-credit-card-form-css' => '8391eb02', 'phortune-css' => '5b99dae0', + 'phortune-invoice-css' => '476055e2', 'phrequent-css' => 'ffc185ad', 'phriction-document-css' => '4282e4ad', 'phui-action-panel-css' => '91c7b835', diff --git a/resources/sql/autopatches/20161029.phortune.invoice.1.sql b/resources/sql/autopatches/20161029.phortune.invoice.1.sql new file mode 100644 index 0000000000..7a4a8a8272 --- /dev/null +++ b/resources/sql/autopatches/20161029.phortune.invoice.1.sql @@ -0,0 +1,5 @@ +ALTER TABLE {$NAMESPACE}_phortune.phortune_merchant + ADD invoiceEmail VARCHAR(255) COLLATE {$COLLATE_TEXT} NOT NULL; + +ALTER TABLE {$NAMESPACE}_phortune.phortune_merchant + ADD invoiceFooter LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index b8eb474a20..d91e588a16 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4174,6 +4174,7 @@ phutil_register_library_map(array( 'PhortuneCurrencyTestCase' => 'applications/phortune/currency/__tests__/PhortuneCurrencyTestCase.php', 'PhortuneDAO' => 'applications/phortune/storage/PhortuneDAO.php', 'PhortuneErrCode' => 'applications/phortune/constants/PhortuneErrCode.php', + 'PhortuneInvoiceView' => 'applications/phortune/view/PhortuneInvoiceView.php', 'PhortuneLandingController' => 'applications/phortune/controller/PhortuneLandingController.php', 'PhortuneMemberHasAccountEdgeType' => 'applications/phortune/edge/PhortuneMemberHasAccountEdgeType.php', 'PhortuneMemberHasMerchantEdgeType' => 'applications/phortune/edge/PhortuneMemberHasMerchantEdgeType.php', @@ -9422,6 +9423,7 @@ phutil_register_library_map(array( 'PhortuneCurrencyTestCase' => 'PhabricatorTestCase', 'PhortuneDAO' => 'PhabricatorLiskDAO', 'PhortuneErrCode' => 'PhortuneConstants', + 'PhortuneInvoiceView' => 'AphrontTagView', 'PhortuneLandingController' => 'PhortuneController', 'PhortuneMemberHasAccountEdgeType' => 'PhabricatorEdgeType', 'PhortuneMemberHasMerchantEdgeType' => 'PhabricatorEdgeType', diff --git a/src/applications/phortune/application/PhabricatorPhortuneApplication.php b/src/applications/phortune/application/PhabricatorPhortuneApplication.php index 434cc4119a..c87914b79f 100644 --- a/src/applications/phortune/application/PhabricatorPhortuneApplication.php +++ b/src/applications/phortune/application/PhabricatorPhortuneApplication.php @@ -61,6 +61,7 @@ final class PhabricatorPhortuneApplication extends PhabricatorApplication { 'cart/(?P\d+)/' => array( '' => 'PhortuneCartViewController', 'checkout/' => 'PhortuneCartCheckoutController', + '(?Pprint)/' => 'PhortuneCartViewController', '(?Pcancel|refund)/' => 'PhortuneCartCancelController', 'update/' => 'PhortuneCartUpdateController', ), diff --git a/src/applications/phortune/controller/PhortuneCartViewController.php b/src/applications/phortune/controller/PhortuneCartViewController.php index e105973e11..8387cacb07 100644 --- a/src/applications/phortune/controller/PhortuneCartViewController.php +++ b/src/applications/phortune/controller/PhortuneCartViewController.php @@ -3,9 +3,12 @@ final class PhortuneCartViewController extends PhortuneCartController { + private $action = null; + public function handleRequest(AphrontRequest $request) { $viewer = $request->getViewer(); $id = $request->getURIData('id'); + $this->action = $request->getURIData('action'); $authority = $this->loadMerchantAuthority(); require_celerity_resource('phortune-css'); @@ -129,7 +132,7 @@ final class PhortuneCartViewController $header = id(new PHUIHeaderView()) ->setUser($viewer) - ->setHeader(pht('Order Detail')) + ->setHeader($cart->getName()) ->setHeaderIcon('fa-shopping-cart'); if ($cart->getStatus() == PhortuneCart::STATUS_PURCHASED) { @@ -188,36 +191,75 @@ final class PhortuneCartViewController $crumbs->addTextCrumb(pht('Cart %d', $cart->getID())); $crumbs->setBorder(true); - $timeline = $this->buildTransactionTimeline( - $cart, - new PhortuneCartTransactionQuery()); - $timeline - ->setShouldTerminate(true); + if (!$this->action) { + $class = 'phortune-cart-page'; + $timeline = $this->buildTransactionTimeline( + $cart, + new PhortuneCartTransactionQuery()); + $timeline + ->setShouldTerminate(true); - $view = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setCurtain($curtain) - ->setMainColumn(array( - $error_view, - $details, - $cart_box, - $description, - $charges, - $timeline, - )); + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setCurtain($curtain) + ->setMainColumn(array( + $error_view, + $details, + $cart_box, + $description, + $charges, + $timeline, + )); - return $this->newPage() + } else { + $class = 'phortune-invoice-view'; + $crumbs = null; + $merchant_phid = $cart->getMerchantPHID(); + $buyer_phid = $cart->getAuthorPHID(); + $merchant = id(new PhortuneMerchantQuery()) + ->setViewer($viewer) + ->withPHIDs(array($merchant_phid)) + ->needProfileImage(true) + ->executeOne(); + $buyer = id(new PhabricatorPeopleQuery()) + ->setViewer($viewer) + ->withPHIDs(array($buyer_phid)) + ->needProfileImage(true) + ->executeOne(); + // TODO: Add account "Contact" info + + $merchant_contact = new PHUIRemarkupView( + $viewer, $merchant->getContactInfo()); + $description = null; + + $view = id(new PhortuneInvoiceView()) + ->setMerchantName($merchant->getName()) + ->setMerchantLogo($merchant->getProfileImageURI()) + ->setMerchantContact($merchant_contact) + ->setMerchantFooter($merchant->getInvoiceFooter()) + ->setAccountName($buyer->getRealName()) + ->setStatus($error_view) + ->setContent(array( + $description, + $details, + $cart_box, + $charges, + )); + } + + $page = $this->newPage() ->setTitle(pht('Cart %d', $cart->getID())) - ->setCrumbs($crumbs) - ->addClass('phortune-cart-page') + ->addClass($class) ->appendChild($view); + if ($crumbs) { + $page->setCrumbs($crumbs); + } + return $page; } private function buildDetailsView(PhortuneCart $cart) { - $viewer = $this->getViewer(); - $view = id(new PHUIPropertyListView()) ->setUser($viewer) ->setObject($cart); @@ -229,9 +271,10 @@ final class PhortuneCartViewController $cart->getMerchantPHID(), )); - $view->addProperty( - pht('Order Name'), - $cart->getName()); + if ($this->action == 'print') { + $view->addProperty(pht('Order Name'), $cart->getName()); + } + $view->addProperty( pht('Account'), $handles[$cart->getAccountPHID()]->renderLink()); @@ -276,7 +319,7 @@ final class PhortuneCartViewController $refund_uri = $this->getApplicationURI("{$prefix}cart/{$id}/refund/"); $update_uri = $this->getApplicationURI("{$prefix}cart/{$id}/update/"); $accept_uri = $this->getApplicationURI("{$prefix}cart/{$id}/accept/"); - $print_uri = $this->getApplicationURI("{$prefix}cart/{$id}/?__print__=1"); + $print_uri = $this->getApplicationURI("{$prefix}cart/{$id}/print/"); $curtain->addAction( id(new PhabricatorActionView()) diff --git a/src/applications/phortune/controller/PhortuneMerchantViewController.php b/src/applications/phortune/controller/PhortuneMerchantViewController.php index 6c74ce8a06..2f5232158d 100644 --- a/src/applications/phortune/controller/PhortuneMerchantViewController.php +++ b/src/applications/phortune/controller/PhortuneMerchantViewController.php @@ -129,6 +129,13 @@ final class PhortuneMerchantViewController $view->addProperty(pht('Status'), $status_view); + $invoice_from = $merchant->getInvoiceEmail(); + if (!$invoice_from) { + $invoice_from = pht('No email address set'); + $invoice_from = phutil_tag('em', array(), $invoice_from); + } + $view->addProperty(pht('Invoice From'), $invoice_from); + $description = $merchant->getDescription(); if (strlen($description)) { $description = new PHUIRemarkupView($viewer, $description); @@ -147,6 +154,15 @@ final class PhortuneMerchantViewController $view->addTextContent($contact_info); } + $footer_info = $merchant->getInvoiceFooter(); + if (strlen($footer_info)) { + $footer_info = new PHUIRemarkupView($viewer, $footer_info); + $view->addSectionHeader( + pht('Invoice Footer'), + PHUIPropertyListView::ICON_SUMMARY); + $view->addTextContent($footer_info); + } + return id(new PHUIObjectBoxView()) ->setHeaderText(pht('Details')) ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) diff --git a/src/applications/phortune/editor/PhortuneMerchantEditEngine.php b/src/applications/phortune/editor/PhortuneMerchantEditEngine.php index 8c2356e6fe..bcc188f7e3 100644 --- a/src/applications/phortune/editor/PhortuneMerchantEditEngine.php +++ b/src/applications/phortune/editor/PhortuneMerchantEditEngine.php @@ -84,6 +84,21 @@ final class PhortuneMerchantEditEngine ->setTransactionType(PhortuneMerchantTransaction::TYPE_NAME) ->setValue($object->getName()), + id(new PhabricatorUsersEditField()) + ->setKey('members') + ->setAliases(array('memberPHIDs')) + ->setLabel(pht('Members')) + ->setUseEdgeTransactions(true) + ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) + ->setMetadataValue( + 'edge:type', + PhortuneMerchantHasMemberEdgeType::EDGECONST) + ->setDescription(pht('Initial merchant members.')) + ->setConduitDescription(pht('Set merchant members.')) + ->setConduitTypeDescription(pht('New list of members.')) + ->setInitialValue($object->getMemberPHIDs()) + ->setValue($member_phids), + id(new PhabricatorRemarkupEditField()) ->setKey('description') ->setLabel(pht('Description')) @@ -100,20 +115,22 @@ final class PhortuneMerchantEditEngine ->setTransactionType(PhortuneMerchantTransaction::TYPE_CONTACTINFO) ->setValue($object->getContactInfo()), - id(new PhabricatorUsersEditField()) - ->setKey('members') - ->setAliases(array('memberPHIDs')) - ->setLabel(pht('Members')) - ->setUseEdgeTransactions(true) - ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) - ->setMetadataValue( - 'edge:type', - PhortuneMerchantHasMemberEdgeType::EDGECONST) - ->setDescription(pht('Initial merchant members.')) - ->setConduitDescription(pht('Set merchant members.')) - ->setConduitTypeDescription(pht('New list of members.')) - ->setInitialValue($object->getMemberPHIDs()) - ->setValue($member_phids), + id(new PhabricatorTextEditField()) + ->setKey('invoiceEmail') + ->setLabel(pht('Invoice From Email')) + ->setDescription(pht('Email address invoices are sent from.')) + ->setConduitTypeDescription( + pht('Email address invoices are sent from.')) + ->setTransactionType(PhortuneMerchantTransaction::TYPE_INVOICEEMAIL) + ->setValue($object->getInvoiceEmail()), + + id(new PhabricatorRemarkupEditField()) + ->setKey('invoiceFooter') + ->setLabel(pht('Invoice Footer')) + ->setDescription(pht('Footer on invoice forms.')) + ->setConduitTypeDescription(pht('Footer on invoice forms.')) + ->setTransactionType(PhortuneMerchantTransaction::TYPE_INVOICEFOOTER) + ->setValue($object->getInvoiceFooter()), ); } diff --git a/src/applications/phortune/editor/PhortuneMerchantEditor.php b/src/applications/phortune/editor/PhortuneMerchantEditor.php index 2d35ddabfd..e2db3688e7 100644 --- a/src/applications/phortune/editor/PhortuneMerchantEditor.php +++ b/src/applications/phortune/editor/PhortuneMerchantEditor.php @@ -18,6 +18,8 @@ final class PhortuneMerchantEditor $types[] = PhortuneMerchantTransaction::TYPE_DESCRIPTION; $types[] = PhortuneMerchantTransaction::TYPE_CONTACTINFO; $types[] = PhortuneMerchantTransaction::TYPE_PICTURE; + $types[] = PhortuneMerchantTransaction::TYPE_INVOICEEMAIL; + $types[] = PhortuneMerchantTransaction::TYPE_INVOICEFOOTER; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDGE; @@ -34,6 +36,10 @@ final class PhortuneMerchantEditor return $object->getDescription(); case PhortuneMerchantTransaction::TYPE_CONTACTINFO: return $object->getContactInfo(); + case PhortuneMerchantTransaction::TYPE_INVOICEEMAIL: + return $object->getInvoiceEmail(); + case PhortuneMerchantTransaction::TYPE_INVOICEFOOTER: + return $object->getInvoiceFooter(); case PhortuneMerchantTransaction::TYPE_PICTURE: return $object->getProfileImagePHID(); } @@ -49,6 +55,8 @@ final class PhortuneMerchantEditor case PhortuneMerchantTransaction::TYPE_NAME: case PhortuneMerchantTransaction::TYPE_DESCRIPTION: case PhortuneMerchantTransaction::TYPE_CONTACTINFO: + case PhortuneMerchantTransaction::TYPE_INVOICEEMAIL: + case PhortuneMerchantTransaction::TYPE_INVOICEFOOTER: case PhortuneMerchantTransaction::TYPE_PICTURE: return $xaction->getNewValue(); } @@ -70,6 +78,12 @@ final class PhortuneMerchantEditor case PhortuneMerchantTransaction::TYPE_CONTACTINFO: $object->setContactInfo($xaction->getNewValue()); return; + case PhortuneMerchantTransaction::TYPE_INVOICEEMAIL: + $object->setInvoiceEmail($xaction->getNewValue()); + return; + case PhortuneMerchantTransaction::TYPE_INVOICEFOOTER: + $object->setInvoiceFooter($xaction->getNewValue()); + return; case PhortuneMerchantTransaction::TYPE_PICTURE: $object->setProfileImagePHID($xaction->getNewValue()); return; @@ -86,6 +100,8 @@ final class PhortuneMerchantEditor case PhortuneMerchantTransaction::TYPE_NAME: case PhortuneMerchantTransaction::TYPE_DESCRIPTION: case PhortuneMerchantTransaction::TYPE_CONTACTINFO: + case PhortuneMerchantTransaction::TYPE_INVOICEEMAIL: + case PhortuneMerchantTransaction::TYPE_INVOICEFOOTER: case PhortuneMerchantTransaction::TYPE_PICTURE: return; } @@ -116,6 +132,29 @@ final class PhortuneMerchantEditor $error->setIsMissingFieldError(true); $errors[] = $error; } + break; + case PhortuneMerchantTransaction::TYPE_INVOICEEMAIL: + foreach ($xactions as $xaction) { + switch ($xaction->getTransactionType()) { + case PhortuneMerchantTransaction::TYPE_INVOICEEMAIL: + $new_email = $xaction->getNewValue(); + break; + } + } + if (strlen($new_email)) { + $email = new PhutilEmailAddress($new_email); + $domain = $email->getDomainName(); + + if (!$domain) { + $error = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + pht('%s is not a valid email.', $new_email), + nonempty(last($xactions), null)); + + $errors[] = $error; + } + } break; } diff --git a/src/applications/phortune/storage/PhortuneMerchant.php b/src/applications/phortune/storage/PhortuneMerchant.php index b2117a2662..a57e5bd69e 100644 --- a/src/applications/phortune/storage/PhortuneMerchant.php +++ b/src/applications/phortune/storage/PhortuneMerchant.php @@ -9,6 +9,8 @@ final class PhortuneMerchant extends PhortuneDAO protected $viewPolicy; protected $description; protected $contactInfo; + protected $invoiceEmail; + protected $invoiceFooter; protected $profileImagePHID; private $memberPHIDs = self::ATTACHABLE; @@ -27,6 +29,8 @@ final class PhortuneMerchant extends PhortuneDAO 'name' => 'text255', 'description' => 'text', 'contactInfo' => 'text', + 'invoiceEmail' => 'text255', + 'invoiceFooter' => 'text', 'profileImagePHID' => 'phid?', ), ) + parent::getConfiguration(); diff --git a/src/applications/phortune/storage/PhortuneMerchantTransaction.php b/src/applications/phortune/storage/PhortuneMerchantTransaction.php index 48895d85ec..c5f8db6d43 100644 --- a/src/applications/phortune/storage/PhortuneMerchantTransaction.php +++ b/src/applications/phortune/storage/PhortuneMerchantTransaction.php @@ -6,6 +6,8 @@ final class PhortuneMerchantTransaction const TYPE_NAME = 'merchant:name'; const TYPE_DESCRIPTION = 'merchant:description'; const TYPE_CONTACTINFO = 'merchant:contactinfo'; + const TYPE_INVOICEEMAIL = 'merchant:invoiceemail'; + const TYPE_INVOICEFOOTER = 'merchant:invoicefooter'; const TYPE_PICTURE = 'merchant:picture'; public function getApplicationName() { @@ -48,6 +50,14 @@ final class PhortuneMerchantTransaction return pht( '%s updated the contact information for this merchant.', $this->renderHandleLink($author_phid)); + case self::TYPE_INVOICEEMAIL: + return pht( + '%s updated the invoice email for this merchant.', + $this->renderHandleLink($author_phid)); + case self::TYPE_INVOICEFOOTER: + return pht( + '%s updated the invoice footer for this merchant.', + $this->renderHandleLink($author_phid)); } return parent::getTitle(); @@ -58,6 +68,8 @@ final class PhortuneMerchantTransaction switch ($this->getTransactionType()) { case self::TYPE_DESCRIPTION: case self::TYPE_CONTACTINFO: + case self::TYPE_INVOICEEMAIL: + case self::TYPE_INVOICEFOOTER: return ($old === null); } return parent::shouldHide(); @@ -69,6 +81,10 @@ final class PhortuneMerchantTransaction return ($this->getOldValue() !== null); case self::TYPE_CONTACTINFO: return ($this->getOldValue() !== null); + case self::TYPE_INVOICEEMAIL: + return ($this->getOldValue() !== null); + case self::TYPE_INVOICEFOOTER: + return ($this->getOldValue() !== null); } return parent::hasChangeDetails(); diff --git a/src/applications/phortune/view/PhortuneInvoiceView.php b/src/applications/phortune/view/PhortuneInvoiceView.php new file mode 100644 index 0000000000..89ad3fd1bc --- /dev/null +++ b/src/applications/phortune/view/PhortuneInvoiceView.php @@ -0,0 +1,159 @@ +merchantName = $name; + return $this; + } + + public function setMerchantLogo($logo) { + $this->merchantLogo = $logo; + return $this; + } + + public function setMerchantContact($contact) { + $this->merchantContact = $contact; + return $this; + } + + public function setMerchantFooter($footer) { + $this->merchantFooter = $footer; + return $this; + } + + public function setAccountName($name) { + $this->accountName = $name; + return $this; + } + + public function setAccountContact($contact) { + $this->accountContact = $contact; + return $this; + } + + public function setStatus($status) { + $this->status = $status; + return $this; + } + + public function setContent($content) { + $this->content = $content; + return $this; + } + + protected function getTagAttributes() { + $classes = array(); + $classes[] = 'phortune-invoice-view'; + + return array( + 'class' => implode(' ', $classes), + ); + } + + protected function getTagContent() { + require_celerity_resource('phortune-invoice-css'); + + $logo = phutil_tag( + 'div', + array( + 'class' => 'phortune-invoice-logo', + ), + phutil_tag( + 'img', + array( + 'height' => '50', + 'width' => '50', + 'alt' => $this->merchantName, + 'src' => $this->merchantLogo, + ))); + + $to_title = phutil_tag( + 'div', + array( + 'class' => 'phortune-mini-header', + ), + pht('To:')); + + $bill_to = phutil_tag( + 'td', + array( + 'class' => 'phortune-invoice-to', + 'width' => '50%', + ), + array( + $to_title, + phutil_tag('strong', array(), $this->accountName), + phutil_tag('br', array()), + $this->accountContact, + )); + + $from_title = phutil_tag( + 'div', + array( + 'class' => 'phortune-mini-header', + ), + pht('From:')); + + $bill_from = phutil_tag( + 'td', + array( + 'class' => 'phortune-invoice-from', + 'width' => '50%', + ), + array( + $from_title, + phutil_tag('strong', array(), $this->merchantName), + phutil_tag('br', array()), + $this->merchantContact, + )); + + $contact = phutil_tag( + 'table', + array( + 'class' => 'phortune-invoice-contact', + 'width' => '100%', + ), + phutil_tag( + 'tr', + array(), + array( + $bill_to, + $bill_from, + ))); + + $status = null; + if ($this->status) { + $status = phutil_tag( + 'div', + array( + 'class' => 'phortune-invoice-status', + ), + $this->status); + } + + $footer = phutil_tag( + 'div', + array( + 'class' => 'phortune-invoice-footer', + ), + $this->merchantFooter); + + return array( + $logo, + $contact, + $status, + $this->content, + $footer, + ); + } +} diff --git a/webroot/rsrc/css/application/phortune/phortune-invoice.css b/webroot/rsrc/css/application/phortune/phortune-invoice.css new file mode 100644 index 0000000000..34bceb4bba --- /dev/null +++ b/webroot/rsrc/css/application/phortune/phortune-invoice.css @@ -0,0 +1,75 @@ +/** + * @provides phortune-invoice-css + */ + +.phortune-invoice-view { + max-width: 800px; + margin: 16px auto; + background: #fff; +} + +.phortune-invoice-view .phabricator-main-menu { + display: none; +} + +.phortune-invoice-view .phabricator-standard-page-footer { + display: none; +} + +.device-desktop .phortune-invoice-view .phui-property-list-key { + width: 16%; +} + +.device-desktop .phortune-invoice-view .phui-property-list-value { + width: 80%; +} + +.phortune-invoice-logo { + margin-bottom: 24px; +} + +.phortune-invoice-logo img { + margin: 0 auto; +} + +.phortune-invoice-contact { + margin-bottom: 32px; +} + +.phortune-invoice-contact td { + padding: 4px 16px; +} + +.phortune-invoice-to { + border-right: 1px solid {$lightblueborder}; +} + +.phortune-mini-header { + color: {$lightbluetext}; + font-weight: bold; + text-transform: uppercase; + margin-bottom: 4px; + letter-spacing: 0.3em; +} + +.phortune-invoice-status { + margin-bottom: 24px; +} + +.phortune-invoice-status .phui-info-view { + margin: 0; +} + +.phortune-invoice-view .phui-box.phui-object-box { + margin-bottom: 24px; +} + +.phortune-invoice-footer { + color: {$lightgreytext}; + margin: 48px 0 64px; + text-align: center; +} + +.phortune-invoice-footer strong { + color: #000; +} From 96b064b7e9b278dcf84959c5c6a29b2ff0a8f51b Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 30 Oct 2016 08:13:08 -0700 Subject: [PATCH 09/41] Properly import all-day events in Calendar Summary: Ref T10747. The transaction version of this copies the "all day" flag over properly, but this non-transaction version needs to copy it explicitly. Test Plan: Imported an all-day event, saw it come in as all-day. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10747 Differential Revision: https://secure.phabricator.com/D16770 --- .../calendar/import/PhabricatorCalendarImportEngine.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php index 74d3866fb7..1bb7df46d2 100644 --- a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php +++ b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php @@ -520,6 +520,8 @@ abstract class PhabricatorCalendarImportEngine ->setStartDateTime($start_datetime) ->setEndDateTime($end_datetime); + $event->setIsAllDay($start_datetime->getIsAllDay()); + // TODO: This should be transactional, but the transaction only accepts // simple frequency rules right now. $rrule = $node->getRecurrenceRule(); From d9c91f857cefe9167ac668966b60534a55f2e5d5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 30 Oct 2016 08:21:12 -0700 Subject: [PATCH 10/41] Apply a TYPE_CREATE transaction when importing events to improve strings in timeline Summary: Ref T10747. This turns on the newer EditEngine behavior so we get a nice "X created this event." transaction, instead of an "X renamed this from to Event Name." Test Plan: Imported an event, saw a nice timeline. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10747 Differential Revision: https://secure.phabricator.com/D16771 --- .../calendar/editor/PhabricatorCalendarEventEditor.php | 9 +++++++++ .../calendar/import/PhabricatorCalendarImportEngine.php | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php index d22d34bee7..bf63d8e8e7 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php @@ -11,6 +11,15 @@ final class PhabricatorCalendarEventEditor return pht('Calendar'); } + public function getCreateObjectTitle($author, $object) { + return pht('%s created this event.', $author); + } + + public function getCreateObjectTitleForFeed($author, $object) { + return pht('%s created %s.', $author, $object); + } + + protected function shouldApplyInitialEffects( PhabricatorLiskDAO $object, array $xactions) { diff --git a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php index 1bb7df46d2..96069e0ee1 100644 --- a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php +++ b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php @@ -471,6 +471,12 @@ abstract class PhabricatorCalendarImportEngine $xactions = array(); $uid = $node->getUID(); + if (!$event->getID()) { + $xactions[] = id(new PhabricatorCalendarEventTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_CREATE) + ->setNewValue(true); + } + $name = $node->getName(); if (!strlen($name)) { if (strlen($uid)) { From e1a9b7694517f5601c6e67b93d66b2b17a3de846 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 30 Oct 2016 09:09:20 -0700 Subject: [PATCH 11/41] Give organizers in ICS exports a dummy email address to placate Gmail Summary: Ref T10747. In the in-email ICS event card that Gmail shows, it has a "Who" field which reads "Unknown Organizer*" if the URI for the organizer isn't email-address-like. Previously, we used a URI like `https://phabricator.install.com/p/username`, which I think is OK as far as RFC 5545 is concerned, but Gmail doesn't like it. Instead, use `PHID-USER-asdfa@phabricator.install.com`, which doesn't go anywhere, but makes Gmail happy. Users don't normally see this URI anyway. Test Plan: Got a readable "Who" in Gmail when importing an event exported from Calendar: {F1890571} Reviewers: chad Reviewed By: chad Maniphest Tasks: T10747 Differential Revision: https://secure.phabricator.com/D16772 --- .../calendar/storage/PhabricatorCalendarEvent.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index c5d5c898f9..cb93e54554 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -758,8 +758,18 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO $host_handle = $handles[$host_phid]; $host_name = $host_handle->getFullName(); - $host_uri = $host_handle->getURI(); - $host_uri = PhabricatorEnv::getURI($host_uri); + + // NOTE: Gmail shows "Who: Unknown Organizer*" if the organizer URI does + // not look like an email address. Use a synthetic address so it shows + // the host name instead. + $install_uri = PhabricatorEnv::getProductionURI('/'); + $install_uri = new PhutilURI($install_uri); + + // This should possibly use "metamta.reply-handler-domain" instead, but + // we do not currently accept mail for users anyway, and that option may + // not be configured. + $mail_domain = $install_uri->getDomain(); + $host_uri = "mailto:{$host_phid}@{$mail_domain}"; $organizer = id(new PhutilCalendarUserNode()) ->setName($host_name) From 5e784c998baec8182ce60ddbb06bae142f87bd22 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 30 Oct 2016 09:15:43 -0700 Subject: [PATCH 12/41] Fix some extra "changed the start time of this event" transactions Summary: Ref T11326. Since we were missing an `(int)` cast here, the code ended up thinking that changing `12345` to `"12345"` was an edit. It isn't. Test Plan: Created/edited events, no more extra "changed start time from X to the same X" transaction clutter. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11326 Differential Revision: https://secure.phabricator.com/D16773 --- src/view/form/control/AphrontFormDateControlValue.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/view/form/control/AphrontFormDateControlValue.php b/src/view/form/control/AphrontFormDateControlValue.php index f9dc14b238..51942c20e7 100644 --- a/src/view/form/control/AphrontFormDateControlValue.php +++ b/src/view/form/control/AphrontFormDateControlValue.php @@ -182,7 +182,7 @@ final class AphrontFormDateControlValue extends Phobject { return null; } - return $datetime->format('U'); + return (int)$datetime->format('U'); } private function getTimeFormat() { From ee834c5958d0118708fbb7230a8facb9b19ec2b3 Mon Sep 17 00:00:00 2001 From: William Light Date: Mon, 31 Oct 2016 08:22:01 -0700 Subject: [PATCH 13/41] oauthserver: get client ID/secret from HTTP auth Summary: This adds the ability for Phabricator's OAuth server implementation to use HTTP basic auth for the client ID and secret and brings it in line with the OAuth 2.0 specification in this respect. Fixes T11794 Test Plan: Fixes my use case. Shouldn't impact other use-cases. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: 0, Korvin Maniphest Tasks: T11794 Differential Revision: https://secure.phabricator.com/D16763 --- .../PhabricatorOAuthServerTokenController.php | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/applications/oauthserver/controller/PhabricatorOAuthServerTokenController.php b/src/applications/oauthserver/controller/PhabricatorOAuthServerTokenController.php index 3b5ff72575..6fb2bfc334 100644 --- a/src/applications/oauthserver/controller/PhabricatorOAuthServerTokenController.php +++ b/src/applications/oauthserver/controller/PhabricatorOAuthServerTokenController.php @@ -18,11 +18,35 @@ final class PhabricatorOAuthServerTokenController $grant_type = $request->getStr('grant_type'); $code = $request->getStr('code'); $redirect_uri = $request->getStr('redirect_uri'); - $client_phid = $request->getStr('client_id'); - $client_secret = $request->getStr('client_secret'); $response = new PhabricatorOAuthResponse(); $server = new PhabricatorOAuthServer(); + $client_id_parameter = $request->getStr('client_id'); + $client_id_header = idx($_SERVER, 'PHP_AUTH_USER'); + if (strlen($client_id_parameter) && strlen($client_id_header)) { + if ($client_id_parameter !== $client_id_header) { + throw new Exception( + pht( + 'Request included a client_id parameter and an "Authorization" '. + 'header with a username, but the values "%s" and "%s") disagree. '. + 'The values must match.', + $client_id_parameter, + $client_id_header)); + } + } + + $client_secret_parameter = $request->getStr('client_secret'); + $client_secret_header = idx($_SERVER, 'PHP_AUTH_PW'); + if (strlen($client_secret_parameter)) { + // If the `client_secret` parameter is present, prefer parameters. + $client_phid = $client_id_parameter; + $client_secret = $client_secret_parameter; + } else { + // Otherwise, read values from the "Authorization" header. + $client_phid = $client_id_header; + $client_secret = $client_secret_header; + } + if ($grant_type != 'authorization_code') { $response->setError('unsupported_grant_type'); $response->setErrorDescription( From 850fcf0207588b2cbad5a4e8f1ce7fa5584b1d8f Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 31 Oct 2016 09:08:01 -0700 Subject: [PATCH 14/41] Move event recurrence controls to a separate page on the workflow Summary: Ref T11326. Currently, the "Create Event" form is pretty wordy. One particular culprit is the "recurring" controls, which are (presumably) rarely used and visually complex. - Reflow the default form to hopefully feel a little better. - Move recurrence stuff to a separate workflow. Test Plan: {F1891355} {F1891356} {F1891357} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11326 Differential Revision: https://secure.phabricator.com/D16774 --- ...PhabricatorCalendarEventViewController.php | 21 ++- .../PhabricatorCalendarEventEditEngine.php | 174 +++++++++--------- .../storage/PhabricatorCalendarEvent.php | 17 ++ ...catorCalendarEventRecurringTransaction.php | 9 +- 4 files changed, 133 insertions(+), 88 deletions(-) diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php index c8bc9b5593..a67bc8b741 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php @@ -146,6 +146,8 @@ final class PhabricatorCalendarEventViewController PhabricatorPolicyCapability::CAN_EDIT); $edit_uri = "event/edit/{$id}/"; + $edit_uri = $this->getApplicationURI($edit_uri); + if ($event->isChildEvent()) { $edit_label = pht('Edit This Instance'); } else { @@ -159,11 +161,28 @@ final class PhabricatorCalendarEventViewController id(new PhabricatorActionView()) ->setName($edit_label) ->setIcon('fa-pencil') - ->setHref($this->getApplicationURI($edit_uri)) + ->setHref($edit_uri) ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); } + $recurring_uri = "{$edit_uri}page/recurring/"; + $can_recurring = $can_edit && !$event->isChildEvent(); + + if ($event->getIsRecurring()) { + $recurring_label = pht('Edit Recurrence'); + } else { + $recurring_label = pht('Make Recurring'); + } + + $curtain->addAction( + id(new PhabricatorActionView()) + ->setName($recurring_label) + ->setIcon('fa-repeat') + ->setHref($recurring_uri) + ->setDisabled(!$can_recurring) + ->setWorkflow(true)); + $can_attend = !$event->isImportedEvent(); if ($is_attending) { diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php index fef14c13e1..318db2e2d2 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php @@ -67,12 +67,15 @@ final class PhabricatorCalendarEventEditEngine $invitee_phids = $object->getInviteePHIDsForEdit(); } - $frequency_options = array( - PhutilCalendarRecurrenceRule::FREQUENCY_DAILY => pht('Daily'), - PhutilCalendarRecurrenceRule::FREQUENCY_WEEKLY => pht('Weekly'), - PhutilCalendarRecurrenceRule::FREQUENCY_MONTHLY => pht('Monthly'), - PhutilCalendarRecurrenceRule::FREQUENCY_YEARLY => pht('Yearly'), - ); + $frequency_map = PhabricatorCalendarEvent::getFrequencyMap(); + $frequency_options = ipull($frequency_map, 'label'); + + $rrule = $object->newRecurrenceRule(); + if ($rrule) { + $frequency = $rrule->getFrequency(); + } else { + $frequency = null; + } $fields = array( id(new PhabricatorTextEditField()) @@ -85,15 +88,38 @@ final class PhabricatorCalendarEventEditEngine ->setConduitDescription(pht('Rename the event.')) ->setConduitTypeDescription(pht('New event name.')) ->setValue($object->getName()), - id(new PhabricatorRemarkupEditField()) - ->setKey('description') - ->setLabel(pht('Description')) - ->setDescription(pht('Description of the event.')) + id(new PhabricatorBoolEditField()) + ->setKey('isAllDay') + ->setLabel(pht('All Day')) + ->setOptions(pht('Normal Event'), pht('All Day Event')) ->setTransactionType( - PhabricatorCalendarEventDescriptionTransaction::TRANSACTIONTYPE) - ->setConduitDescription(pht('Update the event description.')) - ->setConduitTypeDescription(pht('New event description.')) - ->setValue($object->getDescription()), + PhabricatorCalendarEventAllDayTransaction::TRANSACTIONTYPE) + ->setDescription(pht('Marks this as an all day event.')) + ->setConduitDescription(pht('Make the event an all day event.')) + ->setConduitTypeDescription(pht('Mark the event as an all day event.')) + ->setValue($object->getIsAllDay()), + id(new PhabricatorEpochEditField()) + ->setKey('start') + ->setLabel(pht('Start')) + ->setIsLockable(false) + ->setIsDefaultable(false) + ->setTransactionType( + PhabricatorCalendarEventStartDateTransaction::TRANSACTIONTYPE) + ->setDescription(pht('Start time of the event.')) + ->setConduitDescription(pht('Change the start time of the event.')) + ->setConduitTypeDescription(pht('New event start time.')) + ->setValue($object->getStartDateTimeEpoch()), + id(new PhabricatorEpochEditField()) + ->setKey('end') + ->setLabel(pht('End')) + ->setIsLockable(false) + ->setIsDefaultable(false) + ->setTransactionType( + PhabricatorCalendarEventEndDateTransaction::TRANSACTIONTYPE) + ->setDescription(pht('End time of the event.')) + ->setConduitDescription(pht('Change the end time of the event.')) + ->setConduitTypeDescription(pht('New event end time.')) + ->setValue($object->getEndDateTimeEpoch()), id(new PhabricatorBoolEditField()) ->setKey('cancelled') ->setOptions(pht('Active'), pht('Cancelled')) @@ -128,10 +154,27 @@ final class PhabricatorCalendarEventEditEngine ->setConduitTypeDescription(pht('New event invitees.')) ->setValue($invitee_phids) ->setCommentActionLabel(pht('Change Invitees')), - ); - - if ($this->getIsCreate()) { - $fields[] = id(new PhabricatorBoolEditField()) + id(new PhabricatorRemarkupEditField()) + ->setKey('description') + ->setLabel(pht('Description')) + ->setDescription(pht('Description of the event.')) + ->setTransactionType( + PhabricatorCalendarEventDescriptionTransaction::TRANSACTIONTYPE) + ->setConduitDescription(pht('Update the event description.')) + ->setConduitTypeDescription(pht('New event description.')) + ->setValue($object->getDescription()), + id(new PhabricatorIconSetEditField()) + ->setKey('icon') + ->setLabel(pht('Icon')) + ->setIconSet(new PhabricatorCalendarIconSet()) + ->setTransactionType( + PhabricatorCalendarEventIconTransaction::TRANSACTIONTYPE) + ->setDescription(pht('Event icon.')) + ->setConduitDescription(pht('Change the event icon.')) + ->setConduitTypeDescription(pht('New event icon.')) + ->setValue($object->getIcon()), + id(new PhabricatorBoolEditField()) + ->setIsHidden($object->getIsRecurring()) ->setKey('isRecurring') ->setLabel(pht('Recurring')) ->setOptions(pht('One-Time Event'), pht('Recurring Event')) @@ -140,17 +183,8 @@ final class PhabricatorCalendarEventEditEngine ->setDescription(pht('One time or recurring event.')) ->setConduitDescription(pht('Make the event recurring.')) ->setConduitTypeDescription(pht('Mark the event as a recurring event.')) - ->setValue($object->getIsRecurring()); - - - $rrule = $object->newRecurrenceRule(); - if ($rrule) { - $frequency = $rrule->getFrequency(); - } else { - $frequency = null; - } - - $fields[] = id(new PhabricatorSelectEditField()) + ->setValue($object->getIsRecurring()), + id(new PhabricatorSelectEditField()) ->setKey('frequency') ->setLabel(pht('Frequency')) ->setOptions($frequency_options) @@ -159,14 +193,12 @@ final class PhabricatorCalendarEventEditEngine ->setDescription(pht('Recurring event frequency.')) ->setConduitDescription(pht('Change the event frequency.')) ->setConduitTypeDescription(pht('New event frequency.')) - ->setValue($frequency); - } - - if ($this->getIsCreate() || $object->getIsRecurring()) { - $fields[] = id(new PhabricatorEpochEditField()) + ->setValue($frequency), + id(new PhabricatorEpochEditField()) ->setIsLockable(false) ->setIsDefaultable(false) ->setAllowNull(true) + ->setHideTime($object->getIsAllDay()) ->setKey('until') ->setLabel(pht('Repeat Until')) ->setTransactionType( @@ -174,54 +206,8 @@ final class PhabricatorCalendarEventEditEngine ->setDescription(pht('Last instance of the event.')) ->setConduitDescription(pht('Change when the event repeats until.')) ->setConduitTypeDescription(pht('New final event time.')) - ->setValue($object->getUntilDateTimeEpoch()); - } - - $fields[] = id(new PhabricatorBoolEditField()) - ->setKey('isAllDay') - ->setLabel(pht('All Day')) - ->setOptions(pht('Normal Event'), pht('All Day Event')) - ->setTransactionType( - PhabricatorCalendarEventAllDayTransaction::TRANSACTIONTYPE) - ->setDescription(pht('Marks this as an all day event.')) - ->setConduitDescription(pht('Make the event an all day event.')) - ->setConduitTypeDescription(pht('Mark the event as an all day event.')) - ->setValue($object->getIsAllDay()); - - $fields[] = id(new PhabricatorEpochEditField()) - ->setKey('start') - ->setLabel(pht('Start')) - ->setIsLockable(false) - ->setIsDefaultable(false) - ->setTransactionType( - PhabricatorCalendarEventStartDateTransaction::TRANSACTIONTYPE) - ->setDescription(pht('Start time of the event.')) - ->setConduitDescription(pht('Change the start time of the event.')) - ->setConduitTypeDescription(pht('New event start time.')) - ->setValue($object->getStartDateTimeEpoch()); - - $fields[] = id(new PhabricatorEpochEditField()) - ->setKey('end') - ->setLabel(pht('End')) - ->setIsLockable(false) - ->setIsDefaultable(false) - ->setTransactionType( - PhabricatorCalendarEventEndDateTransaction::TRANSACTIONTYPE) - ->setDescription(pht('End time of the event.')) - ->setConduitDescription(pht('Change the end time of the event.')) - ->setConduitTypeDescription(pht('New event end time.')) - ->setValue($object->getEndDateTimeEpoch()); - - $fields[] = id(new PhabricatorIconSetEditField()) - ->setKey('icon') - ->setLabel(pht('Icon')) - ->setIconSet(new PhabricatorCalendarIconSet()) - ->setTransactionType( - PhabricatorCalendarEventIconTransaction::TRANSACTIONTYPE) - ->setDescription(pht('Event icon.')) - ->setConduitDescription(pht('Change the event icon.')) - ->setConduitTypeDescription(pht('New event icon.')) - ->setValue($object->getIcon()); + ->setValue($object->getUntilDateTimeEpoch()), + ); return $fields; } @@ -263,8 +249,28 @@ final class PhabricatorCalendarEventEditEngine } } - - return $fields; } + + protected function newPages($object) { + // Controls for event recurrence behavior go on a separate page which we + // put in a dialog. This simplifies event creation in the common case. + + return array( + id(new PhabricatorEditPage()) + ->setKey('core') + ->setLabel(pht('Core')) + ->setIsDefault(true), + id(new PhabricatorEditPage()) + ->setKey('recurring') + ->setLabel(pht('Recurrence')) + ->setFieldKeys( + array( + 'isRecurring', + 'frequency', + 'until', + )), + ); + } + } diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index cb93e54554..1410c0e106 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -461,6 +461,23 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return $this->assertAttached($this->invitees); } + public static function getFrequencyMap() { + return array( + PhutilCalendarRecurrenceRule::FREQUENCY_DAILY => array( + 'label' => pht('Daily'), + ), + PhutilCalendarRecurrenceRule::FREQUENCY_WEEKLY => array( + 'label' => pht('Weekly'), + ), + PhutilCalendarRecurrenceRule::FREQUENCY_MONTHLY => array( + 'label' => pht('Monthly'), + ), + PhutilCalendarRecurrenceRule::FREQUENCY_YEARLY => array( + 'label' => pht('Yearly'), + ), + ); + } + private function newStubInvitees() { $parent = $this->getParentEvent(); diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventRecurringTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventRecurringTransaction.php index 07aa26fa94..05f312ddc4 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventRecurringTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventRecurringTransaction.php @@ -30,11 +30,14 @@ final class PhabricatorCalendarEventRecurringTransaction continue; } + if ($xaction->getNewValue()) { + continue; + } + $errors[] = $this->newInvalidError( pht( - 'An event can only be made recurring when it is created. '. - 'You can not convert an existing event into a recurring '. - 'event or vice versa.'), + 'An event can not be stopped from recurring once it has been '. + 'made recurring. You can cancel the event.'), $xaction); } From 182611ef7e57d565c35e49806245d27d5489d5c0 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 31 Oct 2016 11:14:27 -0700 Subject: [PATCH 15/41] Schedule monthly events on the 29th, 30th or 31st relative to the end of the month Summary: Ref T11326. If you scheudle a monthly event on the 31st, the default behavior of RRULE means that it only occurs in months with 31 days. This is actually how Google Calendar and Calendar.app both work: if you schedule a monthly event on the 31st, you get about six events per year. This seems real confusing and bad to me? Instead, if the user schedules a monthly event on the 29th, 30th or 31st, pretend they scheduled it on the "last day of the month" or "second-to-last day of the month" or similar, so they always get 12 events per year. This could be slightly confusing too, but seems way less weird than not getting an event every month. Test Plan: Scheduled events on the 31st of October, saw them occur in November too after the patch. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11326 Differential Revision: https://secure.phabricator.com/D16775 --- ...catorCalendarEventFrequencyTransaction.php | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventFrequencyTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventFrequencyTransaction.php index 33c954ac9f..6fb8ae8f31 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventFrequencyTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventFrequencyTransaction.php @@ -19,6 +19,33 @@ final class PhabricatorCalendarEventFrequencyTransaction $rrule = id(new PhutilCalendarRecurrenceRule()) ->setFrequency($value); + // If the user creates a monthly event on the 29th, 30th or 31st of a + // month, it means "the 30th of every month" as far as the RRULE is + // concerned. Such an event will not occur on months with fewer days. + + // This is surprising, and proably not what the user wants. Instead, + // schedule these events relative to the end of the month: on the "-1st", + // "-2nd" or "-3rd" day of the month. For example, a monthly event on + // the 31st of a 31-day month translates to "every month, on the last + // day of the month". + if ($value == PhutilCalendarRecurrenceRule::FREQUENCY_MONTHLY) { + $start_datetime = $object->newStartDateTime(); + + $y = $start_datetime->getYear(); + $m = $start_datetime->getMonth(); + $d = $start_datetime->getDay(); + if ($d >= 29) { + $year_map = PhutilCalendarRecurrenceRule::getYearMap( + $y, + PhutilCalendarRecurrenceRule::WEEKDAY_MONDAY); + + $month_days = $year_map['monthDays'][$m]; + $schedule_on = -(($month_days + 1) - $d); + + $rrule->setByMonthDay(array($schedule_on)); + } + } + $object->setRecurrenceRule($rrule); } From f7b0c09ac47f95217ded8afbd8391af29d516e7b Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 31 Oct 2016 11:38:35 -0700 Subject: [PATCH 16/41] Make the "All Day Event" control use a checkbox instead of a dropdown Summary: This feels a little cleaner: - Clean up transaction log a bit. - Use a checkbox instead of a two-option dropdown. This is a little messy because the browser doesn't send anything if the user submits a form with an un-clicked checkbox. We now send a dummy value ("Hey, there's definitely a checkbox in this form!") so the server can figure out what to do. Test Plan: - Edited all-dayness of an event. - Viewed transaction log. Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D16776 --- resources/celerity/map.php | 4 +-- .../AphrontBoolHTTPParameterType.php | 19 ++++++++++++- .../PhabricatorCalendarEventEditEngine.php | 2 +- ...bricatorCalendarEventAllDayTransaction.php | 2 +- .../editfield/PhabricatorBoolEditField.php | 28 +++++++++++++++++-- .../PhabricatorApplicationTransaction.php | 12 ++++++-- .../control/AphrontFormCheckboxControl.php | 27 ++++++++++++++++++ .../calendar/behavior-event-all-day.js | 2 +- 8 files changed, 86 insertions(+), 10 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 8725524a29..99bc373146 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -382,7 +382,7 @@ return array( 'rsrc/js/application/aphlict/behavior-desktop-notifications-control.js' => 'edd1ba66', 'rsrc/js/application/auth/behavior-persona-login.js' => '9414ff18', 'rsrc/js/application/calendar/behavior-day-view.js' => '4b3c4443', - 'rsrc/js/application/calendar/behavior-event-all-day.js' => '937bb700', + 'rsrc/js/application/calendar/behavior-event-all-day.js' => 'b41537c9', 'rsrc/js/application/calendar/behavior-month-view.js' => 'fe33e256', 'rsrc/js/application/calendar/behavior-recurring-edit.js' => '5f1c4d5f', 'rsrc/js/application/config/behavior-reorder-fields.js' => 'b6993408', @@ -651,7 +651,7 @@ return array( 'javelin-behavior-editengine-reorder-configs' => 'd7a74243', 'javelin-behavior-editengine-reorder-fields' => 'b59e1e96', 'javelin-behavior-error-log' => '6882e80a', - 'javelin-behavior-event-all-day' => '937bb700', + 'javelin-behavior-event-all-day' => 'b41537c9', 'javelin-behavior-fancy-datepicker' => '568931f3', 'javelin-behavior-global-drag-and-drop' => '960f6a39', 'javelin-behavior-herald-rule-editor' => '7ebaeed3', diff --git a/src/aphront/httpparametertype/AphrontBoolHTTPParameterType.php b/src/aphront/httpparametertype/AphrontBoolHTTPParameterType.php index 4fc758ddda..43aec72a56 100644 --- a/src/aphront/httpparametertype/AphrontBoolHTTPParameterType.php +++ b/src/aphront/httpparametertype/AphrontBoolHTTPParameterType.php @@ -3,8 +3,21 @@ final class AphrontBoolHTTPParameterType extends AphrontHTTPParameterType { + protected function getParameterExists(AphrontRequest $request, $key) { + if ($request->getExists($key)) { + return true; + } + + $checkbox_key = $this->getCheckboxKey($key); + if ($request->getExists($checkbox_key)) { + return true; + } + + return false; + } + protected function getParameterValue(AphrontRequest $request, $key) { - return $request->getBool($key); + return (bool)$request->getBool($key); } protected function getParameterTypeName() { @@ -26,4 +39,8 @@ final class AphrontBoolHTTPParameterType ); } + public function getCheckboxKey($key) { + return "{$key}.exists"; + } + } diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php index 318db2e2d2..441c3c5de5 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php @@ -90,8 +90,8 @@ final class PhabricatorCalendarEventEditEngine ->setValue($object->getName()), id(new PhabricatorBoolEditField()) ->setKey('isAllDay') - ->setLabel(pht('All Day')) ->setOptions(pht('Normal Event'), pht('All Day Event')) + ->setAsCheckbox(true) ->setTransactionType( PhabricatorCalendarEventAllDayTransaction::TRANSACTIONTYPE) ->setDescription(pht('Marks this as an all day event.')) diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventAllDayTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventAllDayTransaction.php index eac229c395..325f53757a 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventAllDayTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventAllDayTransaction.php @@ -39,7 +39,7 @@ final class PhabricatorCalendarEventAllDayTransaction public function getTitle() { if ($this->getNewValue()) { return pht( - '%s changed this as an all day event.', + '%s changed this to an all day event.', $this->renderAuthor()); } else { return pht( diff --git a/src/applications/transactions/editfield/PhabricatorBoolEditField.php b/src/applications/transactions/editfield/PhabricatorBoolEditField.php index 11eabf2bad..4ce1afae43 100644 --- a/src/applications/transactions/editfield/PhabricatorBoolEditField.php +++ b/src/applications/transactions/editfield/PhabricatorBoolEditField.php @@ -4,6 +4,7 @@ final class PhabricatorBoolEditField extends PhabricatorEditField { private $options; + private $asCheckbox; public function setOptions($off_label, $on_label) { $this->options = array( @@ -17,6 +18,15 @@ final class PhabricatorBoolEditField return $this->options; } + public function setAsCheckbox($as_checkbox) { + $this->asCheckbox = $as_checkbox; + return $this; + } + + public function getAsCheckbox() { + return $this->asCheckbox; + } + protected function newControl() { $options = $this->getOptions(); @@ -27,8 +37,22 @@ final class PhabricatorBoolEditField ); } - return id(new AphrontFormSelectControl()) - ->setOptions($options); + if ($this->getAsCheckbox()) { + $key = $this->getKey(); + $value = $this->getValueForControl(); + $checkbox_key = $this->newHTTPParameterType() + ->getCheckboxKey($key); + $id = $this->getControlID(); + + $control = id(new AphrontFormCheckboxControl()) + ->setCheckboxKey($checkbox_key) + ->addCheckbox($key, '1', $options['1'], $value, $id); + } else { + $control = id(new AphrontFormSelectControl()) + ->setOptions($options); + } + + return $control; } protected function newHTTPParameterType() { diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index e130522bb2..cd0557dafe 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -553,8 +553,16 @@ abstract class PhabricatorApplicationTransaction return true; } - if (!is_array($old) && !strlen($old)) { - return true; + if (!is_array($old)) { + if (!strlen($old)) { + return true; + } + + // The integer 0 is also uninteresting by default; this is often + // an "off" flag for something like "All Day Event". + if ($old === 0) { + return true; + } } break; diff --git a/src/view/form/control/AphrontFormCheckboxControl.php b/src/view/form/control/AphrontFormCheckboxControl.php index 69be93ab9f..ed6cde3776 100644 --- a/src/view/form/control/AphrontFormCheckboxControl.php +++ b/src/view/form/control/AphrontFormCheckboxControl.php @@ -3,6 +3,16 @@ final class AphrontFormCheckboxControl extends AphrontFormControl { private $boxes = array(); + private $checkboxKey; + + public function setCheckboxKey($checkbox_key) { + $this->checkboxKey = $checkbox_key; + return $this; + } + + public function getCheckboxKey() { + return $this->checkboxKey; + } public function addCheckbox( $name, @@ -52,6 +62,23 @@ final class AphrontFormCheckboxControl extends AphrontFormControl { phutil_tag('th', array(), $label), )); } + + // When a user submits a form with a checkbox unchecked, the browser + // doesn't submit anything to the server. This hidden key lets the server + // know that the checkboxes were present on the client, the user just did + // not select any of them. + + $checkbox_key = $this->getCheckboxKey(); + if ($checkbox_key) { + $rows[] = phutil_tag( + 'input', + array( + 'type' => 'hidden', + 'name' => $checkbox_key, + 'value' => 1, + )); + } + return phutil_tag( 'table', array('class' => 'aphront-form-control-checkbox-layout'), diff --git a/webroot/rsrc/js/application/calendar/behavior-event-all-day.js b/webroot/rsrc/js/application/calendar/behavior-event-all-day.js index 1121759e3c..d9555060ba 100644 --- a/webroot/rsrc/js/application/calendar/behavior-event-all-day.js +++ b/webroot/rsrc/js/application/calendar/behavior-event-all-day.js @@ -6,7 +6,7 @@ JX.behavior('event-all-day', function(config) { var all_day = JX.$(config.allDayID); JX.DOM.listen(all_day, 'change', null, function() { - var is_all_day = !!parseInt(all_day.value, 10); + var is_all_day = !!all_day.checked; for (var ii = 0; ii < config.controlIDs.length; ii++) { var control = JX.$(config.controlIDs[ii]); From 8e5437226fe9424c46ae700b628d2fd021d55314 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 31 Oct 2016 11:51:31 -0700 Subject: [PATCH 17/41] Make calendar intepret all-day dates in a more consistent way Summary: In ICS, an event on "Nov 1" starts on "2016-11-01" and ends on "2016-11-02". This is convenient for computers, but this isn't what users expect to enter in date controls. They expect to enter "nov 1" to "Nov 1" for a one-day, all-day event. This is consistent with other applications. Store the value the user entered, but treat it as the first second of the next day when actually using it if the event is an all day event. Test Plan: Mucked around with multi-day all-day events, recurring all-day events, imports, etc. Couldn't catch any weird/unintuitive stuff anymore offhand. (Previously, entering "Nov 1" to "Nov 2" created a one-day event, which was unclear. Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D16777 --- .../PhabricatorCalendarEventEditEngine.php | 2 +- .../storage/PhabricatorCalendarEvent.php | 21 ++++++++++++++++++- ...ricatorCalendarEventEndDateTransaction.php | 2 +- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php index 441c3c5de5..4457f60fd8 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php @@ -119,7 +119,7 @@ final class PhabricatorCalendarEventEditEngine ->setDescription(pht('End time of the event.')) ->setConduitDescription(pht('Change the end time of the event.')) ->setConduitTypeDescription(pht('New event end time.')) - ->setValue($object->getEndDateTimeEpoch()), + ->setValue($object->newEndDateTimeForEdit()->getEpoch()), id(new PhabricatorBoolEditField()) ->setKey('cancelled') ->setOptions(pht('Active'), pht('Cancelled')) diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index 1410c0e106..ef7a808a61 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -857,7 +857,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return $this->newStartDateTime()->getEpoch(); } - public function newEndDateTime() { + public function newEndDateTimeForEdit() { $datetime = $this->getParameter('endDateTime'); if ($datetime) { return $this->newDateTimeFromDictionary($datetime); @@ -867,6 +867,25 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return $this->newDateTimeFromEpoch($epoch); } + public function newEndDateTime() { + $datetime = $this->newEndDateTimeForEdit(); + + // If this is an all day event, we move the end date time forward to the + // first second of the following day. This is consistent with what users + // expect: an all day event from "Nov 1" to "Nov 1" lasts the entire day. + if ($this->getIsAllDay()) { + $datetime = $datetime + ->newAbsoluteDateTime() + ->setHour(0) + ->setMinute(0) + ->setSecond(0) + ->newRelativeDateTime('P1D') + ->newAbsoluteDateTime(); + } + + return $datetime; + } + public function getEndDateTimeEpoch() { return $this->newEndDateTime()->getEpoch(); } diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php index a0d127c32f..8d66176ea8 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php @@ -7,7 +7,7 @@ final class PhabricatorCalendarEventEndDateTransaction public function generateOldValue($object) { // TODO: Upgrade this. - return $object->getEndDateTimeEpoch(); + return $object->newEndDateTimeForEdit()->getEpoch(); } public function applyInternalEffects($object, $value) { From 91089acbe56b0833e9a973a17722ef2c57e20b81 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 31 Oct 2016 12:47:39 -0700 Subject: [PATCH 18/41] Begin navigating the mess that is edits to recurring events Summary: Ref T11804. This puts us on a path toward some kind of reasonable behavior here. Currently, cancelling recurring events makes approximately zero sense ever in any situation. Instead, give users the choice to cancel just the instance, or all future events. This is similar to Calendar.app. (Google Calendar has a third option, "All Events", which I may implement). When the user picks something, basically do that. The particulars of "do that" are messy. We have to split the series into two different series, stop the first series early, then edit the second series. Then we need to update any concrete events that are now part of the second series. This code will get less junk in the next couple of diffs (I hope?) since I need to make it apply to edits, too, but this was a little easier to get started with. Test Plan: Cancelled an instance of an event; cancelled "All future events". Both of them more or less worked in a reasonble way. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11804 Differential Revision: https://secure.phabricator.com/D16778 --- src/__phutil_library_map__.php | 2 + ...abricatorCalendarEventCancelController.php | 186 ++++++++++++------ ...PhabricatorCalendarEventViewController.php | 16 +- .../query/PhabricatorCalendarEventQuery.php | 22 +++ .../storage/PhabricatorCalendarEvent.php | 19 +- ...habricatorCalendarEventForkTransaction.php | 59 ++++++ 6 files changed, 216 insertions(+), 88 deletions(-) create mode 100644 src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d91e588a16..a7d250eba3 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2049,6 +2049,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarEventEmailCommand' => 'applications/calendar/command/PhabricatorCalendarEventEmailCommand.php', 'PhabricatorCalendarEventEndDateTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php', 'PhabricatorCalendarEventExportController' => 'applications/calendar/controller/PhabricatorCalendarEventExportController.php', + 'PhabricatorCalendarEventForkTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php', 'PhabricatorCalendarEventFrequencyTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventFrequencyTransaction.php', 'PhabricatorCalendarEventFulltextEngine' => 'applications/calendar/search/PhabricatorCalendarEventFulltextEngine.php', 'PhabricatorCalendarEventHeraldAdapter' => 'applications/calendar/herald/PhabricatorCalendarEventHeraldAdapter.php', @@ -6889,6 +6890,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarEventEmailCommand' => 'MetaMTAEmailTransactionCommand', 'PhabricatorCalendarEventEndDateTransaction' => 'PhabricatorCalendarEventDateTransaction', 'PhabricatorCalendarEventExportController' => 'PhabricatorCalendarController', + 'PhabricatorCalendarEventForkTransaction' => 'PhabricatorCalendarEventTransactionType', 'PhabricatorCalendarEventFrequencyTransaction' => 'PhabricatorCalendarEventTransactionType', 'PhabricatorCalendarEventFulltextEngine' => 'PhabricatorFulltextEngine', 'PhabricatorCalendarEventHeraldAdapter' => 'HeraldAdapter', diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php b/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php index fbf7f9d45e..9f4e0e508e 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php @@ -32,88 +32,152 @@ final class PhabricatorCalendarEventCancelController $is_parent = $event->isParentEvent(); $is_child = $event->isChildEvent(); - $is_cancelled = $event->getIsCancelled(); - if ($is_child) { - $is_parent_cancelled = $event->getParentEvent()->getIsCancelled(); - } else { - $is_parent_cancelled = false; - } + $is_cancelled = $event->getIsCancelled(); + $is_recurring = $event->getIsRecurring(); $validation_exception = null; if ($request->isFormPost()) { - $xactions = array(); - $xaction = id(new PhabricatorCalendarEventTransaction()) - ->setTransactionType( - PhabricatorCalendarEventCancelTransaction::TRANSACTIONTYPE) - ->setNewValue(!$is_cancelled); + $targets = array(); + if ($is_recurring) { + $mode = $request->getStr('mode'); + $is_future = ($mode == 'future'); - $editor = id(new PhabricatorCalendarEventEditor()) - ->setActor($viewer) - ->setContentSourceFromRequest($request) - ->setContinueOnNoEffect(true) - ->setContinueOnMissingFields(true); + // We need to fork the event if we're cancelling just the parent, or + // are cancelling a child and all future events. + $must_fork = ($is_child && $is_future) || + ($is_parent && !$is_future); - try { - $editor->applyTransactions($event, array($xaction)); + if ($must_fork) { + if ($is_child) { + $xactions = array(); + + $xaction = id(new PhabricatorCalendarEventTransaction()) + ->setTransactionType( + PhabricatorCalendarEventForkTransaction::TRANSACTIONTYPE) + ->setNewValue(true); + + $editor = id(new PhabricatorCalendarEventEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true); + + $editor->applyTransactions($event, array($xaction)); + + $targets[] = $event; + } else { + // TODO: This is a huge mess; we need to load or generate the + // first child, then fork that, then apply the event to the + // parent. Just bail for now. + throw new Exception( + pht( + 'Individual edits to parent events are not yet supported '. + 'because they are real tricky to implement.')); + } + } else { + $targets[] = $event; + } + + if ($is_future) { + // NOTE: If you can't edit some of the future events, we just + // don't try to update them. This seems like it's probably what + // users are likely to expect. + $future = id(new PhabricatorCalendarEventQuery()) + ->setViewer($viewer) + ->withParentEventPHIDs(array($event->getPHID())) + ->withUTCInitialEpochBetween($event->getUTCInitialEpoch(), null) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->execute(); + foreach ($future as $future_event) { + $targets[] = $future_event; + } + } + } else { + $targets = array($event); + } + + foreach ($targets as $target) { + $xactions = array(); + + $xaction = id(new PhabricatorCalendarEventTransaction()) + ->setTransactionType( + PhabricatorCalendarEventCancelTransaction::TRANSACTIONTYPE) + ->setNewValue(!$is_cancelled); + + $editor = id(new PhabricatorCalendarEventEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true); + + try { + $editor->applyTransactions($target, array($xaction)); + } catch (PhabricatorApplicationTransactionValidationException $ex) { + $validation_exception = $ex; + break; + } + + } + + if (!$validation_exception) { return id(new AphrontRedirectResponse())->setURI($cancel_uri); - } catch (PhabricatorApplicationTransactionValidationException $ex) { - $validation_exception = $ex; } } if ($is_cancelled) { - if ($is_parent_cancelled) { - $title = pht('Cannot Reinstate Instance'); - $paragraph = pht( - 'You cannot reinstate an instance of a cancelled recurring event.'); - $cancel = pht('Back'); - $submit = null; - } else if ($is_child) { - $title = pht('Reinstate Instance'); - $paragraph = pht( - 'Reinstate this instance of this recurring event?'); - $cancel = pht('Back'); - $submit = pht('Reinstate Instance'); - } else if ($is_parent) { - $title = pht('Reinstate Recurring Event'); - $paragraph = pht( - 'Reinstate all instances of this recurring event which have not '. - 'been individually cancelled?'); - $cancel = pht('Back'); - $submit = pht('Reinstate Recurring Event'); + $title = pht('Reinstate Event'); + if ($is_recurring) { + $body = pht( + 'This event is part of a series. Which events do you want to '. + 'reinstate?'); + $show_control = true; } else { - $title = pht('Reinstate Event'); - $paragraph = pht('Reinstate this event?'); - $cancel = pht('Back'); - $submit = pht('Reinstate Event'); + $body = pht('Reinstate this event?'); + $show_control = false; } + $submit = pht('Reinstate Event'); } else { - if ($is_child) { - $title = pht('Cancel Instance'); - $paragraph = pht('Cancel this instance of this recurring event?'); - $cancel = pht('Back'); - $submit = pht('Cancel Instance'); - } else if ($is_parent) { - $title = pht('Cancel Recurrin Event'); - $paragraph = pht('Cancel this entire series of recurring events?'); - $cancel = pht('Back'); - $submit = pht('Cancel Recurring Event'); + $title = pht('Cancel Event'); + if ($is_recurring) { + $body = pht( + 'This event is part of a series. Which events do you want to '. + 'cancel?'); + $show_control = true; } else { - $title = pht('Cancel Event'); - $paragraph = pht( - 'Cancel this event? You can always reinstate the event later.'); - $cancel = pht('Back'); - $submit = pht('Cancel Event'); + $body = pht('Cancel this event?'); + $show_control = false; } + $submit = pht('Cancel Event'); } - return $this->newDialog() + $dialog = $this->newDialog() ->setTitle($title) ->setValidationException($validation_exception) - ->appendParagraph($paragraph) - ->addCancelButton($cancel_uri, $cancel) + ->appendParagraph($body) + ->addCancelButton($cancel_uri, pht('Back')) ->addSubmitButton($submit); + + if ($show_control) { + $form = id(new AphrontFormView()) + ->setViewer($viewer) + ->appendControl( + id(new AphrontFormSelectControl()) + ->setLabel(pht('Cancel Events')) + ->setName('mode') + ->setOptions( + array( + 'this' => pht('Only This Event'), + 'future' => pht('All Future Events'), + ))); + $dialog->appendForm($form); + } + + return $dialog; } } diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php index a67bc8b741..e0d17d55a7 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php @@ -206,20 +206,8 @@ final class PhabricatorCalendarEventViewController $cancel_uri = $this->getApplicationURI("event/cancel/{$id}/"); $cancel_disabled = !$can_edit; - if ($event->isChildEvent()) { - $cancel_label = pht('Cancel This Instance'); - $reinstate_label = pht('Reinstate This Instance'); - - if ($event->getParentEvent()->getIsCancelled()) { - $cancel_disabled = true; - } - } else if ($event->isParentEvent()) { - $cancel_label = pht('Cancel All'); - $reinstate_label = pht('Reinstate All'); - } else { - $cancel_label = pht('Cancel Event'); - $reinstate_label = pht('Reinstate Event'); - } + $cancel_label = pht('Cancel Event'); + $reinstate_label = pht('Reinstate Event'); if ($event->isCancelledEvent()) { $curtain->addAction( diff --git a/src/applications/calendar/query/PhabricatorCalendarEventQuery.php b/src/applications/calendar/query/PhabricatorCalendarEventQuery.php index 5c6b34c9fb..a6ac2e4ffc 100644 --- a/src/applications/calendar/query/PhabricatorCalendarEventQuery.php +++ b/src/applications/calendar/query/PhabricatorCalendarEventQuery.php @@ -17,6 +17,8 @@ final class PhabricatorCalendarEventQuery private $importSourcePHIDs; private $importAuthorPHIDs; private $importUIDs; + private $utcInitialEpochMin; + private $utcInitialEpochMax; private $generateGhosts = false; @@ -45,6 +47,12 @@ final class PhabricatorCalendarEventQuery return $this; } + public function withUTCInitialEpochBetween($min, $max) { + $this->utcInitialEpochMin = $min; + $this->utcInitialEpochMax = $max; + return $this; + } + public function withInvitedPHIDs(array $phids) { $this->inviteePHIDs = $phids; return $this; @@ -371,6 +379,20 @@ final class PhabricatorCalendarEventQuery $this->rangeEnd + phutil_units('16 hours in seconds')); } + if ($this->utcInitialEpochMin !== null) { + $where[] = qsprintf( + $conn, + 'event.utcInitialEpoch >= %d', + $this->utcInitialEpochMin); + } + + if ($this->utcInitialEpochMax !== null) { + $where[] = qsprintf( + $conn, + 'event.utcInitialEpoch <= %d', + $this->utcInitialEpochMax); + } + if ($this->inviteePHIDs !== null) { $where[] = qsprintf( $conn, diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index ef7a808a61..3459601b0a 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -165,6 +165,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO 'editPolicy' => true, 'name' => true, 'description' => true, + 'isCancelled' => true, ); // Read these fields from the parent event instead of this event. For @@ -204,7 +205,8 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO ->setViewPolicy($parent->getViewPolicy()) ->setEditPolicy($parent->getEditPolicy()) ->setName($parent->getName()) - ->setDescription($parent->getDescription()); + ->setDescription($parent->getDescription()) + ->setIsCancelled($parent->getIsCancelled()); if ($start) { $start_datetime = $start; @@ -569,7 +571,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return $this->assertAttached($this->parentEvent); } - public function attachParentEvent($event) { + public function attachParentEvent(PhabricatorCalendarEvent $event = null) { $this->parentEvent = $event; return $this; } @@ -583,17 +585,8 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO } public function isCancelledEvent() { - if ($this->getIsCancelled()) { - return true; - } - - if ($this->isChildEvent()) { - if ($this->getParentEvent()->getIsCancelled()) { - return true; - } - } - - return false; + // TODO: Remove this wrapper. + return $this->getIsCancelled(); } public function renderEventDate( diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php new file mode 100644 index 0000000000..ad7692dafe --- /dev/null +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php @@ -0,0 +1,59 @@ +getParentEvent(); + + $object->setInstanceOfEventPHID(null); + $object->attachParentEvent(null); + + $rrule = $parent->newRecurrenceRule(); + $object->setRecurrenceRule($rrule); + + $until = $parent->newUntilDateTime(); + if ($until) { + $object->setUntilDateTime($until); + } + + $old_sequence_index = $object->getSequenceIndex(); + $object->setSequenceIndex(0); + + // Stop the parent event from recurring after the start date of this event. + $parent->setUntilDateTime($object->newStartDateTime()); + $parent->save(); + + // NOTE: If we implement "COUNT" on editable events, we need to adjust + // the "COUNT" here and divide it up between the parent and the fork. + + // Make all following children of the old parent children of this node + // instead. + $conn = $object->establishConnection('w'); + queryfx( + $conn, + 'UPDATE %T SET + instanceOfEventPHID = %s, + sequenceIndex = (sequenceIndex - %d) + WHERE instanceOfEventPHID = %s + AND utcInstanceEpoch > %d', + $object->getTableName(), + $object->getPHID(), + $old_sequence_index, + $parent->getPHID(), + $object->getUTCInstanceEpoch()); + } + +} From f44a9a4e48963e326ccc53c4cecc11332860fe56 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 31 Oct 2016 13:42:10 -0700 Subject: [PATCH 19/41] Remove "isCancelledEvent()" wrapper on Calendar Events Summary: Ref T11804. The field now reads the correct value directly and we don't need this wrapper. Test Plan: Poked around Calendar without explosions. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11804 Differential Revision: https://secure.phabricator.com/D16779 --- .../PhabricatorCalendarEventViewController.php | 4 ++-- .../phid/PhabricatorCalendarEventPHIDType.php | 2 +- .../query/PhabricatorCalendarEventSearchEngine.php | 6 +++--- .../calendar/storage/PhabricatorCalendarEvent.php | 11 +++-------- 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php index e0d17d55a7..0e253ed994 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php @@ -101,7 +101,7 @@ final class PhabricatorCalendarEventViewController $viewer = $this->getViewer(); $id = $event->getID(); - if ($event->isCancelledEvent()) { + if ($event->getIsCancelled()) { $icon = 'fa-ban'; $color = 'red'; $status = pht('Cancelled'); @@ -209,7 +209,7 @@ final class PhabricatorCalendarEventViewController $cancel_label = pht('Cancel Event'); $reinstate_label = pht('Reinstate Event'); - if ($event->isCancelledEvent()) { + if ($event->getIsCancelled()) { $curtain->addAction( id(new PhabricatorActionView()) ->setName($reinstate_label) diff --git a/src/applications/calendar/phid/PhabricatorCalendarEventPHIDType.php b/src/applications/calendar/phid/PhabricatorCalendarEventPHIDType.php index 90beff28f9..fd8e86a1f2 100644 --- a/src/applications/calendar/phid/PhabricatorCalendarEventPHIDType.php +++ b/src/applications/calendar/phid/PhabricatorCalendarEventPHIDType.php @@ -41,7 +41,7 @@ final class PhabricatorCalendarEventPHIDType extends PhabricatorPHIDType { ->setFullName(pht('%s: %s', $monogram, $name)) ->setURI($uri); - if ($event->isCancelledEvent()) { + if ($event->getIsCancelled()) { $handle->setStatus(PhabricatorObjectHandle::STATUS_CLOSED); } } diff --git a/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php b/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php index 26c3dc938b..37c4cc94f0 100644 --- a/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php +++ b/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php @@ -303,7 +303,7 @@ final class PhabricatorCalendarEventSearchEngine $item->addAttribute($event->renderEventDate($viewer, false)); - if ($event->isCancelledEvent()) { + if ($event->getIsCancelled()) { $item->setDisabled(true); } @@ -368,7 +368,7 @@ final class PhabricatorCalendarEventSearchEngine $event_view = id(new AphrontCalendarEventView()) ->setHostPHID($event->getHostPHID()) ->setEpochRange($epoch_min, $epoch_max) - ->setIsCancelled($event->isCancelledEvent()) + ->setIsCancelled($event->getIsCancelled()) ->setName($event->getName()) ->setURI($event->getURI()) ->setIsAllDay($event->getIsAllDay()) @@ -441,7 +441,7 @@ final class PhabricatorCalendarEventSearchEngine ->setIconColor($status_color) ->setName($event->getName()) ->setURI($event->getURI()) - ->setIsCancelled($event->isCancelledEvent()); + ->setIsCancelled($event->getIsCancelled()); $day_view->addEvent($event_view); } diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index 3459601b0a..7ebcbced72 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -584,11 +584,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return ($this->instanceOfEventPHID !== null); } - public function isCancelledEvent() { - // TODO: Remove this wrapper. - return $this->getIsCancelled(); - } - public function renderEventDate( PhabricatorUser $viewer, $show_end) { @@ -641,7 +636,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO public function getDisplayIcon(PhabricatorUser $viewer) { - if ($this->isCancelledEvent()) { + if ($this->getIsCancelled()) { return 'fa-times'; } @@ -665,7 +660,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO } public function getDisplayIconColor(PhabricatorUser $viewer) { - if ($this->isCancelledEvent()) { + if ($this->getIsCancelled()) { return 'red'; } @@ -689,7 +684,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO } public function getDisplayIconLabel(PhabricatorUser $viewer) { - if ($this->isCancelledEvent()) { + if ($this->getIsCancelled()) { return pht('Cancelled'); } From b084efb36290cb19d37e6db2145fd4edf30cd46c Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 31 Oct 2016 13:47:45 -0700 Subject: [PATCH 20/41] Record a "series parent PHID" on Calendar events that retains relationships after forks Summary: When you edit "X and all future events", X becomes the new parent of an event series. Currently, it loses its relationship to its original parent. Instead, retain that relationship -- it's separate from the normal "parent", but we can use it to make the UI more clear or tweak behaviors later. This mostly just keeps us from losing/destroying data that we might need/want later. Test Plan: - Ran migrations. - Cancelled "X and all future events", saw sensible-appearing beahvior in the database for "seriesParentPHID". Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D16780 --- .../autopatches/20161031.calendar.01.seriesparent.sql | 5 +++++ .../PhabricatorCalendarEventCancelController.php | 7 +++++++ .../calendar/storage/PhabricatorCalendarEvent.php | 11 +++++++++++ 3 files changed, 23 insertions(+) create mode 100644 resources/sql/autopatches/20161031.calendar.01.seriesparent.sql diff --git a/resources/sql/autopatches/20161031.calendar.01.seriesparent.sql b/resources/sql/autopatches/20161031.calendar.01.seriesparent.sql new file mode 100644 index 0000000000..7589a161bc --- /dev/null +++ b/resources/sql/autopatches/20161031.calendar.01.seriesparent.sql @@ -0,0 +1,5 @@ +ALTER TABLE {$NAMESPACE}_calendar.calendar_event + ADD seriesParentPHID VARBINARY(64); + +UPDATE {$NAMESPACE}_calendar.calendar_event + SET seriesParentPHID = instanceOfEventPHID; diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php b/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php index 9f4e0e508e..447e3a3c74 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php @@ -84,6 +84,13 @@ final class PhabricatorCalendarEventCancelController // NOTE: If you can't edit some of the future events, we just // don't try to update them. This seems like it's probably what // users are likely to expect. + + // NOTE: This only affects events that are currently in the same + // series, not all events that were ever in the original series. + // We could use series PHIDs instead of parent PHIDs to affect more + // events if this turns out to be counterintuitive. Other + // applications differ in their behavior. + $future = id(new PhabricatorCalendarEventQuery()) ->setViewer($viewer) ->withParentEventPHIDs(array($event->getPHID())) diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index 7ebcbced72..1a629e07cb 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -27,6 +27,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO protected $isRecurring = 0; + protected $seriesParentPHID; protected $instanceOfEventPHID; protected $sequenceIndex; @@ -140,10 +141,16 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO 'a recurring parent event!')); } + $series_phid = $this->getSeriesParentPHID(); + if (!$series_phid) { + $series_phid = $this->getPHID(); + } + $child = id(new self()) ->setIsCancelled(0) ->setIsStub(0) ->setInstanceOfEventPHID($this->getPHID()) + ->setSeriesParentPHID($series_phid) ->setSequenceIndex($sequence) ->setIsRecurring(true) ->attachParentEvent($this) @@ -401,6 +408,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO 'icon' => 'text32', 'mailKey' => 'bytes20', 'isRecurring' => 'bool', + 'seriesParentPHID' => 'phid?', 'instanceOfEventPHID' => 'phid?', 'sequenceIndex' => 'uint32?', 'isStub' => 'bool', @@ -435,6 +443,9 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO 'columns' => array('instanceOfEventPHID', 'utcInstanceEpoch'), 'unique' => true, ), + 'key_series' => array( + 'columns' => array('seriesParentPHID', 'utcInitialEpoch'), + ), ), self::CONFIG_SERIALIZATION => array( 'recurrenceFrequency' => self::SERIALIZATION_JSON, From 208f8ed526a1543249ae481e0258c7f068c93037 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 31 Oct 2016 14:10:53 -0700 Subject: [PATCH 21/41] Support "Edit just this event" on the parent event in a series Summary: Ref T11804. This one is messy because we have to fork the //next// event, possibly creating it first. Then we can edit the parent normally. Test Plan: Cancelled the first event in a series, only that one cancelled. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11804 Differential Revision: https://secure.phabricator.com/D16781 --- ...abricatorCalendarEventCancelController.php | 50 +++++++++++++------ 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php b/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php index 447e3a3c74..9766db3f79 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php @@ -39,7 +39,7 @@ final class PhabricatorCalendarEventCancelController $validation_exception = null; if ($request->isFormPost()) { - $targets = array(); + $targets = array($event); if ($is_recurring) { $mode = $request->getStr('mode'); $is_future = ($mode == 'future'); @@ -51,6 +51,38 @@ final class PhabricatorCalendarEventCancelController if ($must_fork) { if ($is_child) { + $fork_target = $event; + } else { + if ($event->isValidSequenceIndex($viewer, 1)) { + $next_event = id(new PhabricatorCalendarEventQuery()) + ->setViewer($viewer) + ->withInstanceSequencePairs( + array( + array($event->getPHID(), 1), + )) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->executeOne(); + + if (!$next_event) { + $next_event = $event->newStub($viewer, 1); + } + + $fork_target = $next_event; + } else { + // This appears to be a "recurring" event with no valid + // instances: for example, its "until" date is before the second + // instance would occur. This can happen if we already forked the + // event or if users entered silly stuff. Just edit the event + // directly without forking anything. + $fork_target = null; + } + } + + if ($fork_target) { $xactions = array(); $xaction = id(new PhabricatorCalendarEventTransaction()) @@ -64,20 +96,8 @@ final class PhabricatorCalendarEventCancelController ->setContinueOnNoEffect(true) ->setContinueOnMissingFields(true); - $editor->applyTransactions($event, array($xaction)); - - $targets[] = $event; - } else { - // TODO: This is a huge mess; we need to load or generate the - // first child, then fork that, then apply the event to the - // parent. Just bail for now. - throw new Exception( - pht( - 'Individual edits to parent events are not yet supported '. - 'because they are real tricky to implement.')); + $editor->applyTransactions($fork_target, array($xaction)); } - } else { - $targets[] = $event; } if ($is_future) { @@ -105,8 +125,6 @@ final class PhabricatorCalendarEventCancelController $targets[] = $future_event; } } - } else { - $targets = array($event); } foreach ($targets as $target) { From a0ea31f47f26819792a84da0f357c4b763ccf701 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 31 Oct 2016 14:56:10 -0700 Subject: [PATCH 22/41] When users edit recurring events, prompt to "Edit This Event" or "Edit All Future Events" Summary: Fixes T11804. This probably isn't perfect but seems to work fairly reasonably and not be as much of a weird nonsense mess like the old behavior was. When a user edits a recurring event, we ask them what they're trying to do. Then we more or less do that. Test Plan: - Edited an event in the middle of a series. - Edited the first event in a series. - Edited "just this" and "all future" events in various places in a series. - Edited normal events. - Cancelled various events. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11804 Differential Revision: https://secure.phabricator.com/D16782 --- ...abricatorCalendarEventCancelController.php | 55 +---------- ...PhabricatorCalendarEventEditController.php | 55 ++++++++++- ...PhabricatorCalendarEventViewController.php | 10 +- .../PhabricatorCalendarEventEditEngine.php | 93 +++++++++++++++++++ .../storage/PhabricatorCalendarEvent.php | 64 +++++++++++++ ...habricatorCalendarEventForkTransaction.php | 9 +- .../editengine/PhabricatorEditEngine.php | 11 +++ 7 files changed, 233 insertions(+), 64 deletions(-) diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php b/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php index 9766db3f79..cf4cde5308 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php @@ -48,40 +48,8 @@ final class PhabricatorCalendarEventCancelController // are cancelling a child and all future events. $must_fork = ($is_child && $is_future) || ($is_parent && !$is_future); - if ($must_fork) { - if ($is_child) { - $fork_target = $event; - } else { - if ($event->isValidSequenceIndex($viewer, 1)) { - $next_event = id(new PhabricatorCalendarEventQuery()) - ->setViewer($viewer) - ->withInstanceSequencePairs( - array( - array($event->getPHID(), 1), - )) - ->requireCapabilities( - array( - PhabricatorPolicyCapability::CAN_VIEW, - PhabricatorPolicyCapability::CAN_EDIT, - )) - ->executeOne(); - - if (!$next_event) { - $next_event = $event->newStub($viewer, 1); - } - - $fork_target = $next_event; - } else { - // This appears to be a "recurring" event with no valid - // instances: for example, its "until" date is before the second - // instance would occur. This can happen if we already forked the - // event or if users entered silly stuff. Just edit the event - // directly without forking anything. - $fork_target = null; - } - } - + $fork_target = $event->loadForkTarget($viewer); if ($fork_target) { $xactions = array(); @@ -101,26 +69,7 @@ final class PhabricatorCalendarEventCancelController } if ($is_future) { - // NOTE: If you can't edit some of the future events, we just - // don't try to update them. This seems like it's probably what - // users are likely to expect. - - // NOTE: This only affects events that are currently in the same - // series, not all events that were ever in the original series. - // We could use series PHIDs instead of parent PHIDs to affect more - // events if this turns out to be counterintuitive. Other - // applications differ in their behavior. - - $future = id(new PhabricatorCalendarEventQuery()) - ->setViewer($viewer) - ->withParentEventPHIDs(array($event->getPHID())) - ->withUTCInitialEpochBetween($event->getUTCInitialEpoch(), null) - ->requireCapabilities( - array( - PhabricatorPolicyCapability::CAN_VIEW, - PhabricatorPolicyCapability::CAN_EDIT, - )) - ->execute(); + $future = $event->loadFutureEvents($viewer); foreach ($future as $future_event) { $targets[] = $future_event; } diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php b/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php index 7c01e795b7..3211d8052b 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php @@ -6,6 +6,9 @@ final class PhabricatorCalendarEventEditController public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); + $engine = id(new PhabricatorCalendarEventEditEngine()) + ->setController($this); + $id = $request->getURIData('id'); if ($id) { $event = id(new PhabricatorCalendarEventQuery()) @@ -16,11 +19,57 @@ final class PhabricatorCalendarEventEditController if ($response) { return $response; } + + $cancel_uri = $event->getURI(); + + $page = $request->getURIData('pageKey'); + if ($page == 'recurring') { + if ($event->isChildEvent()) { + return $this->newDialog() + ->setTitle(pht('Series Event')) + ->appendParagraph( + pht( + 'This event is an instance in an event series. To change '. + 'the behavior for the series, edit the parent event.')) + ->addCancelButton($cancel_uri); + } + } else if ($event->getIsRecurring()) { + $mode = $request->getStr('mode'); + if (!$mode) { + $form = id(new AphrontFormView()) + ->setViewer($viewer) + ->appendControl( + id(new AphrontFormSelectControl()) + ->setLabel(pht('Edit Events')) + ->setName('mode') + ->setOptions( + array( + PhabricatorCalendarEventEditEngine::MODE_THIS + => pht('Edit Only This Event'), + PhabricatorCalendarEventEditEngine::MODE_FUTURE + => pht('Edit All Future Events'), + ))); + + return $this->newDialog() + ->setTitle(pht('Edit Event')) + ->appendParagraph( + pht( + 'This event is part of a series. Which events do you '. + 'want to edit?')) + ->appendForm($form) + ->addSubmitButton(pht('Continue')) + ->addCancelButton($cancel_uri) + ->setDisableWorkflowOnSubmit(true); + + } + + $engine + ->addContextParameter('mode', $mode) + ->setSeriesEditMode($mode); + } } - return id(new PhabricatorCalendarEventEditEngine()) - ->setController($this) - ->buildResponse(); + return $engine->buildResponse(); } } diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php index 0e253ed994..a003611886 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php @@ -147,12 +147,8 @@ final class PhabricatorCalendarEventViewController $edit_uri = "event/edit/{$id}/"; $edit_uri = $this->getApplicationURI($edit_uri); - - if ($event->isChildEvent()) { - $edit_label = pht('Edit This Instance'); - } else { - $edit_label = pht('Edit Event'); - } + $is_recurring = $event->getIsRecurring(); + $edit_label = pht('Edit Event'); $curtain = $this->newCurtainView($event); @@ -163,7 +159,7 @@ final class PhabricatorCalendarEventViewController ->setIcon('fa-pencil') ->setHref($edit_uri) ->setDisabled(!$can_edit) - ->setWorkflow(!$can_edit)); + ->setWorkflow(!$can_edit || $is_recurring)); } $recurring_uri = "{$edit_uri}page/recurring/"; diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php index 4457f60fd8..f877bd9d13 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php @@ -5,6 +5,21 @@ final class PhabricatorCalendarEventEditEngine const ENGINECONST = 'calendar.event'; + private $rawTransactions; + private $seriesEditMode = self::MODE_THIS; + + const MODE_THIS = 'this'; + const MODE_FUTURE = 'future'; + + public function setSeriesEditMode($series_edit_mode) { + $this->seriesEditMode = $series_edit_mode; + return $this; + } + + public function getSeriesEditMode() { + return $this->seriesEditMode; + } + public function getEngineName() { return pht('Calendar Events'); } @@ -77,6 +92,10 @@ final class PhabricatorCalendarEventEditEngine $frequency = null; } + // At least for now, just hide "Invitees" when editing all future events. + // This may eventually deserve a more nuanced approach. + $hide_invitees = ($this->getSeriesEditMode() == self::MODE_FUTURE); + $fields = array( id(new PhabricatorTextEditField()) ->setKey('name') @@ -143,6 +162,7 @@ final class PhabricatorCalendarEventEditEngine ->setConduitTypeDescription(pht('New event host.')) ->setSingleValue($object->getHostPHID()), id(new PhabricatorDatasourceEditField()) + ->setIsHidden($hide_invitees) ->setKey('inviteePHIDs') ->setAliases(array('invite', 'invitee', 'invitees', 'inviteePHID')) ->setLabel(pht('Invitees')) @@ -273,4 +293,77 @@ final class PhabricatorCalendarEventEditEngine ); } + protected function willApplyTransactions($object, array $xactions) { + $viewer = $this->getViewer(); + $this->rawTransactions = $xactions; + + $is_parent = $object->isParentEvent(); + $is_child = $object->isChildEvent(); + $is_future = ($this->getSeriesEditMode() === self::MODE_FUTURE); + + $must_fork = ($is_child && $is_future) || + ($is_parent && !$is_future); + + if ($must_fork) { + $fork_target = $object->loadForkTarget($viewer); + if ($fork_target) { + $fork_xaction = id(new PhabricatorCalendarEventTransaction()) + ->setTransactionType( + PhabricatorCalendarEventForkTransaction::TRANSACTIONTYPE) + ->setNewValue(true); + + if ($fork_target->getPHID() == $object->getPHID()) { + // We're forking the object itself, so just slip it into the + // transactions we're going to apply. + array_unshift($xactions, $fork_xaction); + } else { + // Otherwise, we're forking a different object, so we have to + // apply that separately. + $this->applyTransactions($fork_target, array($fork_xaction)); + } + } + } + + return $xactions; + } + + protected function didApplyTransactions($object, array $xactions) { + $viewer = $this->getViewer(); + + if ($this->getSeriesEditMode() !== self::MODE_FUTURE) { + return; + } + + $targets = $object->loadFutureEvents($viewer); + if (!$targets) { + return; + } + + foreach ($targets as $target) { + $apply = clone $this->rawTransactions; + $this->applyTransactions($target, $apply); + } + } + + private function applyTransactions($target, array $xactions) { + $viewer = $this->getViewer(); + + // TODO: This isn't the most accurate source we could use, but this mode + // is web-only for now. + $content_source = PhabricatorContentSource::newForSource( + PhabricatorWebContentSource::SOURCECONST); + + $editor = id(new PhabricatorCalendarEventEditor()) + ->setActor($viewer) + ->setContentSource($content_source) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true); + + try { + $editor->applyTransactions($target, $xactions); + } catch (PhabricatorApplicationTransactionValidationException $ex) { + // Just ignore any issues we run into. + } + } + } diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index 1a629e07cb..07e041a9ce 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -154,6 +154,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO ->setSequenceIndex($sequence) ->setIsRecurring(true) ->attachParentEvent($this) + ->attachImportSource(null) ->setAllDayDateFrom(0) ->setAllDayDateTo(0) ->setDateFrom(0) @@ -1060,6 +1061,69 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return $this; } + public function loadForkTarget(PhabricatorUser $viewer) { + if (!$this->getIsRecurring()) { + // Can't fork an event which isn't recurring. + return null; + } + + if ($this->isChildEvent()) { + // If this is a child event, this is the fork target. + return $this; + } + + if (!$this->isValidSequenceIndex($viewer, 1)) { + // This appears to be a "recurring" event with no valid instances: for + // example, its "until" date is before the second instance would occur. + // This can happen if we already forked the event or if users entered + // silly stuff. Just edit the event directly without forking anything. + return null; + } + + + $next_event = id(new PhabricatorCalendarEventQuery()) + ->setViewer($viewer) + ->withInstanceSequencePairs( + array( + array($this->getPHID(), 1), + )) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->executeOne(); + + if (!$next_event) { + $next_event = $this->newStub($viewer, 1); + } + + return $next_event; + } + + public function loadFutureEvents(PhabricatorUser $viewer) { + // NOTE: If you can't edit some of the future events, we just + // don't try to update them. This seems like it's probably what + // users are likely to expect. + + // NOTE: This only affects events that are currently in the same + // series, not all events that were ever in the original series. + // We could use series PHIDs instead of parent PHIDs to affect more + // events if this turns out to be counterintuitive. Other + // applications differ in their behavior. + + return id(new PhabricatorCalendarEventQuery()) + ->setViewer($viewer) + ->withParentEventPHIDs(array($this->getPHID())) + ->withUTCInitialEpochBetween($this->getUTCInitialEpoch(), null) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->execute(); + } + /* -( Markup Interface )--------------------------------------------------- */ diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php index ad7692dafe..7baba94109 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php @@ -33,7 +33,14 @@ final class PhabricatorCalendarEventForkTransaction $object->setSequenceIndex(0); // Stop the parent event from recurring after the start date of this event. - $parent->setUntilDateTime($object->newStartDateTime()); + // Since the "until" time is inclusive, rewind it by one second. We could + // figure out the previous instance's time instead or use a COUNT, but this + // seems simpler as long as it doesn't cause any issues. + $until_cutoff = $object->newStartDateTime() + ->newRelativeDateTime('-PT1S') + ->newAbsoluteDateTime(); + + $parent->setUntilDateTime($until_cutoff); $parent->save(); // NOTE: If we implement "COUNT" on editable events, we need to adjust diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index 30f8aec320..f22c113f8e 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -996,9 +996,12 @@ abstract class PhabricatorEditEngine ->setContinueOnNoEffect(true); try { + $xactions = $this->willApplyTransactions($object, $xactions); $editor->applyTransactions($object, $xactions); + $this->didApplyTransactions($object, $xactions); + return $this->newEditResponse($request, $object, $xactions); } catch (PhabricatorApplicationTransactionValidationException $ex) { $validation_exception = $ex; @@ -2176,6 +2179,14 @@ abstract class PhabricatorEditEngine return $page_map[$selected_key]; } + protected function willApplyTransactions($object, array $xactions) { + return $xactions; + } + + protected function didApplyTransactions($object, array $xactions) { + return; + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ From 6e6ae36dcf935188257259cabc776e5c2be241ed Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 31 Oct 2016 18:01:33 -0700 Subject: [PATCH 23/41] Add a skeleton for Calendar notifications Summary: Ref T7931. I'm going to do this separate from existing infrastructure because: - events start at different times for different users; - I like the idea of being able to batch stuff (send one email about several upcoming events); - triggering on ghost/recurring events is a real complicated mess. This puts a skeleton in place that finds all the events we need to notify about and writes some silly example bodies to stdout, marking that we notified users so they don't get notified again. Test Plan: Ran `bin/calendar notify`, got a "great" notification in the command output. {F1891625} Reviewers: chad Reviewed By: chad Maniphest Tasks: T7931 Differential Revision: https://secure.phabricator.com/D16783 --- bin/calendar | 1 + .../20161031.calendar.02.notifylog.sql | 8 + scripts/setup/manage_calendar.php | 21 ++ src/__phutil_library_map__.php | 8 + ...icatorCalendarManagementNotifyWorkflow.php | 25 +++ .../PhabricatorCalendarManagementWorkflow.php | 4 + .../PhabricatorCalendarNotificationEngine.php | 200 ++++++++++++++++++ .../query/PhabricatorCalendarEventQuery.php | 18 ++ .../storage/PhabricatorCalendarEvent.php | 13 ++ .../PhabricatorCalendarNotification.php | 27 +++ 10 files changed, 325 insertions(+) create mode 120000 bin/calendar create mode 100644 resources/sql/autopatches/20161031.calendar.02.notifylog.sql create mode 100755 scripts/setup/manage_calendar.php create mode 100644 src/applications/calendar/management/PhabricatorCalendarManagementNotifyWorkflow.php create mode 100644 src/applications/calendar/management/PhabricatorCalendarManagementWorkflow.php create mode 100644 src/applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php create mode 100644 src/applications/calendar/storage/PhabricatorCalendarNotification.php diff --git a/bin/calendar b/bin/calendar new file mode 120000 index 0000000000..33929a5ba1 --- /dev/null +++ b/bin/calendar @@ -0,0 +1 @@ +../scripts/setup/manage_calendar.php \ No newline at end of file diff --git a/resources/sql/autopatches/20161031.calendar.02.notifylog.sql b/resources/sql/autopatches/20161031.calendar.02.notifylog.sql new file mode 100644 index 0000000000..0c6b1b6a80 --- /dev/null +++ b/resources/sql/autopatches/20161031.calendar.02.notifylog.sql @@ -0,0 +1,8 @@ +CREATE TABLE {$NAMESPACE}_calendar.calendar_notification ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + eventPHID VARBINARY(64) NOT NULL, + utcInitialEpoch INT UNSIGNED NOT NULL, + targetPHID VARBINARY(64) NOT NULL, + didNotifyEpoch INT UNSIGNED NOT NULL, + UNIQUE KEY `key_notify` (eventPHID, utcInitialEpoch, targetPHID) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/scripts/setup/manage_calendar.php b/scripts/setup/manage_calendar.php new file mode 100755 index 0000000000..135c42ded1 --- /dev/null +++ b/scripts/setup/manage_calendar.php @@ -0,0 +1,21 @@ +#!/usr/bin/env php +setTagline(pht('manage Calendar')); +$args->setSynopsis(<<parseStandardArguments(); + +$workflows = id(new PhutilClassMapQuery()) + ->setAncestorClass('PhabricatorCalendarManagementWorkflow') + ->execute(); +$workflows[] = new PhutilHelpArgumentWorkflow(); +$args->parseWorkflows($workflows); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index a7d250eba3..a9cf5ef81d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2153,6 +2153,10 @@ phutil_register_library_map(array( 'PhabricatorCalendarImportTriggerLogType' => 'applications/calendar/importlog/PhabricatorCalendarImportTriggerLogType.php', 'PhabricatorCalendarImportUpdateLogType' => 'applications/calendar/importlog/PhabricatorCalendarImportUpdateLogType.php', 'PhabricatorCalendarImportViewController' => 'applications/calendar/controller/PhabricatorCalendarImportViewController.php', + 'PhabricatorCalendarManagementNotifyWorkflow' => 'applications/calendar/management/PhabricatorCalendarManagementNotifyWorkflow.php', + 'PhabricatorCalendarManagementWorkflow' => 'applications/calendar/management/PhabricatorCalendarManagementWorkflow.php', + 'PhabricatorCalendarNotification' => 'applications/calendar/storage/PhabricatorCalendarNotification.php', + 'PhabricatorCalendarNotificationEngine' => 'applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php', 'PhabricatorCalendarRemarkupRule' => 'applications/calendar/remarkup/PhabricatorCalendarRemarkupRule.php', 'PhabricatorCalendarReplyHandler' => 'applications/calendar/mail/PhabricatorCalendarReplyHandler.php', 'PhabricatorCalendarSchemaSpec' => 'applications/calendar/storage/PhabricatorCalendarSchemaSpec.php', @@ -7014,6 +7018,10 @@ phutil_register_library_map(array( 'PhabricatorCalendarImportTriggerLogType' => 'PhabricatorCalendarImportLogType', 'PhabricatorCalendarImportUpdateLogType' => 'PhabricatorCalendarImportLogType', 'PhabricatorCalendarImportViewController' => 'PhabricatorCalendarController', + 'PhabricatorCalendarManagementNotifyWorkflow' => 'PhabricatorCalendarManagementWorkflow', + 'PhabricatorCalendarManagementWorkflow' => 'PhabricatorManagementWorkflow', + 'PhabricatorCalendarNotification' => 'PhabricatorCalendarDAO', + 'PhabricatorCalendarNotificationEngine' => 'Phobject', 'PhabricatorCalendarRemarkupRule' => 'PhabricatorObjectRemarkupRule', 'PhabricatorCalendarReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler', 'PhabricatorCalendarSchemaSpec' => 'PhabricatorConfigSchemaSpec', diff --git a/src/applications/calendar/management/PhabricatorCalendarManagementNotifyWorkflow.php b/src/applications/calendar/management/PhabricatorCalendarManagementNotifyWorkflow.php new file mode 100644 index 0000000000..1c0c9894a9 --- /dev/null +++ b/src/applications/calendar/management/PhabricatorCalendarManagementNotifyWorkflow.php @@ -0,0 +1,25 @@ +setName('notify') + ->setExamples('**notify** [options]') + ->setSynopsis( + pht( + 'Test and debug notifications about upcoming events.')) + ->setArguments(array()); + } + + public function execute(PhutilArgumentParser $args) { + $viewer = $this->getViewer(); + + $engine = new PhabricatorCalendarNotificationEngine(); + $engine->publishNotifications(); + + return 0; + } + +} diff --git a/src/applications/calendar/management/PhabricatorCalendarManagementWorkflow.php b/src/applications/calendar/management/PhabricatorCalendarManagementWorkflow.php new file mode 100644 index 0000000000..613a0bab64 --- /dev/null +++ b/src/applications/calendar/management/PhabricatorCalendarManagementWorkflow.php @@ -0,0 +1,4 @@ +cursor) { + $now = PhabricatorTime::getNow(); + $this->cursor = $now - phutil_units('5 minutes in seconds'); + } + + return $this->cursor; + } + + public function publishNotifications() { + $cursor = $this->getCursor(); + + $window_min = $cursor - phutil_units('16 hours in seconds'); + $window_max = $cursor + phutil_units('16 hours in seconds'); + + $viewer = PhabricatorUser::getOmnipotentUser(); + + $events = id(new PhabricatorCalendarEventQuery()) + ->setViewer($viewer) + ->withDateRange($window_min, $window_max) + ->withIsCancelled(false) + ->withIsImported(false) + ->setGenerateGhosts(true) + ->execute(); + if (!$events) { + // No events are starting soon in any timezone, so there is nothing + // left to be done. + return; + } + + $attendee_map = array(); + foreach ($events as $key => $event) { + $notifiable_phids = array(); + foreach ($event->getInvitees() as $invitee) { + if (!$invitee->isAttending()) { + continue; + } + $notifiable_phids[] = $invitee->getInviteePHID(); + } + if (!$notifiable_phids) { + unset($events[$key]); + } + $attendee_map[$key] = array_fuse($notifiable_phids); + } + if (!$attendee_map) { + // None of the events have any notifiable attendees, so there is no + // one to notify of anything. + return; + } + + $all_attendees = array(); + foreach ($attendee_map as $key => $attendee_phids) { + foreach ($attendee_phids as $attendee_phid) { + $all_attendees[$attendee_phid] = $attendee_phid; + } + } + + $user_map = id(new PhabricatorPeopleQuery()) + ->setViewer($viewer) + ->withPHIDs($all_attendees) + ->withIsDisabled(false) + ->needUserSettings(true) + ->execute(); + $user_map = mpull($user_map, null, 'getPHID'); + if (!$user_map) { + // None of the attendees are valid users: they're all imported users + // or projects or invalid or some other kind of unnotifiable entity. + return; + } + + $all_event_phids = array(); + foreach ($events as $key => $event) { + foreach ($event->getNotificationPHIDs() as $phid) { + $all_event_phids[$phid] = $phid; + } + } + + $table = new PhabricatorCalendarNotification(); + $conn = $table->establishConnection('w'); + + $rows = queryfx_all( + $conn, + 'SELECT * FROM %T WHERE eventPHID IN (%Ls) AND targetPHID IN (%Ls)', + $table->getTableName(), + $all_event_phids, + $all_attendees); + $sent_map = array(); + foreach ($rows as $row) { + $event_phid = $row['eventPHID']; + $target_phid = $row['targetPHID']; + $initial_epoch = $row['utcInitialEpoch']; + $sent_map[$event_phid][$target_phid][$initial_epoch] = $row; + } + + $notify_min = $cursor; + $notify_max = $cursor + phutil_units('15 minutes in seconds'); + $notify_map = array(); + foreach ($events as $key => $event) { + $initial_epoch = $event->getUTCInitialEpoch(); + $event_phids = $event->getNotificationPHIDs(); + + // Select attendees who actually exist, and who we have not sent any + // notifications to yet. + $attendee_phids = $attendee_map[$key]; + $users = array_select_keys($user_map, $attendee_phids); + foreach ($users as $user_phid => $user) { + foreach ($event_phids as $event_phid) { + if (isset($sent_map[$event_phid][$user_phid][$initial_epoch])) { + unset($users[$user_phid]); + continue 2; + } + } + } + + if (!$users) { + continue; + } + + // Discard attendees for whom the event start time isn't soon. Events + // may start at different times for different users, so we need to + // check every user's start time. + foreach ($users as $user_phid => $user) { + $user_datetime = $event->newStartDateTime() + ->setViewerTimezone($user->getTimezoneIdentifier()); + + $user_epoch = $user_datetime->getEpoch(); + if ($user_epoch < $notify_min || $user_epoch > $notify_max) { + unset($users[$user_phid]); + continue; + } + + $notify_map[$user_phid][] = array( + 'event' => $event, + 'datetime' => $user_datetime, + 'epoch' => $user_epoch, + ); + } + } + + $mail_list = array(); + $mark_list = array(); + $now = PhabricatorTime::getNow(); + foreach ($notify_map as $user_phid => $events) { + $user = $user_map[$user_phid]; + $events = isort($events, 'epoch'); + + // TODO: This is just a proof-of-concept that gets dumped to the console; + // it will be replaced with a nice fancy email and notification. + + $body = array(); + $body[] = pht('%s, these events start soon:', $user->getUsername()); + $body[] = null; + foreach ($events as $spec) { + $event = $spec['event']; + $body[] = $event->getName(); + } + $body = implode("\n", $body); + + $mail_list[] = $body; + + foreach ($events as $spec) { + $event = $spec['event']; + foreach ($event->getNotificationPHIDs() as $phid) { + $mark_list[] = qsprintf( + $conn, + '(%s, %s, %d, %d)', + $phid, + $user_phid, + $event->getUTCInitialEpoch(), + $now); + } + } + } + + // Mark all the notifications we're about to send as delivered so we + // do not double-notify. + foreach (PhabricatorLiskDAO::chunkSQL($mark_list) as $chunk) { + queryfx( + $conn, + 'INSERT IGNORE INTO %T + (eventPHID, targetPHID, utcInitialEpoch, didNotifyEpoch) + VALUES %Q', + $table->getTableName(), + $chunk); + } + + foreach ($mail_list as $mail) { + echo $mail; + echo "\n\n"; + } + } + +} diff --git a/src/applications/calendar/query/PhabricatorCalendarEventQuery.php b/src/applications/calendar/query/PhabricatorCalendarEventQuery.php index a6ac2e4ffc..3980c0635d 100644 --- a/src/applications/calendar/query/PhabricatorCalendarEventQuery.php +++ b/src/applications/calendar/query/PhabricatorCalendarEventQuery.php @@ -19,6 +19,7 @@ final class PhabricatorCalendarEventQuery private $importUIDs; private $utcInitialEpochMin; private $utcInitialEpochMax; + private $isImported; private $generateGhosts = false; @@ -103,6 +104,11 @@ final class PhabricatorCalendarEventQuery return $this; } + public function withIsImported($is_imported) { + $this->isImported = $is_imported; + return $this; + } + protected function getDefaultOrderVector() { return array('start', 'id'); } @@ -472,6 +478,18 @@ final class PhabricatorCalendarEventQuery $this->importUIDs); } + if ($this->isImported !== null) { + if ($this->isImported) { + $where[] = qsprintf( + $conn, + 'event.importSourcePHID IS NOT NULL'); + } else { + $where[] = qsprintf( + $conn, + 'event.importSourcePHID IS NULL'); + } + } + return $where; } diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index 07e041a9ce..c40afe443e 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -1124,6 +1124,19 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO ->execute(); } + public function getNotificationPHIDs() { + $phids = array(); + if ($this->getPHID()) { + $phids[] = $this->getPHID(); + } + + if ($this->getSeriesParentPHID()) { + $phids[] = $this->getSeriesParentPHID(); + } + + return $phids; + } + /* -( Markup Interface )--------------------------------------------------- */ diff --git a/src/applications/calendar/storage/PhabricatorCalendarNotification.php b/src/applications/calendar/storage/PhabricatorCalendarNotification.php new file mode 100644 index 0000000000..46daaa2b3a --- /dev/null +++ b/src/applications/calendar/storage/PhabricatorCalendarNotification.php @@ -0,0 +1,27 @@ + false, + self::CONFIG_COLUMN_SCHEMA => array( + 'utcInitialEpoch' => 'epoch', + 'didNotifyEpoch' => 'epoch', + ), + self::CONFIG_KEY_SCHEMA => array( + 'key_notify' => array( + 'columns' => array('eventPHID', 'utcInitialEpoch', 'targetPHID'), + 'unique' => true, + ), + ), + ) + parent::getConfiguration(); + } + +} From 6b16f930c47c626416894cb492c56b195331ee21 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 1 Nov 2016 09:45:48 -0700 Subject: [PATCH 24/41] Automatically send (not-so-great) email notifications for upcoming events Summary: Ref T7931. This is still quite rough, but should technically send vaguely-useful email as part of the standard trigger infrastructure. Test Plan: Ran `bin/phd start`, created an event shortly, saw reminder email send in `bin/mail list-outbound`. Reviewers: chad Reviewed By: chad Maniphest Tasks: T7931 Differential Revision: https://secure.phabricator.com/D16784 --- src/__phutil_library_map__.php | 2 + ...icatorCalendarManagementNotifyWorkflow.php | 17 +- ...abricatorCalendarEventNotificationView.php | 61 +++++++ .../PhabricatorCalendarNotificationEngine.php | 149 +++++++++++++++--- .../workers/PhabricatorTriggerDaemon.php | 20 +++ 5 files changed, 225 insertions(+), 24 deletions(-) create mode 100644 src/applications/calendar/notifications/PhabricatorCalendarEventNotificationView.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index a9cf5ef81d..121d3e2de3 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2067,6 +2067,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarEventMailReceiver' => 'applications/calendar/mail/PhabricatorCalendarEventMailReceiver.php', 'PhabricatorCalendarEventNameHeraldField' => 'applications/calendar/herald/PhabricatorCalendarEventNameHeraldField.php', 'PhabricatorCalendarEventNameTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventNameTransaction.php', + 'PhabricatorCalendarEventNotificationView' => 'applications/calendar/notifications/PhabricatorCalendarEventNotificationView.php', 'PhabricatorCalendarEventPHIDType' => 'applications/calendar/phid/PhabricatorCalendarEventPHIDType.php', 'PhabricatorCalendarEventQuery' => 'applications/calendar/query/PhabricatorCalendarEventQuery.php', 'PhabricatorCalendarEventRSVPEmailCommand' => 'applications/calendar/command/PhabricatorCalendarEventRSVPEmailCommand.php', @@ -6915,6 +6916,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarEventMailReceiver' => 'PhabricatorObjectMailReceiver', 'PhabricatorCalendarEventNameHeraldField' => 'PhabricatorCalendarEventHeraldField', 'PhabricatorCalendarEventNameTransaction' => 'PhabricatorCalendarEventTransactionType', + 'PhabricatorCalendarEventNotificationView' => 'Phobject', 'PhabricatorCalendarEventPHIDType' => 'PhabricatorPHIDType', 'PhabricatorCalendarEventQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorCalendarEventRSVPEmailCommand' => 'PhabricatorCalendarEventEmailCommand', diff --git a/src/applications/calendar/management/PhabricatorCalendarManagementNotifyWorkflow.php b/src/applications/calendar/management/PhabricatorCalendarManagementNotifyWorkflow.php index 1c0c9894a9..b72474b9d5 100644 --- a/src/applications/calendar/management/PhabricatorCalendarManagementNotifyWorkflow.php +++ b/src/applications/calendar/management/PhabricatorCalendarManagementNotifyWorkflow.php @@ -10,13 +10,28 @@ final class PhabricatorCalendarManagementNotifyWorkflow ->setSynopsis( pht( 'Test and debug notifications about upcoming events.')) - ->setArguments(array()); + ->setArguments( + array( + array( + 'name' => 'minutes', + 'param' => 'N', + 'help' => pht( + 'Notify about events in the next __N__ minutes (default: 15). '. + 'Setting this to a larger value makes testing easier.'), + ), + )); } public function execute(PhutilArgumentParser $args) { $viewer = $this->getViewer(); $engine = new PhabricatorCalendarNotificationEngine(); + + $minutes = $args->getArg('minutes'); + if ($minutes) { + $engine->setNotifyWindow(phutil_units("{$minutes} minutes in seconds")); + } + $engine->publishNotifications(); return 0; diff --git a/src/applications/calendar/notifications/PhabricatorCalendarEventNotificationView.php b/src/applications/calendar/notifications/PhabricatorCalendarEventNotificationView.php new file mode 100644 index 0000000000..7568f7d8dc --- /dev/null +++ b/src/applications/calendar/notifications/PhabricatorCalendarEventNotificationView.php @@ -0,0 +1,61 @@ +viewer = $viewer; + return $this; + } + + public function getViewer() { + return $this->viewer; + } + + public function setEvent(PhabricatorCalendarEvent $event) { + $this->event = $event; + return $this; + } + + public function getEvent() { + return $this->event; + } + + public function setEpoch($epoch) { + $this->epoch = $epoch; + return $this; + } + + public function getEpoch() { + return $this->epoch; + } + + public function setDateTime(PhutilCalendarDateTime $date_time) { + $this->dateTime = $date_time; + return $this; + } + + public function getDateTime() { + return $this->dateTime; + } + + public function getDisplayMinutes() { + $epoch = $this->getEpoch(); + $now = PhabricatorTime::getNow(); + $minutes = (int)ceil(($epoch - $now) / 60); + return new PhutilNumber($minutes); + } + + public function getDisplayTime() { + $viewer = $this->getViewer(); + + $epoch = $this->getEpoch(); + return phabricator_datetime($epoch, $viewer); + } + +} diff --git a/src/applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php b/src/applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php index 2051ef55d2..68d4f37a2c 100644 --- a/src/applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php +++ b/src/applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php @@ -4,19 +4,75 @@ final class PhabricatorCalendarNotificationEngine extends Phobject { private $cursor; + private $notifyWindow; public function getCursor() { if (!$this->cursor) { $now = PhabricatorTime::getNow(); - $this->cursor = $now - phutil_units('5 minutes in seconds'); + $this->cursor = $now - phutil_units('10 minutes in seconds'); } return $this->cursor; } + public function setCursor($cursor) { + $this->cursor = $cursor; + return $this; + } + + public function setNotifyWindow($notify_window) { + $this->notifyWindow = $notify_window; + return $this; + } + + public function getNotifyWindow() { + if (!$this->notifyWindow) { + return phutil_units('15 minutes in seconds'); + } + + return $this->notifyWindow; + } + public function publishNotifications() { $cursor = $this->getCursor(); + $now = PhabricatorTime::getNow(); + if ($cursor > $now) { + return; + } + + $calendar_class = 'PhabricatorCalendarApplication'; + if (!PhabricatorApplication::isClassInstalled($calendar_class)) { + return; + } + + try { + $lock = PhabricatorGlobalLock::newLock('calendar.notify') + ->lock(5); + } catch (PhutilLockException $ex) { + return; + } + + $caught = null; + try { + $this->sendNotifications(); + } catch (Exception $ex) { + $caught = $ex; + } + + $lock->unlock(); + + // Wait a little while before checking for new notifications to send. + $this->setCursor($cursor + phutil_units('1 minute in seconds')); + + if ($caught) { + throw $caught; + } + } + + private function sendNotifications() { + $cursor = $this->getCursor(); + $window_min = $cursor - phutil_units('16 hours in seconds'); $window_max = $cursor + phutil_units('16 hours in seconds'); @@ -100,7 +156,7 @@ final class PhabricatorCalendarNotificationEngine } $notify_min = $cursor; - $notify_max = $cursor + phutil_units('15 minutes in seconds'); + $notify_max = $cursor + $this->getNotifyWindow(); $notify_map = array(); foreach ($events as $key => $event) { $initial_epoch = $event->getUTCInitialEpoch(); @@ -136,11 +192,13 @@ final class PhabricatorCalendarNotificationEngine continue; } - $notify_map[$user_phid][] = array( - 'event' => $event, - 'datetime' => $user_datetime, - 'epoch' => $user_epoch, - ); + $view = id(new PhabricatorCalendarEventNotificationView()) + ->setViewer($user) + ->setEvent($event) + ->setDateTime($user_datetime) + ->setEpoch($user_epoch); + + $notify_map[$user_phid][] = $view; } } @@ -149,24 +207,23 @@ final class PhabricatorCalendarNotificationEngine $now = PhabricatorTime::getNow(); foreach ($notify_map as $user_phid => $events) { $user = $user_map[$user_phid]; - $events = isort($events, 'epoch'); - // TODO: This is just a proof-of-concept that gets dumped to the console; - // it will be replaced with a nice fancy email and notification. - - $body = array(); - $body[] = pht('%s, these events start soon:', $user->getUsername()); - $body[] = null; - foreach ($events as $spec) { - $event = $spec['event']; - $body[] = $event->getName(); + $locale = PhabricatorEnv::beginScopedLocale($user->getTranslation()); + $caught = null; + try { + $mail_list[] = $this->newMailMessage($user, $events); + } catch (Exception $ex) { + $caught = $ex; } - $body = implode("\n", $body); - $mail_list[] = $body; + unset($locale); - foreach ($events as $spec) { - $event = $spec['event']; + if ($caught) { + throw $ex; + } + + foreach ($events as $view) { + $event = $view->getEvent(); foreach ($event->getNotificationPHIDs() as $phid) { $mark_list[] = qsprintf( $conn, @@ -192,9 +249,55 @@ final class PhabricatorCalendarNotificationEngine } foreach ($mail_list as $mail) { - echo $mail; - echo "\n\n"; + $mail->saveAndSend(); } } + + private function newMailMessage(PhabricatorUser $viewer, array $events) { + $events = msort($events, 'getEpoch'); + + $next_event = head($events); + + $body = new PhabricatorMetaMTAMailBody(); + foreach ($events as $event) { + $body->addTextSection( + null, + pht( + '%s is starting in %s minute(s), at %s.', + $event->getEvent()->getName(), + $event->getDisplayMinutes(), + $event->getDisplayTime())); + + $body->addLinkSection( + pht('EVENT DETAIL'), + PhabricatorEnv::getProductionURI($event->getEvent()->getURI())); + } + + $next_event = head($events)->getEvent(); + $subject = $next_event->getName(); + if (count($events) > 1) { + $more = pht( + '(+%s more...)', + new PhutilNumber(count($events) - 1)); + $subject = "{$subject} {$more}"; + } + + $calendar_phid = id(new PhabricatorCalendarApplication()) + ->getPHID(); + + return id(new PhabricatorMetaMTAMail()) + ->setSubject($subject) + ->addTos(array($viewer->getPHID())) + ->setSensitiveContent(false) + ->setFrom($calendar_phid) + ->setIsBulk(true) + ->setSubjectPrefix(pht('[Calendar]')) + ->setVarySubjectPrefix(pht('[Reminder]')) + ->setThreadID($next_event->getPHID(), false) + ->setRelatedPHID($next_event->getPHID()) + ->setBody($body->render()) + ->setHTMLBody($body->renderHTML()); + } + } diff --git a/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php b/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php index a51280d6ab..cec2fbf80e 100644 --- a/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php +++ b/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php @@ -20,6 +20,8 @@ final class PhabricatorTriggerDaemon private $nuanceSources; private $nuanceCursors; + private $calendarEngine; + protected function run() { // The trigger daemon is a low-level infrastructure daemon which schedules @@ -105,6 +107,7 @@ final class PhabricatorTriggerDaemon $sleep_duration = $this->getSleepDuration(); $sleep_duration = $this->runNuanceImportCursors($sleep_duration); $sleep_duration = $this->runGarbageCollection($sleep_duration); + $sleep_duration = $this->runCalendarNotifier($sleep_duration); $this->sleep($sleep_duration); } while (!$this->shouldExit()); } @@ -456,4 +459,21 @@ final class PhabricatorTriggerDaemon return true; } + +/* -( Calendar Notifier )-------------------------------------------------- */ + + + private function runCalendarNotifier($duration) { + $run_until = (PhabricatorTime::getNow() + $duration); + + if (!$this->calendarEngine) { + $this->calendarEngine = new PhabricatorCalendarNotificationEngine(); + } + + $this->calendarEngine->publishNotifications(); + + $remaining = max(0, $run_until - PhabricatorTime::getNow()); + return $remaining; + } + } From 3e15e0b9806b1207b0cd9f5f7b7217eb9b71a706 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 1 Nov 2016 12:55:27 -0700 Subject: [PATCH 25/41] Store more datetime information on Calendar transactions and improve rendering behaviors Summary: Fixes T11805. Depends on D16785. This generally tries to smooth out transactions: - All-day stuff now says "Nov 3" instead of "Nov 3 12:00:00 AM". - Fewer weird bugs / extra transactions. - No more silly extra "yeah, you definitely set that event time" transaction on create. Test Plan: Edited events; changed from all-day to not-all-day and back again, viewed transaction log. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11805 Differential Revision: https://secure.phabricator.com/D16786 --- .../editor/PhabricatorCalendarEventEditor.php | 28 +++++++++++- ...habricatorCalendarEventDateTransaction.php | 6 ++- ...ricatorCalendarEventEndDateTransaction.php | 24 ++++++++--- ...catorCalendarEventStartDateTransaction.php | 24 ++++++++--- ...catorCalendarEventUntilDateTransaction.php | 16 ++++--- .../PhabricatorModularTransactionType.php | 43 ++++++++++++++----- .../control/AphrontFormDateControlValue.php | 26 +++++++++++ 7 files changed, 136 insertions(+), 31 deletions(-) diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php index bf63d8e8e7..12f7b8ba8d 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php @@ -3,6 +3,9 @@ final class PhabricatorCalendarEventEditor extends PhabricatorApplicationTransactionEditor { + private $oldIsAllDay; + private $newIsAllDay; + public function getEditorApplicationClass() { return 'PhabricatorCalendarApplication'; } @@ -19,13 +22,20 @@ final class PhabricatorCalendarEventEditor return pht('%s created %s.', $author, $object); } - protected function shouldApplyInitialEffects( PhabricatorLiskDAO $object, array $xactions) { return true; } + public function getOldIsAllDay() { + return $this->oldIsAllDay; + } + + public function getNewIsAllDay() { + return $this->newIsAllDay; + } + protected function applyInitialEffects( PhabricatorLiskDAO $object, array $xactions) { @@ -34,6 +44,22 @@ final class PhabricatorCalendarEventEditor if ($object->getIsStub()) { $this->materializeStub($object); } + + // Before doing anything, figure out if the event will be an all day event + // or not after the edit. This affects how we store datetime values, and + // whether we render times or not. + $old_allday = $object->getIsAllDay(); + $new_allday = $old_allday; + $type_allday = PhabricatorCalendarEventAllDayTransaction::TRANSACTIONTYPE; + foreach ($xactions as $xaction) { + if ($xaction->getTransactionType() != $type_allday) { + continue; + } + $target_alllday = (bool)$xaction->getNewValue(); + } + + $this->oldIsAllDay = $old_allday; + $this->newIsAllDay = $new_allday; } private function materializeStub(PhabricatorCalendarEvent $event) { diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventDateTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventDateTransaction.php index 5a4958ba4a..cc2cb59da9 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventDateTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventDateTransaction.php @@ -6,7 +6,11 @@ abstract class PhabricatorCalendarEventDateTransaction abstract protected function getInvalidDateMessage(); public function generateNewValue($object, $value) { - return $value->getEpoch(); + $editor = $this->getEditor(); + return $value->newPhutilDateTime() + ->setIsAllDay($editor->getNewIsAllDay()) + ->newAbsoluteDateTime() + ->toDictionary(); } public function validateTransactions($object, array $xactions) { diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php index 8d66176ea8..4c8bb0fb74 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php @@ -6,20 +6,32 @@ final class PhabricatorCalendarEventEndDateTransaction const TRANSACTIONTYPE = 'calendar.enddate'; public function generateOldValue($object) { - // TODO: Upgrade this. - return $object->newEndDateTimeForEdit()->getEpoch(); + $editor = $this->getEditor(); + + return $object->newEndDateTimeForEdit() + ->newAbsoluteDateTime() + ->setIsAllDay($editor->getOldIsAllDay()) + ->toDictionary(); } public function applyInternalEffects($object, $value) { $actor = $this->getActor(); + $editor = $this->getEditor(); + + $datetime = PhutilCalendarAbsoluteDateTime::newFromDictionary($value); + $datetime->setIsAllDay($editor->getNewIsAllDay()); - $datetime = PhutilCalendarAbsoluteDateTime::newFromEpoch( - $value, - $actor->getTimezoneIdentifier()); - $datetime->setIsAllDay($object->getIsAllDay()); $object->setEndDateTime($datetime); } + public function shouldHide() { + if ($this->isCreateTransaction()) { + return true; + } + + return false; + } + public function getTitle() { return pht( '%s changed the end date for this event from %s to %s.', diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventStartDateTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventStartDateTransaction.php index e08bbac780..484591fa5f 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventStartDateTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventStartDateTransaction.php @@ -6,20 +6,32 @@ final class PhabricatorCalendarEventStartDateTransaction const TRANSACTIONTYPE = 'calendar.startdate'; public function generateOldValue($object) { - // TODO: Upgrade this. - return $object->getStartDateTimeEpoch(); + $editor = $this->getEditor(); + + return $object->newStartDateTime() + ->newAbsoluteDateTime() + ->setIsAllDay($editor->getOldIsAllDay()) + ->toDictionary(); } public function applyInternalEffects($object, $value) { $actor = $this->getActor(); + $editor = $this->getEditor(); + + $datetime = PhutilCalendarAbsoluteDateTime::newFromDictionary($value); + $datetime->setIsAllDay($editor->getNewIsAllDay()); - $datetime = PhutilCalendarAbsoluteDateTime::newFromEpoch( - $value, - $actor->getTimezoneIdentifier()); - $datetime->setIsAllDay($object->getIsAllDay()); $object->setStartDateTime($datetime); } + public function shouldHide() { + if ($this->isCreateTransaction()) { + return true; + } + + return false; + } + public function getTitle() { return pht( '%s changed the start date for this event from %s to %s.', diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php index 736ed13704..a841d4d09c 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php @@ -6,20 +6,24 @@ final class PhabricatorCalendarEventUntilDateTransaction const TRANSACTIONTYPE = 'calendar.recurrenceenddate'; public function generateOldValue($object) { - // TODO: Upgrade this. - return $object->getUntilDateTimeEpoch(); + $editor = $this->getEditor(); + + return $object->newUntilDateTime() + ->newAbsoluteDateTime() + ->setIsAllDay($editor->getOldIsAllDay()) + ->toDictionary(); } public function applyInternalEffects($object, $value) { $actor = $this->getActor(); + $editor = $this->getEditor(); // TODO: DEPRECATED. $object->setRecurrenceEndDate($value); - $datetime = PhutilCalendarAbsoluteDateTime::newFromEpoch( - $value, - $actor->getTimezoneIdentifier()); - $datetime->setIsAllDay($object->getIsAllDay()); + $datetime = PhutilCalendarAbsoluteDateTime::newFromDictionary($value); + $datetime->setIsAllDay($editor->getNewIsAllDay()); + $object->setUntilDateTime($datetime); } diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php index a5ddcc3e94..01a02f7fd3 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php +++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php @@ -196,19 +196,36 @@ abstract class PhabricatorModularTransactionType final protected function renderDate($epoch) { $viewer = $this->getViewer(); - $display = phabricator_datetime($epoch, $viewer); + // We accept either epoch timestamps or dictionaries describing a + // PhutilCalendarDateTime. + if (is_array($epoch)) { + $datetime = PhutilCalendarAbsoluteDateTime::newFromDictionary($epoch) + ->setViewerTimezone($viewer->getTimezoneIdentifier()); - // When rendering to text, we explicitly render the offset from UTC to - // provide context to the date: the mail may be generating with the - // server's settings, or the user may later refer back to it after changing - // timezones. + $all_day = $datetime->getIsAllDay(); - if ($this->isTextMode()) { - $offset = $viewer->getTimeZoneOffsetInHours(); - if ($offset >= 0) { - $display = pht('%s (UTC+%d)', $display, $offset); - } else { - $display = pht('%s (UTC-%d)', $display, abs($offset)); + $epoch = $datetime->getEpoch(); + } else { + $all_day = false; + } + + if ($all_day) { + $display = phabricator_date($epoch, $viewer); + } else { + $display = phabricator_datetime($epoch, $viewer); + + // When rendering to text, we explicitly render the offset from UTC to + // provide context to the date: the mail may be generating with the + // server's settings, or the user may later refer back to it after + // changing timezones. + + if ($this->isTextMode()) { + $offset = $viewer->getTimeZoneOffsetInHours(); + if ($offset >= 0) { + $display = pht('%s (UTC+%d)', $display, $offset); + } else { + $display = pht('%s (UTC-%d)', $display, abs($offset)); + } } } @@ -262,4 +279,8 @@ abstract class PhabricatorModularTransactionType ->setTransaction($this->getStorage()); } + final protected function isCreateTransaction() { + return $this->getStorage()->getIsCreateTransaction(); + } + } diff --git a/src/view/form/control/AphrontFormDateControlValue.php b/src/view/form/control/AphrontFormDateControlValue.php index 51942c20e7..873ad44fe4 100644 --- a/src/view/form/control/AphrontFormDateControlValue.php +++ b/src/view/form/control/AphrontFormDateControlValue.php @@ -231,6 +231,32 @@ final class AphrontFormDateControlValue extends Phobject { return $datetime; } + public function newPhutilDateTime() { + $datetime = $this->getDateTime(); + if (!$datetime) { + return null; + } + + $all_day = !strlen($this->valueTime); + $zone_identifier = $this->viewer->getTimezoneIdentifier(); + + $result = id(new PhutilCalendarAbsoluteDateTime()) + ->setYear((int)$datetime->format('Y')) + ->setMonth((int)$datetime->format('m')) + ->setDay((int)$datetime->format('d')) + ->setHour((int)$datetime->format('G')) + ->setMinute((int)$datetime->format('i')) + ->setSecond((int)$datetime->format('s')) + ->setTimezone($zone_identifier); + + if ($all_day) { + $result->setIsAllDay(true); + } + + return $result; + } + + private function getFormattedDateFromParts( $year, $month, From 191b9398a5f6687ddd1086adb2783a1d17b3d6c4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 1 Nov 2016 13:57:18 -0700 Subject: [PATCH 26/41] Fix some minor Calendar issues, including a paging issue on imports Summary: Fixes T11808. I couldn't reproduce the issue there locally so I'm just cheating a little bit until a better reproduction case shows up. We don't need to do a full load here anyway, and testing for any row is more efficient. Test Plan: Poked around imports without issues, but couldn't reproduce this problem locally. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11808 Differential Revision: https://secure.phabricator.com/D16787 --- .../PhabricatorCalendarImportEngine.php | 20 +++++++++++++------ .../PhabricatorCalendarNotificationEngine.php | 5 +++-- .../query/PhabricatorCalendarEventQuery.php | 2 +- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php index 96069e0ee1..bf2200b981 100644 --- a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php +++ b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php @@ -225,6 +225,7 @@ abstract class PhabricatorCalendarImportEngine $xactions[$full_uid] = $this->newUpdateTransactions($event, $node); $update_map[$full_uid] = $event; + $attendee_map[$full_uid] = array(); $attendees = $node->getAttendees(); $private_index = 1; foreach ($attendees as $attendee) { @@ -526,7 +527,7 @@ abstract class PhabricatorCalendarImportEngine ->setStartDateTime($start_datetime) ->setEndDateTime($end_datetime); - $event->setIsAllDay($start_datetime->getIsAllDay()); + $event->setIsAllDay((int)$start_datetime->getIsAllDay()); // TODO: This should be transactional, but the transaction only accepts // simple frequency rules right now. @@ -551,11 +552,18 @@ abstract class PhabricatorCalendarImportEngine PhabricatorUser $viewer, PhabricatorCalendarImport $import) { - $any_event = id(new PhabricatorCalendarEventQuery()) - ->setViewer($viewer) - ->withImportSourcePHIDs(array($import->getPHID())) - ->setLimit(1) - ->execute(); + $table = new PhabricatorCalendarEvent(); + $conn = $table->establishConnection('r'); + + // Using a CalendarEventQuery here was failing oddly in a way that was + // difficult to reproduce locally (see T11808). Just check the table + // directly; this is significantly more efficient anyway. + + $any_event = queryfx_all( + $conn, + 'SELECT phid FROM %T WHERE importSourcePHID = %s LIMIT 1', + $table->getTableName(), + $import->getPHID()); return (bool)$any_event; } diff --git a/src/applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php b/src/applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php index 68d4f37a2c..b0a762b577 100644 --- a/src/applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php +++ b/src/applications/calendar/notifications/PhabricatorCalendarNotificationEngine.php @@ -155,8 +155,9 @@ final class PhabricatorCalendarNotificationEngine $sent_map[$event_phid][$target_phid][$initial_epoch] = $row; } - $notify_min = $cursor; - $notify_max = $cursor + $this->getNotifyWindow(); + $now = PhabricatorTime::getNow(); + $notify_min = $now; + $notify_max = $now + $this->getNotifyWindow(); $notify_map = array(); foreach ($events as $key => $event) { $initial_epoch = $event->getUTCInitialEpoch(); diff --git a/src/applications/calendar/query/PhabricatorCalendarEventQuery.php b/src/applications/calendar/query/PhabricatorCalendarEventQuery.php index 3980c0635d..0c762ffa54 100644 --- a/src/applications/calendar/query/PhabricatorCalendarEventQuery.php +++ b/src/applications/calendar/query/PhabricatorCalendarEventQuery.php @@ -126,7 +126,7 @@ final class PhabricatorCalendarEventQuery return array( 'start' => array( 'table' => $this->getPrimaryTableAlias(), - 'column' => 'dateFrom', + 'column' => 'utcInitialEpoch', 'reverse' => true, 'type' => 'int', 'unique' => false, From 6982bded7124dd6f7cf4662aa5d3f97d96736ccb Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 1 Nov 2016 15:47:52 -0700 Subject: [PATCH 27/41] Remove ancient "Holiday" storage Summary: Ref T11809. This came out of Facebook many years ago for computing the number of business days that revisions had been stale. We removed the little staleness marker a few months ago and haven't seen complaints about it. If we did holidays now it would make sense to integrate them more directly with Calendar as real events, but I have no plans to pursue this anytime soon. It's easy enough to add the federal holidays manually (~5 minutes of work per year?) if you want them, and they're commentable/editable and you can add local holidays if you're not in the US. Test Plan: - Ran `bin/storage upgrade -f`. - Grepped for `CalendarHoliday`. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11809 Differential Revision: https://secure.phabricator.com/D16788 --- .../20161101.calendar.01.noholiday.sql | 1 + scripts/calendar/import_us_holidays.php | 62 ------------------- src/__phutil_library_map__.php | 4 -- .../storage/PhabricatorCalendarHoliday.php | 24 ------- .../PhabricatorCalendarHolidayTestCase.php | 19 ------ 5 files changed, 1 insertion(+), 109 deletions(-) create mode 100644 resources/sql/autopatches/20161101.calendar.01.noholiday.sql delete mode 100755 scripts/calendar/import_us_holidays.php delete mode 100644 src/applications/calendar/storage/PhabricatorCalendarHoliday.php delete mode 100644 src/applications/calendar/storage/__tests__/PhabricatorCalendarHolidayTestCase.php diff --git a/resources/sql/autopatches/20161101.calendar.01.noholiday.sql b/resources/sql/autopatches/20161101.calendar.01.noholiday.sql new file mode 100644 index 0000000000..bfbabfd926 --- /dev/null +++ b/resources/sql/autopatches/20161101.calendar.01.noholiday.sql @@ -0,0 +1 @@ +DROP TABLE {$NAMESPACE}_calendar.calendar_holiday; diff --git a/scripts/calendar/import_us_holidays.php b/scripts/calendar/import_us_holidays.php deleted file mode 100755 index 78d1d50c39..0000000000 --- a/scripts/calendar/import_us_holidays.php +++ /dev/null @@ -1,62 +0,0 @@ -#!/usr/bin/env php - "New Year's Day", - '2014-01-20' => 'Birthday of Martin Luther King, Jr.', - '2014-02-17' => "Washington's Birthday", - '2014-05-26' => 'Memorial Day', - '2014-07-04' => 'Independence Day', - '2014-09-01' => 'Labor Day', - '2014-10-13' => 'Columbus Day', - '2014-11-11' => 'Veterans Day', - '2014-11-27' => 'Thanksgiving Day', - '2014-12-25' => 'Christmas Day', - '2015-01-01' => "New Year's Day", - '2015-01-19' => 'Birthday of Martin Luther King, Jr.', - '2015-02-16' => "Washington's Birthday", - '2015-05-25' => 'Memorial Day', - '2015-07-03' => 'Independence Day', - '2015-09-07' => 'Labor Day', - '2015-10-12' => 'Columbus Day', - '2015-11-11' => 'Veterans Day', - '2015-11-26' => 'Thanksgiving Day', - '2015-12-25' => 'Christmas Day', - '2016-01-01' => "New Year's Day", - '2016-01-18' => 'Birthday of Martin Luther King, Jr.', - '2016-02-15' => "Washington's Birthday", - '2016-05-30' => 'Memorial Day', - '2016-07-04' => 'Independence Day', - '2016-09-05' => 'Labor Day', - '2016-10-10' => 'Columbus Day', - '2016-11-11' => 'Veterans Day', - '2016-11-24' => 'Thanksgiving Day', - '2016-12-26' => 'Christmas Day', - '2017-01-02' => "New Year's Day", - '2017-01-16' => 'Birthday of Martin Luther King, Jr.', - '2017-02-10' => "Washington's Birthday", - '2017-05-29' => 'Memorial Day', - '2017-07-04' => 'Independence Day', - '2017-09-04' => 'Labor Day', - '2017-10-09' => 'Columbus Day', - '2017-11-10' => 'Veterans Day', - '2017-11-23' => 'Thanksgiving Day', - '2017-12-25' => 'Christmas Day', -); - -$table = new PhabricatorCalendarHoliday(); -$conn_w = $table->establishConnection('w'); -$table_name = $table->getTableName(); - -foreach ($holidays as $day => $name) { - queryfx( - $conn_w, - 'INSERT IGNORE INTO %T (day, name) VALUES (%s, %s)', - $table_name, - $day, - $name); -} diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 121d3e2de3..8353c66603 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2103,8 +2103,6 @@ phutil_register_library_map(array( 'PhabricatorCalendarExternalInvitee' => 'applications/calendar/storage/PhabricatorCalendarExternalInvitee.php', 'PhabricatorCalendarExternalInviteePHIDType' => 'applications/calendar/phid/PhabricatorCalendarExternalInviteePHIDType.php', 'PhabricatorCalendarExternalInviteeQuery' => 'applications/calendar/query/PhabricatorCalendarExternalInviteeQuery.php', - 'PhabricatorCalendarHoliday' => 'applications/calendar/storage/PhabricatorCalendarHoliday.php', - 'PhabricatorCalendarHolidayTestCase' => 'applications/calendar/storage/__tests__/PhabricatorCalendarHolidayTestCase.php', 'PhabricatorCalendarICSFileImportEngine' => 'applications/calendar/import/PhabricatorCalendarICSFileImportEngine.php', 'PhabricatorCalendarICSImportEngine' => 'applications/calendar/import/PhabricatorCalendarICSImportEngine.php', 'PhabricatorCalendarICSURIImportEngine' => 'applications/calendar/import/PhabricatorCalendarICSURIImportEngine.php', @@ -6960,8 +6958,6 @@ phutil_register_library_map(array( ), 'PhabricatorCalendarExternalInviteePHIDType' => 'PhabricatorPHIDType', 'PhabricatorCalendarExternalInviteeQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', - 'PhabricatorCalendarHoliday' => 'PhabricatorCalendarDAO', - 'PhabricatorCalendarHolidayTestCase' => 'PhabricatorTestCase', 'PhabricatorCalendarICSFileImportEngine' => 'PhabricatorCalendarICSImportEngine', 'PhabricatorCalendarICSImportEngine' => 'PhabricatorCalendarImportEngine', 'PhabricatorCalendarICSURIImportEngine' => 'PhabricatorCalendarICSImportEngine', diff --git a/src/applications/calendar/storage/PhabricatorCalendarHoliday.php b/src/applications/calendar/storage/PhabricatorCalendarHoliday.php deleted file mode 100644 index be6dde58f5..0000000000 --- a/src/applications/calendar/storage/PhabricatorCalendarHoliday.php +++ /dev/null @@ -1,24 +0,0 @@ - false, - self::CONFIG_COLUMN_SCHEMA => array( - 'day' => 'date', - 'name' => 'text64', - ), - self::CONFIG_KEY_SCHEMA => array( - 'day' => array( - 'columns' => array('day'), - 'unique' => true, - ), - ), - ) + parent::getConfiguration(); - } - -} diff --git a/src/applications/calendar/storage/__tests__/PhabricatorCalendarHolidayTestCase.php b/src/applications/calendar/storage/__tests__/PhabricatorCalendarHolidayTestCase.php deleted file mode 100644 index de9a4cddd8..0000000000 --- a/src/applications/calendar/storage/__tests__/PhabricatorCalendarHolidayTestCase.php +++ /dev/null @@ -1,19 +0,0 @@ - true, - ); - } - - protected function willRunTests() { - parent::willRunTests(); - id(new PhabricatorCalendarHoliday()) - ->setDay('2012-01-02') - ->setName(pht('International Testing Day')) - ->save(); - } - -} From 3f2f81a1c8ab6ccb99caa01b7fd12b928ed30eba Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 1 Nov 2016 16:27:05 -0700 Subject: [PATCH 28/41] Remove obsolete Calendar event date storage fields Summary: Ref T11809. These have been replaced with more flexible storage that accommodates a wider range of behaviors, including those in the ICS format and RRULEs. Test Plan: - Ran migration. - Viewed, created, edited events. - Grepped for all removed names/symbols. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11809 Differential Revision: https://secure.phabricator.com/D16789 --- resources/celerity/map.php | 2 - .../20161101.calendar.02.removecolumns.sql | 17 +++++++ .../storage/PhabricatorCalendarEvent.php | 49 ++----------------- ...catorCalendarEventUntilDateTransaction.php | 3 -- .../calendar/behavior-recurring-edit.js | 41 ---------------- 5 files changed, 21 insertions(+), 91 deletions(-) create mode 100644 resources/sql/autopatches/20161101.calendar.02.removecolumns.sql delete mode 100644 webroot/rsrc/js/application/calendar/behavior-recurring-edit.js diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 99bc373146..b7f6c9042d 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -384,7 +384,6 @@ return array( 'rsrc/js/application/calendar/behavior-day-view.js' => '4b3c4443', 'rsrc/js/application/calendar/behavior-event-all-day.js' => 'b41537c9', 'rsrc/js/application/calendar/behavior-month-view.js' => 'fe33e256', - 'rsrc/js/application/calendar/behavior-recurring-edit.js' => '5f1c4d5f', 'rsrc/js/application/config/behavior-reorder-fields.js' => 'b6993408', 'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => '358c717b', 'rsrc/js/application/conpherence/behavior-conpherence-search.js' => '9bbf3762', @@ -704,7 +703,6 @@ return array( 'javelin-behavior-project-create' => '065227cc', 'javelin-behavior-quicksand-blacklist' => '7927a7d3', 'javelin-behavior-read-only-warning' => 'ba158207', - 'javelin-behavior-recurring-edit' => '5f1c4d5f', 'javelin-behavior-refresh-csrf' => 'ab2f381b', 'javelin-behavior-releeph-preview-branch' => 'b2b4fbaf', 'javelin-behavior-releeph-request-state-change' => 'a0b57eb8', diff --git a/resources/sql/autopatches/20161101.calendar.02.removecolumns.sql b/resources/sql/autopatches/20161101.calendar.02.removecolumns.sql new file mode 100644 index 0000000000..116f0f3972 --- /dev/null +++ b/resources/sql/autopatches/20161101.calendar.02.removecolumns.sql @@ -0,0 +1,17 @@ +ALTER TABLE {$NAMESPACE}_calendar.calendar_event + DROP allDayDateFrom; + +ALTER TABLE {$NAMESPACE}_calendar.calendar_event + DROP allDayDateTo; + +ALTER TABLE {$NAMESPACE}_calendar.calendar_event + DROP dateFrom; + +ALTER TABLE {$NAMESPACE}_calendar.calendar_event + DROP dateTo; + +ALTER TABLE {$NAMESPACE}_calendar.calendar_event + DROP recurrenceEndDate; + +ALTER TABLE {$NAMESPACE}_calendar.calendar_event + DROP recurrenceFrequency; diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index c40afe443e..2863ee64e8 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -52,14 +52,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO private $viewerTimezone; - // TODO: DEPRECATED. Remove once we're sure the migrations worked. - protected $allDayDateFrom; - protected $allDayDateTo; - protected $dateFrom; - protected $dateTo; - protected $recurrenceEndDate; - protected $recurrenceFrequency = array(); - private $isGhostEvent = false; private $stubInvitees; @@ -95,10 +87,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO ->setEditPolicy($edit_policy) ->setSpacePHID($actor->getDefaultSpacePHID()) ->attachInvitees(array()) - ->setDateFrom(0) - ->setDateTo(0) - ->setAllDayDateFrom(0) - ->setAllDayDateTo(0) ->setStartDateTime($datetime_start) ->setEndDateTime($datetime_end) ->attachImportSource(null) @@ -154,11 +142,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO ->setSequenceIndex($sequence) ->setIsRecurring(true) ->attachParentEvent($this) - ->attachImportSource(null) - ->setAllDayDateFrom(0) - ->setAllDayDateTo(0) - ->setDateFrom(0) - ->setDateTo(0); + ->attachImportSource(null); return $child->copyFromParent($actor, $start); } @@ -421,18 +405,8 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO 'importSourcePHID' => 'phid?', 'importUIDIndex' => 'bytes12?', 'importUID' => 'text?', - - // TODO: DEPRECATED. - 'allDayDateFrom' => 'epoch', - 'allDayDateTo' => 'epoch', - 'dateFrom' => 'epoch', - 'dateTo' => 'epoch', - 'recurrenceEndDate' => 'epoch?', ), self::CONFIG_KEY_SCHEMA => array( - 'key_date' => array( - 'columns' => array('dateFrom', 'dateTo'), - ), 'key_instance' => array( 'columns' => array('instanceOfEventPHID', 'sequenceIndex'), 'unique' => true, @@ -449,7 +423,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO ), ), self::CONFIG_SERIALIZATION => array( - 'recurrenceFrequency' => self::SERIALIZATION_JSON, 'parameters' => self::SERIALIZATION_JSON, ), ) + parent::getConfiguration(); @@ -845,12 +818,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO public function newStartDateTime() { $datetime = $this->getParameter('startDateTime'); - if ($datetime) { - return $this->newDateTimeFromDictionary($datetime); - } - - $epoch = $this->getDateFrom(); - return $this->newDateTimeFromEpoch($epoch); + return $this->newDateTimeFromDictionary($datetime); } public function getStartDateTimeEpoch() { @@ -859,12 +827,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO public function newEndDateTimeForEdit() { $datetime = $this->getParameter('endDateTime'); - if ($datetime) { - return $this->newDateTimeFromDictionary($datetime); - } - - $epoch = $this->getDateTo(); - return $this->newDateTimeFromEpoch($epoch); + return $this->newDateTimeFromDictionary($datetime); } public function newEndDateTime() { @@ -896,11 +859,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return $this->newDateTimeFromDictionary($datetime); } - $epoch = $this->getRecurrenceEndDate(); - if (!$epoch) { - return null; - } - return $this->newDateTimeFromEpoch($epoch); + return null; } public function getUntilDateTimeEpoch() { diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php index a841d4d09c..d5a914dcb4 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php @@ -18,9 +18,6 @@ final class PhabricatorCalendarEventUntilDateTransaction $actor = $this->getActor(); $editor = $this->getEditor(); - // TODO: DEPRECATED. - $object->setRecurrenceEndDate($value); - $datetime = PhutilCalendarAbsoluteDateTime::newFromDictionary($value); $datetime->setIsAllDay($editor->getNewIsAllDay()); diff --git a/webroot/rsrc/js/application/calendar/behavior-recurring-edit.js b/webroot/rsrc/js/application/calendar/behavior-recurring-edit.js deleted file mode 100644 index 7e75e3b838..0000000000 --- a/webroot/rsrc/js/application/calendar/behavior-recurring-edit.js +++ /dev/null @@ -1,41 +0,0 @@ -/** - * @provides javelin-behavior-recurring-edit - */ - - -JX.behavior('recurring-edit', function(config) { - var checkbox = JX.$(config.isRecurring); - var frequency = JX.$(config.frequency); - var end_date = JX.$(config.recurrenceEndDate); - - var end_date_checkbox = JX.DOM.find(end_date, 'input', 'calendar-enable'); - - JX.DOM.listen(checkbox, 'change', null, function() { - if (checkbox.checked) { - enableRecurring(); - } else { - disableRecurring(); - } - }); - - JX.DOM.listen(end_date, 'change', null, function() { - if (end_date_checkbox.checked) { - enableRecurring(); - } - }); - - function enableRecurring() { - checkbox.checked = true; - frequency.disabled = false; - end_date.disabled = false; - } - - function disableRecurring() { - checkbox.checked = false; - frequency.disabled = true; - end_date.disabled = true; - end_date_checkbox.checked = false; - - JX.DOM.alterClass(end_date, 'datepicker-disabled', true); - } -}); From 713f8fb373e4f8b97d7a5dbe51d2e40c0fcec7ba Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 2 Nov 2016 11:31:57 -0700 Subject: [PATCH 29/41] Fix a bug which could cause imported events to set themselves as their own parents Summary: Ref T11808. This variable is wrong, and would sometimes cause events to set themsevles as their own parents. They would then fail to load, and disrupt cursor paging. Test Plan: - Reproduced T11808 locally by reloading test data 2+ times, creating events with themselves as their own parents. - Appplied fix. - Nuked data, reloaded, no more self-parents. - Test datafile: {F1894017} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11808 Differential Revision: https://secure.phabricator.com/D16793 --- .../calendar/import/PhabricatorCalendarImportEngine.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php index bf2200b981..eafa8e332f 100644 --- a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php +++ b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php @@ -331,7 +331,7 @@ abstract class PhabricatorCalendarImportEngine foreach ($update_map as $full_uid => $event) { $parent_uid = $this->getParentNodeUID($node_map[$full_uid]); if ($parent_uid) { - $parent_phid = $update_map[$full_uid]->getPHID(); + $parent_phid = $update_map[$parent_uid]->getPHID(); } else { $parent_phid = null; } From bf0004744b3c3a4f084fb08dfc06086de962a155 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 2 Nov 2016 09:50:26 -0700 Subject: [PATCH 30/41] Move "Calendar" above "Badges" on user profiles Summary: Ref T11809. As we move toward unprototyping, this panel is probably more relevant/dynamic/interesting more often than the badges panel, I think? Particularly, I want to make the red dots a little easier to understand, and I think putting this above the fold will help aid discovery (red dot -> click -> see red dot -> see "away until ..." -> see calendar -> "oh they're at a meeting"?). This is entirely a product/subjective thing so I'm fine with not doing it or using a different order. I think there's maybe even an argument for putting this above "Projects", but "Projects" feels more core to me, at least for now. Test Plan: Viewed a user profile, saw "Calendar" above "Badges". Reviewers: chad Reviewed By: chad Maniphest Tasks: T11809 Differential Revision: https://secure.phabricator.com/D16790 --- .../controller/PhabricatorPeopleProfileViewController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/applications/people/controller/PhabricatorPeopleProfileViewController.php b/src/applications/people/controller/PhabricatorPeopleProfileViewController.php index ece5a88462..f04bc1c5fa 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileViewController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileViewController.php @@ -58,8 +58,8 @@ final class PhabricatorPeopleProfileViewController ->appendChild($feed); $projects = $this->buildProjectsView($user); - $badges = $this->buildBadgesView($user); $calendar = $this->buildCalendarDayView($user); + $badges = $this->buildBadgesView($user); require_celerity_resource('project-view-css'); $home = id(new PHUITwoColumnView()) @@ -73,8 +73,8 @@ final class PhabricatorPeopleProfileViewController ->setSideColumn( array( $projects, - $badges, $calendar, + $badges, )); $nav = $this->getProfileMenu(); From c9510cc1184e3eccae35e5b44f34e3d79c37da92 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 2 Nov 2016 10:02:51 -0700 Subject: [PATCH 31/41] Make it more clear that red dots next to usernames mean Calendar availability Summary: Ref T11809. We show a red dot next to a username to indicate that the user is away (on vacation, in a meeting, etc). It's not very obvious what this means unless you know that's what it is: when you click the username or view a hovercard, there's no visual hint about what the red dot means. It does say "Away", but there is a lot of information and it doesn't visually connect the two. Connect the two visually by putting a red dot next to the "Away" bit, too. Test Plan: Here's my version of it, this feels OK to me but could maybe be more designed: {F1893916} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11809 Differential Revision: https://secure.phabricator.com/D16791 --- src/__phutil_library_map__.php | 2 + .../view/PHUIUserAvailabilityView.php | 44 +++++++++++++++++++ .../PhabricatorUserStatusField.php | 7 ++- .../people/storage/PhabricatorUser.php | 17 ------- .../people/view/PhabricatorUserCardView.php | 7 ++- 5 files changed, 56 insertions(+), 21 deletions(-) create mode 100644 src/applications/calendar/view/PHUIUserAvailabilityView.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 8353c66603..6f18acd467 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1687,6 +1687,7 @@ phutil_register_library_map(array( 'PHUITimelineView' => 'view/phui/PHUITimelineView.php', 'PHUITwoColumnView' => 'view/phui/PHUITwoColumnView.php', 'PHUITypeaheadExample' => 'applications/uiexample/examples/PHUITypeaheadExample.php', + 'PHUIUserAvailabilityView' => 'applications/calendar/view/PHUIUserAvailabilityView.php', 'PHUIWorkboardView' => 'view/phui/PHUIWorkboardView.php', 'PHUIWorkpanelView' => 'view/phui/PHUIWorkpanelView.php', 'PassphraseAbstractKey' => 'applications/passphrase/keys/PassphraseAbstractKey.php', @@ -6461,6 +6462,7 @@ phutil_register_library_map(array( 'PHUITimelineView' => 'AphrontView', 'PHUITwoColumnView' => 'AphrontTagView', 'PHUITypeaheadExample' => 'PhabricatorUIExample', + 'PHUIUserAvailabilityView' => 'AphrontTagView', 'PHUIWorkboardView' => 'AphrontTagView', 'PHUIWorkpanelView' => 'AphrontTagView', 'PassphraseAbstractKey' => 'Phobject', diff --git a/src/applications/calendar/view/PHUIUserAvailabilityView.php b/src/applications/calendar/view/PHUIUserAvailabilityView.php new file mode 100644 index 0000000000..57583118c4 --- /dev/null +++ b/src/applications/calendar/view/PHUIUserAvailabilityView.php @@ -0,0 +1,44 @@ +user = $user; + return $this; + } + + public function getAvailableUser() { + return $this->user; + } + + protected function getTagContent() { + $viewer = $this->getViewer(); + $user = $this->getAvailableUser(); + + $until = $user->getAwayUntil(); + if (!$until) { + return pht('Available'); + } + + $away_tag = id(new PHUITagView()) + ->setType(PHUITagView::TYPE_SHADE) + ->setShade(PHUITagView::COLOR_RED) + ->setName(pht('Away')) + ->setDotColor(PHUITagView::COLOR_RED); + + $now = PhabricatorTime::getNow(); + $description = pht( + 'Away until %s', + $viewer->formatShortDateTime($until, $now)); + + return array( + $away_tag, + ' ', + $description, + ); + } + +} diff --git a/src/applications/people/customfield/PhabricatorUserStatusField.php b/src/applications/people/customfield/PhabricatorUserStatusField.php index 69e16d83db..2ae9158566 100644 --- a/src/applications/people/customfield/PhabricatorUserStatusField.php +++ b/src/applications/people/customfield/PhabricatorUserStatusField.php @@ -10,7 +10,7 @@ final class PhabricatorUserStatusField } public function getFieldName() { - return pht('Status'); + return pht('Availability'); } public function getFieldDescription() { @@ -29,7 +29,10 @@ final class PhabricatorUserStatusField public function renderPropertyViewValue(array $handles) { $user = $this->getObject(); $viewer = $this->requireViewer(); - return $user->getAvailabilityDescription($viewer); + + return id(new PHUIUserAvailabilityView()) + ->setViewer($viewer) + ->setAvailableUser($user); } } diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index acbf477eb4..b7a384a149 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -960,23 +960,6 @@ final class PhabricatorUser } - /** - * Describe the user's availability. - * - * @param PhabricatorUser Viewing user. - * @return string Human-readable description of away status. - * @task availability - */ - public function getAvailabilityDescription(PhabricatorUser $viewer) { - $until = $this->getAwayUntil(); - if ($until) { - return pht('Away until %s', phabricator_datetime($until, $viewer)); - } else { - return pht('Available'); - } - } - - /** * Get cached availability, if present. * diff --git a/src/applications/people/view/PhabricatorUserCardView.php b/src/applications/people/view/PhabricatorUserCardView.php index 60ba08c93b..0b9bc439ab 100644 --- a/src/applications/people/view/PhabricatorUserCardView.php +++ b/src/applications/people/view/PhabricatorUserCardView.php @@ -75,8 +75,11 @@ final class PhabricatorUserCardView extends AphrontTagView { if (PhabricatorApplication::isClassInstalledForViewer( 'PhabricatorCalendarApplication', $viewer)) { - $availability = $user->getAvailabilityDescription($viewer); - $body[] = $this->addItem(pht('Status'), $availability); + $body[] = $this->addItem( + pht('Availability'), + id(new PHUIUserAvailabilityView()) + ->setViewer($viewer) + ->setAvailableUser($user)); } $badges = $this->buildBadges($user, $viewer); From 64cf9204c1d9495a9a896fae563edf2a34675710 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 2 Nov 2016 13:09:08 -0700 Subject: [PATCH 32/41] In Calendar, only include the event description in the original event mail Summary: Ref T11809. This makes the mail more consistent with Differential and Maniphest, which only include additional details in the first mail in the thread. Test Plan: - Created an event with a description. - First mail included it. - Followups did not. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11809 Differential Revision: https://secure.phabricator.com/D16794 --- .../editor/PhabricatorCalendarEventEditor.php | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php index 12f7b8ba8d..b0cb844e9a 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php @@ -304,28 +304,31 @@ final class PhabricatorCalendarEventEditor protected function buildMailTemplate(PhabricatorLiskDAO $object) { $id = $object->getID(); $name = $object->getName(); + $monogram = $object->getMonogram(); return id(new PhabricatorMetaMTAMail()) - ->setSubject("E{$id}: {$name}") - ->addHeader('Thread-Topic', "E{$id}: ".$object->getName()); + ->setSubject("{$monogram}: {$name}") + ->addHeader('Thread-Topic', $monogram); } protected function buildMailBody( PhabricatorLiskDAO $object, array $xactions) { - $description = $object->getDescription(); $body = parent::buildMailBody($object, $xactions); - if (strlen($description)) { - $body->addRemarkupSection( - pht('EVENT DESCRIPTION'), - $description); + $description = $object->getDescription(); + if ($this->getIsNewObject()) { + if (strlen($description)) { + $body->addRemarkupSection( + pht('EVENT DESCRIPTION'), + $description); + } } $body->addLinkSection( pht('EVENT DETAIL'), - PhabricatorEnv::getProductionURI('/E'.$object->getID())); + PhabricatorEnv::getProductionURI($object->getURI())); $ics_attachment = $this->newICSAttachment($object); $body->addAttachment($ics_attachment); From 29313372e7159cee68dba8282b93fab9b75432fa Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 2 Nov 2016 13:34:58 -0700 Subject: [PATCH 33/41] Improve some commenting/editing behaviors for recurring events Summary: Ref T11809. Currently, commenting on a recurring event hits the same "one or all?" dialog that other edits do. For comments and edits submitted via the comment widget, we can safely assume that you mean "just this one", since it doesn't really make sense to try to bulk-edit an event from that UI. Test Plan: Commented on a recurring event parent and an event in the series. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11809 Differential Revision: https://secure.phabricator.com/D16795 --- ...PhabricatorCalendarEventEditController.php | 13 +++++-- .../PhabricatorCalendarEventEditEngine.php | 36 +++++++++++++++++-- ...abricatorCalendarEventReplyTransaction.php | 4 +++ ...habricatorCalendarEventTransactionType.php | 8 ++++- .../editengine/PhabricatorEditEngine.php | 13 ++++++- 5 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php b/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php index 3211d8052b..928526f33f 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php @@ -34,7 +34,17 @@ final class PhabricatorCalendarEventEditController ->addCancelButton($cancel_uri); } } else if ($event->getIsRecurring()) { - $mode = $request->getStr('mode'); + + // If the user submits a comment or makes an edit via comment actions, + // always target only the current event. It doesn't make sense to add + // comments to every instance of an event, and the other actions don't + // make much sense to apply to all instances either. + if ($engine->isCommentAction()) { + $mode = PhabricatorCalendarEventEditEngine::MODE_THIS; + } else { + $mode = $request->getStr('mode'); + } + if (!$mode) { $form = id(new AphrontFormView()) ->setViewer($viewer) @@ -60,7 +70,6 @@ final class PhabricatorCalendarEventEditController ->addSubmitButton(pht('Continue')) ->addCancelButton($cancel_uri) ->setDisableWorkflowOnSubmit(true); - } $engine diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php index f877bd9d13..c56cd7febb 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php @@ -295,7 +295,7 @@ final class PhabricatorCalendarEventEditEngine protected function willApplyTransactions($object, array $xactions) { $viewer = $this->getViewer(); - $this->rawTransactions = $xactions; + $this->rawTransactions = $this->cloneTransactions($xactions); $is_parent = $object->isParentEvent(); $is_child = $object->isChildEvent(); @@ -304,6 +304,30 @@ final class PhabricatorCalendarEventEditEngine $must_fork = ($is_child && $is_future) || ($is_parent && !$is_future); + if ($is_parent && !$is_future) { + // We don't necessarily need to fork if whatever we're editing is not + // inherited by children. For example, we can add a comment to the parent + // event without needing to fork. Test all the stuff we're doing and see + // if anything is actually inherited. + + $inherited_edit = false; + foreach ($xactions as $xaction) { + $modular_type = $xaction->getTransactionImplementation(); + if ($modular_type instanceof PhabricatorCalendarEventTransactionType) { + $inherited_edit = $modular_type->isInheritedEdit(); + if ($inherited_edit) { + break; + } + } + } + + // Nothing the user is trying to do requires us to fork, so we can just + // apply the changes normally. + if (!$inherited_edit) { + $must_fork = false; + } + } + if ($must_fork) { $fork_target = $object->loadForkTarget($viewer); if ($fork_target) { @@ -340,7 +364,7 @@ final class PhabricatorCalendarEventEditEngine } foreach ($targets as $target) { - $apply = clone $this->rawTransactions; + $apply = $this->cloneTransactions($this->rawTransactions); $this->applyTransactions($target, $apply); } } @@ -366,4 +390,12 @@ final class PhabricatorCalendarEventEditEngine } } + private function cloneTransactions(array $xactions) { + $result = array(); + foreach ($xactions as $xaction) { + $result[] = clone $xaction; + } + return $result; + } + } diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventReplyTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventReplyTransaction.php index e2ad9ea4be..2aad3d2bb2 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventReplyTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventReplyTransaction.php @@ -8,6 +8,10 @@ abstract class PhabricatorCalendarEventReplyTransaction return $object->getUserInviteStatus($actor_phid); } + public function isInheritedEdit() { + return false; + } + public function applyExternalEffects($object, $value) { $acting_phid = $this->getActingAsPHID(); diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventTransactionType.php b/src/applications/calendar/xaction/PhabricatorCalendarEventTransactionType.php index 53c00969e9..bcb1e13c21 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventTransactionType.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventTransactionType.php @@ -1,4 +1,10 @@ getController(); $request = $controller->getRequest(); - $action = $request->getURIData('editAction'); + $action = $this->getEditAction(); $capabilities = array(); $use_default = false; @@ -2098,6 +2098,17 @@ abstract class PhabricatorEditEngine PhabricatorPolicyCapability::CAN_EDIT); } + public function isCommentAction() { + return ($this->getEditAction() == 'comment'); + } + + public function getEditAction() { + $controller = $this->getController(); + $request = $controller->getRequest(); + return $request->getURIData('editAction'); + } + + /* -( Form Pages )--------------------------------------------------------- */ From e9b861ff1583249588932bfa836fe771770fcfdc Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 2 Nov 2016 11:02:26 -0700 Subject: [PATCH 34/41] Write a basic Calendar user guide Summary: Ref T11809. Roughly documents most of the tricky/unintuitive stuff. Also fixes a bug with "Make Recurring" with no "Until" date. Test Plan: Read document. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11809 Differential Revision: https://secure.phabricator.com/D16792 --- ...catorCalendarEventUntilDateTransaction.php | 7 +- src/docs/user/userguide/calendar.diviner | 75 +++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php index d5a914dcb4..f3d7bf5595 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php @@ -8,7 +8,12 @@ final class PhabricatorCalendarEventUntilDateTransaction public function generateOldValue($object) { $editor = $this->getEditor(); - return $object->newUntilDateTime() + $until = $object->newUntilDateTime(); + if (!$until) { + return null; + } + + return $until ->newAbsoluteDateTime() ->setIsAllDay($editor->getOldIsAllDay()) ->toDictionary(); diff --git a/src/docs/user/userguide/calendar.diviner b/src/docs/user/userguide/calendar.diviner index 3aefa84e17..dd0a31fe5d 100644 --- a/src/docs/user/userguide/calendar.diviner +++ b/src/docs/user/userguide/calendar.diviner @@ -9,6 +9,81 @@ Overview IMPORTANT: Calendar is a prototype application. See @{article:User Guide: Prototype Applications}. +Calendar allows you to schedule parties and invite other users to party with +you. Everyone loves to party. Use Calendar primarily for partying. + + +Reminders +========= + +Calendar sends reminder email before events occur. You will receive a reminder +if: + + - you have marked yourself as **attending** the event; + - the event has not been cancelled; and + - the event was not imported from an external source. + +Reminders are sent 15 minutes before events begin. + + +Availability +============ + +Across all applications, Phabricator shows a red dot next to usernames if the +user is currently attending an event. This provides a hint that they may be in +a meeting (or on vacation) and could take a while to get back to you about a +revision or task. + +You can click through to a user's profile to see more details about their +availability. + + +Importing Events +================ + +You can import events from email and from other calendar applications +(like Google Calendar and Calendar.app) into Calendar. For a detailed +guide, see @{article:Calendar User Guide: Importing Events}. + + +Exporting Events +================ + +You can export events from Calendar to other applications by downloading +events as `.ics` files or configuring a calendar subscription. + +Calendar also attaches `.ics` files containing event information when it sends +email. Most calendar applications can import these files. + +For a detailed guide to exporting events, see +@{article:Calendar User Guide: Exporting Events}. + + +Recurring Events +================ + +To create a recurring event (like a weekly meeting), first create an event +normally, then select {nav Make Recurring} from the action menu and configure +how often the event should repeat. + +**Monthly Events on the 29th, 30th or 31st**: If you configure an event to +repeat monthly and schedule the first instance on the 29th, 30th, or 31st of +the month, it can not occur on the same day every month because some months +do not have enough days. + +Instead, these events are internally scheduled to occur relative to the end +of the month. For example, if you schedule a monthly event on the 30th of a +31 day month, it will occur on the second-to-last day of each following month. + +**Complex RRULEs**: Calendar supports complex RRULEs internally (like events +that occur every-other Thursday in prime-numbered months) but does not +currently have a UI for scheduling events with complex rules. + +Future versions of Calendar may improve support for complex scheduling by using +the UI. In some cases, a partial workaround is to schedule the event in another +application (which has more complex scheduling controls available) and then +import it into Calendar. + Next Steps ========== From e4c6ae5345188c26f30eb9533270575316ed51f5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 2 Nov 2016 15:15:09 -0700 Subject: [PATCH 35/41] Smooth out various transaction/editing behaviors for Calendar Summary: Ref T11809. - Allow users to remove the "Until" date from recurring events. - When removing "Until", show a sensible string ("...set this event to repeat forever.") - When users go through the "Make Recurring" workflow, don't require them to explicitly select "Recurring: Recurring" from the dropdown. This intent is clear from clicking "Make Recurring". - When editing "All Future Events", don't literally apply date changes to them, since that doesn't make sense. We update the template, then reschedule any events which haven't been edited already. I think this is what users probably mean if they make this edit. - When creating an event with a non-default icon, don't show "alice changed the icon from Default to Party.". - Hide the "recurring mode" transaction, which had no string ("alice edited this Event.") and was redundant anyway. - Also, add a little piece of developer text to make hunting these things down easier. Test Plan: Edited various events, parents, children, made events recur, set until, unset until, viewed transactions, rescheduled parents, rescheduled children. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11809 Differential Revision: https://secure.phabricator.com/D16796 --- .../PhabricatorCalendarEventEditEngine.php | 56 +++++++++++-------- .../storage/PhabricatorCalendarEvent.php | 12 ++-- ...habricatorCalendarEventDateTransaction.php | 9 +++ ...habricatorCalendarEventIconTransaction.php | 8 +++ ...catorCalendarEventRecurringTransaction.php | 11 ++++ ...catorCalendarEventUntilDateTransaction.php | 42 +++++++++----- .../PhabricatorApplicationTransaction.php | 23 ++++++-- .../storage/PhabricatorModularTransaction.php | 4 ++ 8 files changed, 120 insertions(+), 45 deletions(-) diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php index c56cd7febb..5b31afbe1a 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php @@ -94,7 +94,7 @@ final class PhabricatorCalendarEventEditEngine // At least for now, just hide "Invitees" when editing all future events. // This may eventually deserve a more nuanced approach. - $hide_invitees = ($this->getSeriesEditMode() == self::MODE_FUTURE); + $is_future = ($this->getSeriesEditMode() == self::MODE_FUTURE); $fields = array( id(new PhabricatorTextEditField()) @@ -162,7 +162,7 @@ final class PhabricatorCalendarEventEditEngine ->setConduitTypeDescription(pht('New event host.')) ->setSingleValue($object->getHostPHID()), id(new PhabricatorDatasourceEditField()) - ->setIsHidden($hide_invitees) + ->setIsHidden($is_future) ->setKey('inviteePHIDs') ->setAliases(array('invite', 'invitee', 'invitees', 'inviteePHID')) ->setLabel(pht('Invitees')) @@ -193,8 +193,16 @@ final class PhabricatorCalendarEventEditEngine ->setConduitDescription(pht('Change the event icon.')) ->setConduitTypeDescription(pht('New event icon.')) ->setValue($object->getIcon()), + + // NOTE: We're being a little sneaky here. This field is hidden and + // always has the value "true", so it makes the event recurring when you + // submit a form which contains the field. Then we put the the field on + // the "recurring" page in the "Make Recurring" dialog to simplify the + // workflow. This is still normal, explicit field from the perspective + // of the API. + id(new PhabricatorBoolEditField()) - ->setIsHidden($object->getIsRecurring()) + ->setIsHidden(true) ->setKey('isRecurring') ->setLabel(pht('Recurring')) ->setOptions(pht('One-Time Event'), pht('Recurring Event')) @@ -203,7 +211,7 @@ final class PhabricatorCalendarEventEditEngine ->setDescription(pht('One time or recurring event.')) ->setConduitDescription(pht('Make the event recurring.')) ->setConduitTypeDescription(pht('Mark the event as a recurring event.')) - ->setValue($object->getIsRecurring()), + ->setValue(true), id(new PhabricatorSelectEditField()) ->setKey('frequency') ->setLabel(pht('Frequency')) @@ -295,35 +303,35 @@ final class PhabricatorCalendarEventEditEngine protected function willApplyTransactions($object, array $xactions) { $viewer = $this->getViewer(); - $this->rawTransactions = $this->cloneTransactions($xactions); $is_parent = $object->isParentEvent(); $is_child = $object->isChildEvent(); $is_future = ($this->getSeriesEditMode() === self::MODE_FUTURE); + // Figure out which transactions we can apply to the whole series of events. + // Some transactions (like comments) can never be bulk applied. + $inherited_xactions = array(); + foreach ($xactions as $xaction) { + $modular_type = $xaction->getModularType(); + if (!($modular_type instanceof PhabricatorCalendarEventTransactionType)) { + continue; + } + + $inherited_edit = $modular_type->isInheritedEdit(); + if ($inherited_edit) { + $inherited_xactions[] = $xaction; + } + } + $this->rawTransactions = $this->cloneTransactions($inherited_xactions); + $must_fork = ($is_child && $is_future) || ($is_parent && !$is_future); + // We don't need to fork when editing a parent event if none of the edits + // can transfer to child events. For example, commenting on a parent is + // fine. if ($is_parent && !$is_future) { - // We don't necessarily need to fork if whatever we're editing is not - // inherited by children. For example, we can add a comment to the parent - // event without needing to fork. Test all the stuff we're doing and see - // if anything is actually inherited. - - $inherited_edit = false; - foreach ($xactions as $xaction) { - $modular_type = $xaction->getTransactionImplementation(); - if ($modular_type instanceof PhabricatorCalendarEventTransactionType) { - $inherited_edit = $modular_type->isInheritedEdit(); - if ($inherited_edit) { - break; - } - } - } - - // Nothing the user is trying to do requires us to fork, so we can just - // apply the changes normally. - if (!$inherited_edit) { + if (!$inherited_xactions) { $must_fork = false; } } diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index 2863ee64e8..440e35f980 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -935,10 +935,14 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO $datetime->newAbsoluteDateTime()->toDictionary()); } - public function setUntilDateTime(PhutilCalendarDateTime $datetime) { - return $this->setParameter( - 'untilDateTime', - $datetime->newAbsoluteDateTime()->toDictionary()); + public function setUntilDateTime(PhutilCalendarDateTime $datetime = null) { + if ($datetime) { + $value = $datetime->newAbsoluteDateTime()->toDictionary(); + } else { + $value = null; + } + + return $this->setParameter('untilDateTime', $value); } public function setRecurrenceRule(PhutilCalendarRecurrenceRule $rrule) { diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventDateTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventDateTransaction.php index cc2cb59da9..253609a99e 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventDateTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventDateTransaction.php @@ -5,8 +5,17 @@ abstract class PhabricatorCalendarEventDateTransaction abstract protected function getInvalidDateMessage(); + public function isInheritedEdit() { + return false; + } + public function generateNewValue($object, $value) { $editor = $this->getEditor(); + + if ($value->isDisabled()) { + return null; + } + return $value->newPhutilDateTime() ->setIsAllDay($editor->getNewIsAllDay()) ->newAbsoluteDateTime() diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventIconTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventIconTransaction.php index d69cf20859..44daa13811 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventIconTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventIconTransaction.php @@ -13,6 +13,14 @@ final class PhabricatorCalendarEventIconTransaction $object->setIcon($value); } + public function shouldHide() { + if ($this->isCreateTransaction()) { + return true; + } + + return false; + } + public function getTitle() { $old = $this->getIconLabel($this->getOldValue()); $new = $this->getIconLabel($this->getNewValue()); diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventRecurringTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventRecurringTransaction.php index 05f312ddc4..9e4904085f 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventRecurringTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventRecurringTransaction.php @@ -13,6 +13,17 @@ final class PhabricatorCalendarEventRecurringTransaction return (int)$value; } + public function isInheritedEdit() { + return false; + } + + public function shouldHide() { + // This event isn't interesting on its own, and is accompanied by an + // "alice set this event to repeat weekly." event in normal circumstances + // anyway. + return true; + } + public function applyInternalEffects($object, $value) { $object->setIsRecurring($value); } diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php index f3d7bf5595..3d0649eab8 100644 --- a/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php @@ -23,25 +23,41 @@ final class PhabricatorCalendarEventUntilDateTransaction $actor = $this->getActor(); $editor = $this->getEditor(); - $datetime = PhutilCalendarAbsoluteDateTime::newFromDictionary($value); - $datetime->setIsAllDay($editor->getNewIsAllDay()); - - $object->setUntilDateTime($datetime); + if ($value) { + $datetime = PhutilCalendarAbsoluteDateTime::newFromDictionary($value); + $datetime->setIsAllDay($editor->getNewIsAllDay()); + $object->setUntilDateTime($datetime); + } else { + $object->setUntilDateTime(null); + } } public function getTitle() { - return pht( - '%s changed this event to repeat until %s.', - $this->renderAuthor(), - $this->renderNewDate()); + if ($this->getNewValue()) { + return pht( + '%s changed this event to repeat until %s.', + $this->renderAuthor(), + $this->renderNewDate()); + } else { + return pht( + '%s changed this event to repeat forever.', + $this->renderAuthor()); + } } public function getTitleForFeed() { - return pht( - '%s changed %s to repeat until %s.', - $this->renderAuthor(), - $this->renderObject(), - $this->renderNewDate()); + if ($this->getNewValue()) { + return pht( + '%s changed %s to repeat until %s.', + $this->renderAuthor(), + $this->renderObject(), + $this->renderNewDate()); + } else { + return pht( + '%s changed %s to repeat forever.', + $this->renderAuthor(), + $this->renderObject()); + } } protected function getInvalidDateMessage() { diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index cd0557dafe..cc583d34c2 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -1073,10 +1073,21 @@ abstract class PhabricatorApplicationTransaction break; default: - return pht( - '%s edited this %s.', - $this->renderHandleLink($author_phid), - $this->getApplicationObjectTypeName()); + // In developer mode, provide a better hint here about which string + // we're missing. + $developer_mode = 'phabricator.developer-mode'; + $is_developer = PhabricatorEnv::getEnvConfig($developer_mode); + if ($is_developer) { + return pht( + '%s edited this object (transaction type "%s").', + $this->renderHandleLink($author_phid), + $this->getTransactionType()); + } else { + return pht( + '%s edited this %s.', + $this->renderHandleLink($author_phid), + $this->getApplicationObjectTypeName()); + } } } @@ -1615,6 +1626,10 @@ abstract class PhabricatorApplicationTransaction 'editable by the transaction author.'); } + public function getModularType() { + return null; + } + /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/transactions/storage/PhabricatorModularTransaction.php b/src/applications/transactions/storage/PhabricatorModularTransaction.php index c93a559108..e62d550bb9 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransaction.php +++ b/src/applications/transactions/storage/PhabricatorModularTransaction.php @@ -7,6 +7,10 @@ abstract class PhabricatorModularTransaction abstract public function getBaseTransactionClass(); + public function getModularType() { + return $this->getTransactionImplementation(); + } + final protected function getTransactionImplementation() { if (!$this->implementation) { $this->implementation = $this->newTransactionImplementation(); From 0f1785c0aa8c9bc1fa89b3fb9678009e5ec523a3 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 3 Nov 2016 11:20:56 -0700 Subject: [PATCH 36/41] Allow EditEngine to build NUX buttons that point at the right place Summary: Fixes T11812. - Pull the logic for building the "Create Whatever" dropdown out. - Use it to generate NUX buttons, too. - Use the new logic in Paste and Maniphest. Test Plan: - Viewed Paste NUX, button worked. - Viewed Maniphest NUX with multiple create forms, button worked. Reviewers: chad, avivey Reviewed By: avivey Subscribers: avivey Maniphest Tasks: T11812 Differential Revision: https://secure.phabricator.com/D16797 --- .../query/ManiphestTaskSearchEngine.php | 10 +- .../query/PhabricatorPasteSearchEngine.php | 10 +- .../editengine/PhabricatorEditEngine.php | 137 ++++++++++++------ 3 files changed, 103 insertions(+), 54 deletions(-) diff --git a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php index c73ab12690..5246f32a0e 100644 --- a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php +++ b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php @@ -405,11 +405,11 @@ final class ManiphestTaskSearchEngine } protected function getNewUserBody() { - $create_button = id(new PHUIButtonView()) - ->setTag('a') - ->setText(pht('Create a Task')) - ->setHref('/maniphest/task/edit/') - ->setColor(PHUIButtonView::GREEN); + $viewer = $this->requireViewer(); + + $create_button = id(new ManiphestEditEngine()) + ->setViewer($viewer) + ->newNUXBUtton(pht('Create a Task')); $icon = $this->getApplication()->getIcon(); $app_name = $this->getApplication()->getName(); diff --git a/src/applications/paste/query/PhabricatorPasteSearchEngine.php b/src/applications/paste/query/PhabricatorPasteSearchEngine.php index da1f04ae09..d6b1b0e2eb 100644 --- a/src/applications/paste/query/PhabricatorPasteSearchEngine.php +++ b/src/applications/paste/query/PhabricatorPasteSearchEngine.php @@ -204,11 +204,11 @@ final class PhabricatorPasteSearchEngine } protected function getNewUserBody() { - $create_button = id(new PHUIButtonView()) - ->setTag('a') - ->setText(pht('Create a Paste')) - ->setHref('/paste/create/') - ->setColor(PHUIButtonView::GREEN); + $viewer = $this->requireViewer(); + + $create_button = id(new PhabricatorPasteEditEngine()) + ->setViewer($viewer) + ->newNUXButton(pht('Create a Paste')); $icon = $this->getApplication()->getIcon(); $app_name = $this->getApplication()->getName(); diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index feb710295f..5002b5d97d 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -1352,11 +1352,80 @@ abstract class PhabricatorEditEngine } + public function newNUXButton($text) { + $specs = $this->newCreateActionSpecifications(array()); + $head = head($specs); + + return id(new PHUIButtonView()) + ->setTag('a') + ->setText($text) + ->setHref($head['uri']) + ->setDisabled($head['disabled']) + ->setWorkflow($head['workflow']) + ->setColor(PHUIButtonView::GREEN); + } + + final public function addActionToCrumbs( PHUICrumbsView $crumbs, array $parameters = array()) { $viewer = $this->getViewer(); + $specs = $this->newCreateActionSpecifications($parameters); + + $head = head($specs); + $menu_uri = $head['uri']; + + $dropdown = null; + if (count($specs) > 1) { + $menu_icon = 'fa-caret-square-o-down'; + $menu_name = $this->getObjectCreateShortText(); + $workflow = false; + $disabled = false; + + $dropdown = id(new PhabricatorActionListView()) + ->setUser($viewer); + + foreach ($specs as $spec) { + $dropdown->addAction( + id(new PhabricatorActionView()) + ->setName($spec['name']) + ->setIcon($spec['icon']) + ->setHref($spec['uri'])) + ->setDisabled($head['disabled']) + ->setWorkflow($head['workflow']); + } + + } else { + $menu_icon = $head['icon']; + $menu_name = $head['name']; + + $workflow = $head['workflow']; + $disabled = $head['disabled']; + } + + $action = id(new PHUIListItemView()) + ->setName($menu_name) + ->setHref($menu_uri) + ->setIcon($menu_icon) + ->setWorkflow($workflow) + ->setDisabled($disabled); + + if ($dropdown) { + $action->setDropdownMenu($dropdown); + } + + $crumbs->addAction($action); + } + + + /** + * Build a raw description of available "Create New Object" UI options so + * other methods can build menus or buttons. + */ + private function newCreateActionSpecifications(array $parameters) { + $viewer = $this->getViewer(); + $can_create = $this->hasCreateCapability(); if ($can_create) { $configs = $this->loadUsableConfigurationsForCreate(); @@ -1364,12 +1433,11 @@ abstract class PhabricatorEditEngine $configs = array(); } - $dropdown = null; $disabled = false; $workflow = false; $menu_icon = 'fa-plus-square'; - + $specs = array(); if (!$configs) { if ($viewer->isLoggedIn()) { $disabled = true; @@ -1385,54 +1453,35 @@ abstract class PhabricatorEditEngine } else { $create_uri = $this->getEditURI(null, 'nocreate/'); } + + $specs[] = array( + 'name' => $this->getObjectCreateShortText(), + 'uri' => $create_uri, + 'icon' => $menu_icon, + 'disabled' => $disabled, + 'workflow' => $workflow, + ); } else { - $config = head($configs); - $form_key = $config->getIdentifier(); - $create_uri = $this->getEditURI(null, "form/{$form_key}/"); + foreach ($configs as $config) { + $form_key = $config->getIdentifier(); + $config_uri = $this->getEditURI(null, "form/{$form_key}/"); - if ($parameters) { - $create_uri = (string)id(new PhutilURI($create_uri)) - ->setQueryParams($parameters); - } - - if (count($configs) > 1) { - $menu_icon = 'fa-caret-square-o-down'; - - $dropdown = id(new PhabricatorActionListView()) - ->setUser($viewer); - - foreach ($configs as $config) { - $form_key = $config->getIdentifier(); - $config_uri = $this->getEditURI(null, "form/{$form_key}/"); - - if ($parameters) { - $config_uri = (string)id(new PhutilURI($config_uri)) - ->setQueryParams($parameters); - } - - $item_icon = 'fa-plus'; - - $dropdown->addAction( - id(new PhabricatorActionView()) - ->setName($config->getDisplayName()) - ->setIcon($item_icon) - ->setHref($config_uri)); + if ($parameters) { + $config_uri = (string)id(new PhutilURI($config_uri)) + ->setQueryParams($parameters); } + + $specs[] = array( + 'name' => $config->getDisplayName(), + 'uri' => $config_uri, + 'icon' => 'fa-plus', + 'disabled' => false, + 'workflow' => false, + ); } } - $action = id(new PHUIListItemView()) - ->setName($this->getObjectCreateShortText()) - ->setHref($create_uri) - ->setIcon($menu_icon) - ->setWorkflow($workflow) - ->setDisabled($disabled); - - if ($dropdown) { - $action->setDropdownMenu($dropdown); - } - - $crumbs->addAction($action); + return $specs; } final public function buildEditEngineCommentView($object) { From c2565d5e24371e4ece74ffbd7a21f5d55210f796 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 3 Nov 2016 13:38:06 -0700 Subject: [PATCH 37/41] Fix a bug with creating Phortune merchant accounts without applying an email address transaction and some null field issues Summary: When Phortune merchant accounts are created via mechanisms other than the web UI (for example, by Phacility unit tests) this validation check may fail. Transactions are validated even if no transactions of the given type are being applied, to allow the editor to raise errors like "Name is required!". If there's no TYPE_INVOICEEMAIL transaction, we'll get called with empty `$xactions` and fail on `strlen($new_email)` because the variable is never defined. As a secondary issue, if contactInfo, invoiceEmail or invoiceFooter are not provided the record will fail to insert (none of these are nullable). Test Plan: Ran Phacility unit tests, got a clean result for new instance creation. Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D16798 --- src/applications/phortune/editor/PhortuneMerchantEditor.php | 1 + src/applications/phortune/storage/PhortuneMerchant.php | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/applications/phortune/editor/PhortuneMerchantEditor.php b/src/applications/phortune/editor/PhortuneMerchantEditor.php index e2db3688e7..53fc7c94f2 100644 --- a/src/applications/phortune/editor/PhortuneMerchantEditor.php +++ b/src/applications/phortune/editor/PhortuneMerchantEditor.php @@ -134,6 +134,7 @@ final class PhortuneMerchantEditor } break; case PhortuneMerchantTransaction::TYPE_INVOICEEMAIL: + $new_email = null; foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case PhortuneMerchantTransaction::TYPE_INVOICEEMAIL: diff --git a/src/applications/phortune/storage/PhortuneMerchant.php b/src/applications/phortune/storage/PhortuneMerchant.php index a57e5bd69e..69675c2ebc 100644 --- a/src/applications/phortune/storage/PhortuneMerchant.php +++ b/src/applications/phortune/storage/PhortuneMerchant.php @@ -19,7 +19,10 @@ final class PhortuneMerchant extends PhortuneDAO public static function initializeNewMerchant(PhabricatorUser $actor) { return id(new PhortuneMerchant()) ->setViewPolicy(PhabricatorPolicies::getMostOpenPolicy()) - ->attachMemberPHIDs(array()); + ->attachMemberPHIDs(array()) + ->setContactInfo('') + ->setInvoiceEmail('') + ->setInvoiceFooter(''); } protected function getConfiguration() { From ac8b156e4bb21ea10c1666a469c3db9058dfbd0b Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 4 Nov 2016 15:13:36 -0700 Subject: [PATCH 38/41] Raise ICS warnings in Phabricator on ICS import Summary: Ref T11816. Depends on D16800. Show warnings generated by ICS import in the UI. Test Plan: {F1904122} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11816 Differential Revision: https://secure.phabricator.com/D16801 --- src/__phutil_library_map__.php | 2 + .../PhabricatorCalendarICSImportEngine.php | 11 ++++++ ...ricatorCalendarImportICSWarningLogType.php | 37 +++++++++++++++++++ 3 files changed, 50 insertions(+) create mode 100644 src/applications/calendar/importlog/PhabricatorCalendarImportICSWarningLogType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6f18acd467..46069785a6 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2130,6 +2130,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarImportICSFileTransaction' => 'applications/calendar/xaction/PhabricatorCalendarImportICSFileTransaction.php', 'PhabricatorCalendarImportICSLogType' => 'applications/calendar/importlog/PhabricatorCalendarImportICSLogType.php', 'PhabricatorCalendarImportICSURITransaction' => 'applications/calendar/xaction/PhabricatorCalendarImportICSURITransaction.php', + 'PhabricatorCalendarImportICSWarningLogType' => 'applications/calendar/importlog/PhabricatorCalendarImportICSWarningLogType.php', 'PhabricatorCalendarImportIgnoredNodeLogType' => 'applications/calendar/importlog/PhabricatorCalendarImportIgnoredNodeLogType.php', 'PhabricatorCalendarImportListController' => 'applications/calendar/controller/PhabricatorCalendarImportListController.php', 'PhabricatorCalendarImportLog' => 'applications/calendar/storage/PhabricatorCalendarImportLog.php', @@ -6991,6 +6992,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarImportICSFileTransaction' => 'PhabricatorCalendarImportTransactionType', 'PhabricatorCalendarImportICSLogType' => 'PhabricatorCalendarImportLogType', 'PhabricatorCalendarImportICSURITransaction' => 'PhabricatorCalendarImportTransactionType', + 'PhabricatorCalendarImportICSWarningLogType' => 'PhabricatorCalendarImportLogType', 'PhabricatorCalendarImportIgnoredNodeLogType' => 'PhabricatorCalendarImportLogType', 'PhabricatorCalendarImportListController' => 'PhabricatorCalendarController', 'PhabricatorCalendarImportLog' => array( diff --git a/src/applications/calendar/import/PhabricatorCalendarICSImportEngine.php b/src/applications/calendar/import/PhabricatorCalendarICSImportEngine.php index 3d94085469..b0cf9ba2cb 100644 --- a/src/applications/calendar/import/PhabricatorCalendarICSImportEngine.php +++ b/src/applications/calendar/import/PhabricatorCalendarICSImportEngine.php @@ -28,6 +28,17 @@ abstract class PhabricatorCalendarICSImportEngine $document = null; } + foreach ($parser->getWarnings() as $warning) { + $import->newLogMessage( + PhabricatorCalendarImportICSWarningLogType::LOGTYPE, + array( + 'ics.warning.code' => $warning['code'], + 'ics.warning.line' => $warning['line'], + 'ics.warning.text' => $warning['text'], + 'ics.warning.message' => $warning['message'], + )); + } + return $this->importEventDocument($viewer, $import, $document); } diff --git a/src/applications/calendar/importlog/PhabricatorCalendarImportICSWarningLogType.php b/src/applications/calendar/importlog/PhabricatorCalendarImportICSWarningLogType.php new file mode 100644 index 0000000000..5956d51e29 --- /dev/null +++ b/src/applications/calendar/importlog/PhabricatorCalendarImportICSWarningLogType.php @@ -0,0 +1,37 @@ +getParameter('ics.warning.code'), + $log->getParameter('ics.warning.line'), + $log->getParameter('ics.warning.message')); + } + + + public function getDisplayIcon( + PhabricatorUser $viewer, + PhabricatorCalendarImportLog $log) { + return 'fa-exclamation-triangle'; + } + + public function getDisplayColor( + PhabricatorUser $viewer, + PhabricatorCalendarImportLog $log) { + return 'yellow'; + } + +} From e337029769ceb1568f0c1a71e1aec4d39a43ee67 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 4 Nov 2016 15:42:24 -0700 Subject: [PATCH 39/41] Allow users to mark themselves as "Available", "Busy" or "Away" while attending an event Summary: Ref T11816. - Now that we can do something meaningful with them, bring back the yellow dots for "busy". - Default to "busy" when attending events (we could make this "busy" for short events and "away" for long events or something). - Let users pick how to display their attending status on the event page. - Also show which event the user is attending since I had to mess with the cache code anyway. We can get rid of this again if it doesn't feel good. Test Plan: {F1904179} {F1904180} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11816 Differential Revision: https://secure.phabricator.com/D16802 --- .../20161104.calendar.01.availability.sql | 2 + .../20161104.calendar.02.availdefault.sql | 3 + src/__phutil_library_map__.php | 2 + .../PhabricatorCalendarApplication.php | 2 + ...torCalendarEventAvailabilityController.php | 56 +++++++++++++++++++ ...PhabricatorCalendarEventViewController.php | 37 ++++++++++++ .../storage/PhabricatorCalendarEvent.php | 6 ++ .../PhabricatorCalendarEventInvitee.php | 51 +++++++++++++++++ .../view/PHUIUserAvailabilityView.php | 53 ++++++++++++++++-- .../phid/PhabricatorPeopleUserPHIDType.php | 7 ++- .../people/query/PhabricatorPeopleQuery.php | 32 ++++++++++- .../people/storage/PhabricatorUser.php | 26 +++++++++ 12 files changed, 269 insertions(+), 8 deletions(-) create mode 100644 resources/sql/autopatches/20161104.calendar.01.availability.sql create mode 100644 resources/sql/autopatches/20161104.calendar.02.availdefault.sql create mode 100644 src/applications/calendar/controller/PhabricatorCalendarEventAvailabilityController.php diff --git a/resources/sql/autopatches/20161104.calendar.01.availability.sql b/resources/sql/autopatches/20161104.calendar.01.availability.sql new file mode 100644 index 0000000000..cb64339df2 --- /dev/null +++ b/resources/sql/autopatches/20161104.calendar.01.availability.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_calendar.calendar_eventinvitee + ADD availability VARCHAR(64) NOT NULL; diff --git a/resources/sql/autopatches/20161104.calendar.02.availdefault.sql b/resources/sql/autopatches/20161104.calendar.02.availdefault.sql new file mode 100644 index 0000000000..12a42f711a --- /dev/null +++ b/resources/sql/autopatches/20161104.calendar.02.availdefault.sql @@ -0,0 +1,3 @@ +UPDATE {$NAMESPACE}_calendar.calendar_eventinvitee + SET availability = 'default' + WHERE availability = ''; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 46069785a6..4c6fb41333 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2035,6 +2035,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarEvent' => 'applications/calendar/storage/PhabricatorCalendarEvent.php', 'PhabricatorCalendarEventAcceptTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventAcceptTransaction.php', 'PhabricatorCalendarEventAllDayTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventAllDayTransaction.php', + 'PhabricatorCalendarEventAvailabilityController' => 'applications/calendar/controller/PhabricatorCalendarEventAvailabilityController.php', 'PhabricatorCalendarEventCancelController' => 'applications/calendar/controller/PhabricatorCalendarEventCancelController.php', 'PhabricatorCalendarEventCancelTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventCancelTransaction.php', 'PhabricatorCalendarEventDateTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventDateTransaction.php', @@ -6881,6 +6882,7 @@ phutil_register_library_map(array( ), 'PhabricatorCalendarEventAcceptTransaction' => 'PhabricatorCalendarEventReplyTransaction', 'PhabricatorCalendarEventAllDayTransaction' => 'PhabricatorCalendarEventTransactionType', + 'PhabricatorCalendarEventAvailabilityController' => 'PhabricatorCalendarController', 'PhabricatorCalendarEventCancelController' => 'PhabricatorCalendarController', 'PhabricatorCalendarEventCancelTransaction' => 'PhabricatorCalendarEventTransactionType', 'PhabricatorCalendarEventDateTransaction' => 'PhabricatorCalendarEventTransactionType', diff --git a/src/applications/calendar/application/PhabricatorCalendarApplication.php b/src/applications/calendar/application/PhabricatorCalendarApplication.php index 8caf8c27ec..fa534280a0 100644 --- a/src/applications/calendar/application/PhabricatorCalendarApplication.php +++ b/src/applications/calendar/application/PhabricatorCalendarApplication.php @@ -61,6 +61,8 @@ final class PhabricatorCalendarApplication extends PhabricatorApplication { => 'PhabricatorCalendarEventJoinController', 'export/(?P[1-9]\d*)/(?P[^/]*)' => 'PhabricatorCalendarEventExportController', + 'availability/(?P[1-9]\d*)/(?P[^/]+)/' + => 'PhabricatorCalendarEventAvailabilityController', ), 'export/' => array( $this->getQueryRoutePattern() diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventAvailabilityController.php b/src/applications/calendar/controller/PhabricatorCalendarEventAvailabilityController.php new file mode 100644 index 0000000000..04a2454a0f --- /dev/null +++ b/src/applications/calendar/controller/PhabricatorCalendarEventAvailabilityController.php @@ -0,0 +1,56 @@ +getViewer(); + $id = $request->getURIData('id'); + + $event = id(new PhabricatorCalendarEventQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->executeOne(); + if (!$event) { + return new Aphront404Response(); + } + + $response = $this->newImportedEventResponse($event); + if ($response) { + return $response; + } + + $cancel_uri = $event->getURI(); + + if (!$event->getIsUserAttending($viewer->getPHID())) { + return $this->newDialog() + ->setTitle(pht('Not Attending Event')) + ->appendParagraph( + pht( + 'You can not change your display availability for events you '. + 'are not attending.')) + ->addCancelButton($cancel_uri); + } + + // TODO: This endpoint currently only works via AJAX. It would be vaguely + // nice to provide a plain HTML version of the workflow where we return + // a dialog with a vanilla