Editing tasks results in "You cannot add more than 0 objects to the relationship" error
Closed, ResolvedPublicBUG REPORT
Actions

Assigned To

Authored By

	dancy
	Jun 15 2022, 7:21 PM

Description

List of steps to reproduce (step by step, including full links if applicable):

Visit a task.
Click "Edit Related Tasks"->"Edit Parent Tasks"
Select a parent task and click "Save Parent Tasks"

What happens?:
A dialog appears with title "Invalid Relationship" and message "You cannot add more than 0 objects to the relationship."

What should have happened instead?:
The parent tasks list should be updated successfully.

Taavi says:
looks like a custom hack of ours broken by some upstream change: https://github.com/wikimedia/phabricator/commit/4f4cae8e183d76bc758b5aaa5c966d5a607e027a

Related Objects

Mentioned In: T308069: 1.39.0-wmf.16 deployment blockers
T310665: Impact module: summary panel

Event Timeline

dancy created this task.Jun 15 2022, 7:21 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 15 2022, 7:21 PM

dancy added subscribers: kostajh, taavi.Jun 15 2022, 7:21 PM

It wasn't broken by an upstream change, it was only deployed today and the change doesn't work properly because only ManiphestTaskMergeInRelationship and ManiphestTaskCloseAsDuplicateRelationship specify a value in getMaximumSelectionSize(). This means other relationship types default to null here: https://github.com/wikimedia/phabricator/blob/0f94052396d1a7f1da1d0ed48a414c388efe38b3/src/applications/search/relationship/PhabricatorObjectRelationship.php#L108.

Aklapper triaged this task as High priority.Jun 15 2022, 7:47 PM

Aklapper mentioned this in T310665: Impact module: summary panel.

Aklapper added a project: Regression.

Ok, think I have a patch:

--git a/src/applications/maniphest/relationship/ManiphestTaskMergeInRelationship.php b/src/applications/maniphest/relationship/ManiphestTaskMergeInRelationship.php
index c2a35361d3..b99335401f 100644
--- a/src/applications/maniphest/relationship/ManiphestTaskMergeInRelationship.php
+++ b/src/applications/maniphest/relationship/ManiphestTaskMergeInRelationship.php
@@ -58,10 +58,6 @@ final class ManiphestTaskMergeInRelationship
   }
 
   public function willUpdateRelationships($object, array $add, array $rem) {
-    if (count($add) > $this->getMaximumSelectionSize()) {
-
-    }
-
     return $this->newMergeFromTransactions($add);
   }
 
diff --git a/src/applications/search/controller/PhabricatorSearchRelationshipController.php b/src/applications/search/controller/PhabricatorSearchRelationshipController.php
index 796822b25c..10c57f5a9a 100644
--- a/src/applications/search/controller/PhabricatorSearchRelationshipController.php
+++ b/src/applications/search/controller/PhabricatorSearchRelationshipController.php
@@ -81,12 +81,14 @@ final class PhabricatorSearchRelationshipController
 
       try {
         $max_selection = $relationship->getMaximumSelectionSize();
-        if (count($add_phids) && count($add_phids) > $max_selection) {
-          throw new Exception(
-            pht(
-              'You cannot add more than %d objects to the relationship.',
-              $max_selection
-            ));
+        if (! is_null($max_selection)) {
+          if (count($add_phids) && count($add_phids) > $max_selection) {
+            throw new Exception(
+              pht(
+                'You cannot add more than %d objects to the relationship.',
+                $max_selection
+              ));
+          }
         }
         foreach ($add_phids as $add_phid) {
           $dst_object = idx($dst_objects, $add_phid);`