Skip to content

Commit 0204489

Browse files
author
epriestley
committed
Modularize workboard column transactions
Summary: Depends on D20279. Ref T5474. Modernize these transactions before I add a new "TriggerTransaction" for setting triggers. Test Plan: Created a column. Edited a column name and point limit. Hid and un-hid a column. Grepped for removed symbols. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T5474 Differential Revision: https://secure.phabricator.com/D20286
1 parent 252b6f2 commit 0204489

9 files changed

+209
-194
lines changed

‎src/__phutil_library_map__.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4059,6 +4059,8 @@
40594059
'PhabricatorProjectColumnEditController' => 'applications/project/controller/PhabricatorProjectColumnEditController.php',
40604060
'PhabricatorProjectColumnHeader' => 'applications/project/order/PhabricatorProjectColumnHeader.php',
40614061
'PhabricatorProjectColumnHideController' => 'applications/project/controller/PhabricatorProjectColumnHideController.php',
4062+
'PhabricatorProjectColumnLimitTransaction' => 'applications/project/xaction/column/PhabricatorProjectColumnLimitTransaction.php',
4063+
'PhabricatorProjectColumnNameTransaction' => 'applications/project/xaction/column/PhabricatorProjectColumnNameTransaction.php',
40624064
'PhabricatorProjectColumnNaturalOrder' => 'applications/project/order/PhabricatorProjectColumnNaturalOrder.php',
40634065
'PhabricatorProjectColumnOrder' => 'applications/project/order/PhabricatorProjectColumnOrder.php',
40644066
'PhabricatorProjectColumnOwnerOrder' => 'applications/project/order/PhabricatorProjectColumnOwnerOrder.php',
@@ -4070,10 +4072,12 @@
40704072
'PhabricatorProjectColumnQuery' => 'applications/project/query/PhabricatorProjectColumnQuery.php',
40714073
'PhabricatorProjectColumnSearchEngine' => 'applications/project/query/PhabricatorProjectColumnSearchEngine.php',
40724074
'PhabricatorProjectColumnStatusOrder' => 'applications/project/order/PhabricatorProjectColumnStatusOrder.php',
4075+
'PhabricatorProjectColumnStatusTransaction' => 'applications/project/xaction/column/PhabricatorProjectColumnStatusTransaction.php',
40734076
'PhabricatorProjectColumnTitleOrder' => 'applications/project/order/PhabricatorProjectColumnTitleOrder.php',
40744077
'PhabricatorProjectColumnTransaction' => 'applications/project/storage/PhabricatorProjectColumnTransaction.php',
40754078
'PhabricatorProjectColumnTransactionEditor' => 'applications/project/editor/PhabricatorProjectColumnTransactionEditor.php',
40764079
'PhabricatorProjectColumnTransactionQuery' => 'applications/project/query/PhabricatorProjectColumnTransactionQuery.php',
4080+
'PhabricatorProjectColumnTransactionType' => 'applications/project/xaction/column/PhabricatorProjectColumnTransactionType.php',
40774081
'PhabricatorProjectConfigOptions' => 'applications/project/config/PhabricatorProjectConfigOptions.php',
40784082
'PhabricatorProjectConfiguredCustomField' => 'applications/project/customfield/PhabricatorProjectConfiguredCustomField.php',
40794083
'PhabricatorProjectController' => 'applications/project/controller/PhabricatorProjectController.php',
@@ -10166,6 +10170,8 @@
1016610170
'PhabricatorProjectColumnEditController' => 'PhabricatorProjectBoardController',
1016710171
'PhabricatorProjectColumnHeader' => 'Phobject',
1016810172
'PhabricatorProjectColumnHideController' => 'PhabricatorProjectBoardController',
10173+
'PhabricatorProjectColumnLimitTransaction' => 'PhabricatorProjectColumnTransactionType',
10174+
'PhabricatorProjectColumnNameTransaction' => 'PhabricatorProjectColumnTransactionType',
1016910175
'PhabricatorProjectColumnNaturalOrder' => 'PhabricatorProjectColumnOrder',
1017010176
'PhabricatorProjectColumnOrder' => 'Phobject',
1017110177
'PhabricatorProjectColumnOwnerOrder' => 'PhabricatorProjectColumnOrder',
@@ -10180,10 +10186,12 @@
1018010186
'PhabricatorProjectColumnQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
1018110187
'PhabricatorProjectColumnSearchEngine' => 'PhabricatorApplicationSearchEngine',
1018210188
'PhabricatorProjectColumnStatusOrder' => 'PhabricatorProjectColumnOrder',
10189+
'PhabricatorProjectColumnStatusTransaction' => 'PhabricatorProjectColumnTransactionType',
1018310190
'PhabricatorProjectColumnTitleOrder' => 'PhabricatorProjectColumnOrder',
10184-
'PhabricatorProjectColumnTransaction' => 'PhabricatorApplicationTransaction',
10191+
'PhabricatorProjectColumnTransaction' => 'PhabricatorModularTransaction',
1018510192
'PhabricatorProjectColumnTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
1018610193
'PhabricatorProjectColumnTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
10194+
'PhabricatorProjectColumnTransactionType' => 'PhabricatorModularTransactionType',
1018710195
'PhabricatorProjectConfigOptions' => 'PhabricatorApplicationConfigOptions',
1018810196
'PhabricatorProjectConfiguredCustomField' => array(
1018910197
'PhabricatorProjectStandardCustomField',

‎src/applications/project/controller/PhabricatorProjectColumnEditController.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ public function handleRequest(AphrontRequest $request) {
7676

7777
$xactions = array();
7878

79-
$type_name = PhabricatorProjectColumnTransaction::TYPE_NAME;
80-
$type_limit = PhabricatorProjectColumnTransaction::TYPE_LIMIT;
79+
$type_name = PhabricatorProjectColumnNameTransaction::TRANSACTIONTYPE;
80+
$type_limit = PhabricatorProjectColumnLimitTransaction::TRANSACTIONTYPE;
8181

8282
if (!$column->getProxy()) {
8383
$xactions[] = id(new PhabricatorProjectColumnTransaction())
@@ -93,7 +93,6 @@ public function handleRequest(AphrontRequest $request) {
9393
$editor = id(new PhabricatorProjectColumnTransactionEditor())
9494
->setActor($viewer)
9595
->setContinueOnNoEffect(true)
96-
->setContinueOnMissingFields(true)
9796
->setContentSourceFromRequest($request)
9897
->applyTransactions($column, $xactions);
9998
return id(new AphrontRedirectResponse())->setURI($view_uri);

‎src/applications/project/controller/PhabricatorProjectColumnHideController.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,9 @@ public function handleRequest(AphrontRequest $request) {
8282
$new_status = PhabricatorProjectColumn::STATUS_HIDDEN;
8383
}
8484

85-
$type_status = PhabricatorProjectColumnTransaction::TYPE_STATUS;
85+
$type_status =
86+
PhabricatorProjectColumnStatusTransaction::TRANSACTIONTYPE;
87+
8688
$xactions = array(
8789
id(new PhabricatorProjectColumnTransaction())
8890
->setTransactionType($type_status)

‎src/applications/project/editor/PhabricatorProjectColumnTransactionEditor.php‎

Lines changed: 4 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -11,130 +11,12 @@ public function getEditorObjectsDescription() {
1111
return pht('Workboard Columns');
1212
}
1313

14-
public function getTransactionTypes() {
15-
$types = parent::getTransactionTypes();
16-
17-
$types[] = PhabricatorProjectColumnTransaction::TYPE_NAME;
18-
$types[] = PhabricatorProjectColumnTransaction::TYPE_STATUS;
19-
$types[] = PhabricatorProjectColumnTransaction::TYPE_LIMIT;
20-
21-
return $types;
22-
}
23-
24-
protected function getCustomTransactionOldValue(
25-
PhabricatorLiskDAO $object,
26-
PhabricatorApplicationTransaction $xaction) {
27-
28-
switch ($xaction->getTransactionType()) {
29-
case PhabricatorProjectColumnTransaction::TYPE_NAME:
30-
return $object->getName();
31-
case PhabricatorProjectColumnTransaction::TYPE_STATUS:
32-
return $object->getStatus();
33-
case PhabricatorProjectColumnTransaction::TYPE_LIMIT:
34-
return $object->getPointLimit();
35-
36-
}
37-
38-
return parent::getCustomTransactionOldValue($object, $xaction);
14+
public function getCreateObjectTitle($author, $object) {
15+
return pht('%s created this column.', $author);
3916
}
4017

41-
protected function getCustomTransactionNewValue(
42-
PhabricatorLiskDAO $object,
43-
PhabricatorApplicationTransaction $xaction) {
44-
45-
switch ($xaction->getTransactionType()) {
46-
case PhabricatorProjectColumnTransaction::TYPE_NAME:
47-
case PhabricatorProjectColumnTransaction::TYPE_STATUS:
48-
return $xaction->getNewValue();
49-
case PhabricatorProjectColumnTransaction::TYPE_LIMIT:
50-
$value = $xaction->getNewValue();
51-
if (strlen($value)) {
52-
return (int)$xaction->getNewValue();
53-
} else {
54-
return null;
55-
}
56-
}
57-
58-
return parent::getCustomTransactionNewValue($object, $xaction);
59-
}
60-
61-
protected function applyCustomInternalTransaction(
62-
PhabricatorLiskDAO $object,
63-
PhabricatorApplicationTransaction $xaction) {
64-
65-
switch ($xaction->getTransactionType()) {
66-
case PhabricatorProjectColumnTransaction::TYPE_NAME:
67-
$object->setName($xaction->getNewValue());
68-
return;
69-
case PhabricatorProjectColumnTransaction::TYPE_STATUS:
70-
$object->setStatus($xaction->getNewValue());
71-
return;
72-
case PhabricatorProjectColumnTransaction::TYPE_LIMIT:
73-
$object->setPointLimit($xaction->getNewValue());
74-
return;
75-
}
76-
77-
return parent::applyCustomInternalTransaction($object, $xaction);
78-
}
79-
80-
protected function applyCustomExternalTransaction(
81-
PhabricatorLiskDAO $object,
82-
PhabricatorApplicationTransaction $xaction) {
83-
84-
switch ($xaction->getTransactionType()) {
85-
case PhabricatorProjectColumnTransaction::TYPE_NAME:
86-
case PhabricatorProjectColumnTransaction::TYPE_STATUS:
87-
case PhabricatorProjectColumnTransaction::TYPE_LIMIT:
88-
return;
89-
}
90-
91-
return parent::applyCustomExternalTransaction($object, $xaction);
92-
}
93-
94-
protected function validateTransaction(
95-
PhabricatorLiskDAO $object,
96-
$type,
97-
array $xactions) {
98-
99-
$errors = parent::validateTransaction($object, $type, $xactions);
100-
101-
switch ($type) {
102-
case PhabricatorProjectColumnTransaction::TYPE_LIMIT:
103-
foreach ($xactions as $xaction) {
104-
$value = $xaction->getNewValue();
105-
if (strlen($value) && !preg_match('/^\d+\z/', $value)) {
106-
$errors[] = new PhabricatorApplicationTransactionValidationError(
107-
$type,
108-
pht('Invalid'),
109-
pht(
110-
'Column point limit must either be empty or a nonnegative '.
111-
'integer.'),
112-
$xaction);
113-
}
114-
}
115-
break;
116-
case PhabricatorProjectColumnTransaction::TYPE_NAME:
117-
$missing = $this->validateIsEmptyTextField(
118-
$object->getName(),
119-
$xactions);
120-
121-
// The default "Backlog" column is allowed to be unnamed, which
122-
// means we use the default name.
123-
124-
if ($missing && !$object->isDefaultColumn()) {
125-
$error = new PhabricatorApplicationTransactionValidationError(
126-
$type,
127-
pht('Required'),
128-
pht('Column name is required.'),
129-
nonempty(last($xactions), null));
130-
131-
$error->setIsMissingFieldError(true);
132-
$errors[] = $error;
133-
}
134-
break;
135-
}
136-
137-
return $errors;
18+
public function getCreateObjectTitleForFeed($author, $object) {
19+
return pht('%s created %s.', $author, $object);
13820
}
13921

14022
}

‎src/applications/project/storage/PhabricatorProjectColumnTransaction.php‎

Lines changed: 3 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
11
<?php
22

33
final class PhabricatorProjectColumnTransaction
4-
extends PhabricatorApplicationTransaction {
5-
6-
const TYPE_NAME = 'project:col:name';
7-
const TYPE_STATUS = 'project:col:status';
8-
const TYPE_LIMIT = 'project:col:limit';
4+
extends PhabricatorModularTransaction {
95

106
public function getApplicationName() {
117
return 'project';
@@ -15,68 +11,8 @@ public function getApplicationTransactionType() {
1511
return PhabricatorProjectColumnPHIDType::TYPECONST;
1612
}
1713

18-
public function getTitle() {
19-
$old = $this->getOldValue();
20-
$new = $this->getNewValue();
21-
$author_handle = $this->renderHandleLink($this->getAuthorPHID());
22-
23-
switch ($this->getTransactionType()) {
24-
case self::TYPE_NAME:
25-
if ($old === null) {
26-
return pht(
27-
'%s created this column.',
28-
$author_handle);
29-
} else {
30-
if (!strlen($old)) {
31-
return pht(
32-
'%s named this column "%s".',
33-
$author_handle,
34-
$new);
35-
} else if (strlen($new)) {
36-
return pht(
37-
'%s renamed this column from "%s" to "%s".',
38-
$author_handle,
39-
$old,
40-
$new);
41-
} else {
42-
return pht(
43-
'%s removed the custom name of this column.',
44-
$author_handle);
45-
}
46-
}
47-
case self::TYPE_LIMIT:
48-
if (!$old) {
49-
return pht(
50-
'%s set the point limit for this column to %s.',
51-
$author_handle,
52-
$new);
53-
} else if (!$new) {
54-
return pht(
55-
'%s removed the point limit for this column.',
56-
$author_handle);
57-
} else {
58-
return pht(
59-
'%s changed point limit for this column from %s to %s.',
60-
$author_handle,
61-
$old,
62-
$new);
63-
}
64-
65-
case self::TYPE_STATUS:
66-
switch ($new) {
67-
case PhabricatorProjectColumn::STATUS_ACTIVE:
68-
return pht(
69-
'%s marked this column visible.',
70-
$author_handle);
71-
case PhabricatorProjectColumn::STATUS_HIDDEN:
72-
return pht(
73-
'%s marked this column hidden.',
74-
$author_handle);
75-
}
76-
break;
77-
}
78-
79-
return parent::getTitle();
14+
public function getBaseTransactionClass() {
15+
return 'PhabricatorProjectColumnTransactionType';
8016
}
8117

8218
}

‎src/applications/project/xaction/column/PhabricatorProjectColumnLimitTransaction.php‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
3+
final class PhabricatorProjectColumnLimitTransaction
4+
extends PhabricatorProjectColumnTransactionType {
5+
6+
const TRANSACTIONTYPE = 'project:col:limit';
7+
8+
public function generateOldValue($object) {
9+
return $object->getPointLimit();
10+
}
11+
12+
public function generateNewValue($object, $value) {
13+
if (strlen($value)) {
14+
return (int)$value;
15+
} else {
16+
return null;
17+
}
18+
}
19+
20+
public function applyInternalEffects($object, $value) {
21+
$object->setPointLimit($value);
22+
}
23+
24+
public function getTitle() {
25+
$old = $this->getOldValue();
26+
$new = $this->getNewValue();
27+
28+
if (!$old) {
29+
return pht(
30+
'%s set the point limit for this column to %s.',
31+
$this->renderAuthor(),
32+
$this->renderNewValue());
33+
} else if (!$new) {
34+
return pht(
35+
'%s removed the point limit for this column.',
36+
$this->renderAuthor());
37+
} else {
38+
return pht(
39+
'%s changed the point limit for this column from %s to %s.',
40+
$this->renderAuthor(),
41+
$this->renderOldValue(),
42+
$this->renderNewValue());
43+
}
44+
}
45+
46+
public function validateTransactions($object, array $xactions) {
47+
$errors = array();
48+
49+
foreach ($xactions as $xaction) {
50+
$value = $xaction->getNewValue();
51+
if (strlen($value) && !preg_match('/^\d+\z/', $value)) {
52+
$errors[] = $this->newInvalidError(
53+
pht(
54+
'Column point limit must either be empty or a nonnegative '.
55+
'integer.'),
56+
$xaction);
57+
}
58+
}
59+
60+
return $errors;
61+
}
62+
63+
}

0 commit comments

Comments
 (0)