Page MenuHomePhorge
Differential D25111

Allow for reordering milestones
AbandonedPublic
Actions

Authored by Cigaryno on Apr 5 2023, 17:23.
Tags
Referenced Files
F3351311: D25111.1743785433.diff
Thu, Apr 3, 16:50
F3351310: D25111.1743785430.diff
Thu, Apr 3, 16:50
F3345595: D25111.1743687565.diff
Wed, Apr 2, 13:39
F3341757: D25111.1743657280.diff
Wed, Apr 2, 05:14
F3331890: D25111.1743514170.diff
Mon, Mar 31, 13:29
F3328889: D25111.1743464271.diff
Sun, Mar 30, 23:37
F3325376: D25111.1743414504.diff
Sun, Mar 30, 09:48
F3324013: D25111.1743393375.diff
Sun, Mar 30, 03:56
View All 33 Files
Subscribers
Cigaryno
Matthew
speck
tobiaswiese
valerio.bozzolan

Details

Reviewers
valerio.bozzolan
Group Reviewers
O1: Blessed Committers
Maniphest Tasks
T15082: Consider allowing milestone columns to be ordered arbitrarily on workboards
Summary
This revision's diff has been copied from Wikimedia. Please warn me if there is an error.

Ref T15082

Test Plan

Reorder milestones in the Subprojects menu.

Diff Detail

Repository
rP Phorge
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Cigaryno created this revision.Apr 5 2023, 17:23
Owners added a reviewer: O1: Blessed Committers.Apr 5 2023, 17:23
Herald added subscribers: Matthew, valerio.bozzolan, tobiaswiese, speck. · View Herald TranscriptApr 5 2023, 17:23
Cigaryno requested review of this revision.Apr 5 2023, 17:23
Cigaryno edited the summary of this revision. (Show Details)Apr 5 2023, 17:29
Cigaryno edited the test plan for this revision. (Show Details)
valerio.bozzolan added a comment.Apr 5 2023, 17:30

Maybe I can propose to control this new behavior by a new method:

public isMilestoneReordable() {
   // historically this was false, but hey, this is a community edition
   return true;
}

So that Evan can keep this to false
So other installations can customize this thing

Cigaryno mentioned this in Z1: Phorge.Apr 5 2023, 17:33
Cigaryno subscribed.
valerio.bozzolan mentioned this in D25123: Fix a PHP 8.1 issue related to preg_match() and null subject.Apr 14 2023, 14:02
valerio.bozzolan mentioned this in rARCf4a639944de4: Fix a PHP 8.1 issue related to preg_match() and null subject.Apr 14 2023, 20:13
valerio.bozzolan requested changes to this revision.EditedMay 3 2023, 09:50

Since this change is highly controversial from the opinion of the glorious Evan, I would like to propose to introduce a flag, like this one:

public isMilestoneReordable() {
   return true;  // if we are bold
   return false; // if we are not bold
}

Premising I have NOT a strong opinion about the default value, I think that a dedicated activation method is strongly necessary to allow Evan to disable this more easily, or Wikimedia to enable this more easily, without very minimal custom ad-hoc code.

This revision now requires changes to proceed.May 3 2023, 09:50
valerio.bozzolan edited the summary of this revision. (Show Details)Oct 11 2023, 06:18
valerio.bozzolan added a task: T15082: Consider allowing milestone columns to be ordered arbitrarily on workboards.
Cigaryno abandoned this revision.Dec 17 2024, 18:48

Revision Contents
Changeset List

Diff 455

View Options

src/applications/project/controller/PhabricatorProjectBoardReorderController.php

Show All 26 Lines protected function getBuiltinProfileItems($object) {
$items[] = $this->newItem() $items[] = $this->newItem()
->setMenuItemKey(PhabricatorDashboardPortalMenuItem::MENUITEMKEY) ->setMenuItemKey(PhabricatorDashboardPortalMenuItem::MENUITEMKEY)
->setBuiltinKey('manage') ->setBuiltinKey('manage')
->setIsTailItem(true); ->setIsTailItem(true);
return $items; return $items;
} }
protected function newNoMenuItemsView(array $items) { protected function newNoContentView(array $items) {
valerio.bozzolanUnsubmitted
Done
Inline Actions

Interestingly, the method newNoMenuItemsView() seems orphan!

$ grep -RF -- '->newNoMenuItemsView' .
no results

So probably we can just drop this method

valerio.bozzolan: Interestingly, the method `newNoMenuItemsView()` seems orphan! ``` $ grep -RF -- '…
aklapperAuthorUnsubmitted
Done
Inline Actions

Uh right, thanks, seems https://we.phorge.it/rPdfe47157d322d3500f5eee204672cdb3d2c32c16 removed all except for this one, and creating an empty portal shows the strings defined in src/applications/search/engine/PhabricatorProfileMenuEngine.php instead.

aklapper: Uh right, thanks, seems https://we.phorge.it/rPdfe47157d322d3500f5eee204672cdb3d2c32c16 removed…
valerio.bozzolanUnsubmitted
Done
Inline Actions

Last comment promise... but I discovered that renaming to newNoContentView() ... renders something! with:

protected function newNoContentView(array $items) {
  $object = $this->getProfileObject();
  $builtins = $this->getBuiltinProfileItems($object);
  if (count($items) <= count($builtins)) {
    return $this->newEmptyView(
      pht('New Portal'),
      pht('Use "Edit Menu" to add menu items to this portal.'));
  } else {
    return $this->newEmptyView(
      pht('No Portal Content'),
      pht(
        'None of the visible menu items in this portal can render any '.
        'content.'),
    );
  }
}

With:

  • line 35 from newNoMenuItemsView to newNoContentView
  • line newNoContentView to be adjusted to avoid newNoContentView() (to avoid recursions)

In this case the test plan would be:

  • having a portal with nothing: renders first message
  • having a portal with just one "Divider": renders second message
  • having a portal with something useful (e.g. a Dashboard): shows the Dashboard
valerio.bozzolan: Last comment promise... but I discovered that renaming to `newNoContentView()` ... renders…
$object = $this->getProfileObject(); $object = $this->getProfileObject();
$builtins = $this->getBuiltinProfileItems($object); $builtins = $this->getBuiltinProfileItems($object);
if (count($items) <= count($builtins)) { if (count($items) <= count($builtins)) {
return $this->newEmptyView( return $this->newEmptyView(
pht('New Portal'), pht('New Portal'),
pht('Use "Edit Menu" to add menu items to this portal.')); pht('Use "Edit Menu" to add menu items to this portal.'));
} else { } else {
return $this->newNoContentView( return $this->newEmptyView(
pht('No Portal Content'), pht('No Portal Content'),
pht( pht(
'None of the visible menu items in this portal can render any '. 'None of the visible menu items in this portal can render any '.
'content.')); 'content.'));
} }
} }
} }
View Options

src/applications/project/storage/PhabricatorProjectColumn.php

Show All 26 Lines protected function getBuiltinProfileItems($object) {
$items[] = $this->newItem() $items[] = $this->newItem()
->setMenuItemKey(PhabricatorDashboardPortalMenuItem::MENUITEMKEY) ->setMenuItemKey(PhabricatorDashboardPortalMenuItem::MENUITEMKEY)
->setBuiltinKey('manage') ->setBuiltinKey('manage')
->setIsTailItem(true); ->setIsTailItem(true);
return $items; return $items;
} }
protected function newNoMenuItemsView(array $items) { protected function newNoContentView(array $items) {
valerio.bozzolanUnsubmitted
Done
Inline Actions

Interestingly, the method newNoMenuItemsView() seems orphan!

$ grep -RF -- '->newNoMenuItemsView' .
no results

So probably we can just drop this method

valerio.bozzolan: Interestingly, the method `newNoMenuItemsView()` seems orphan! ``` $ grep -RF -- '…
aklapperAuthorUnsubmitted
Done
Inline Actions

Uh right, thanks, seems https://we.phorge.it/rPdfe47157d322d3500f5eee204672cdb3d2c32c16 removed all except for this one, and creating an empty portal shows the strings defined in src/applications/search/engine/PhabricatorProfileMenuEngine.php instead.

aklapper: Uh right, thanks, seems https://we.phorge.it/rPdfe47157d322d3500f5eee204672cdb3d2c32c16 removed…
valerio.bozzolanUnsubmitted
Done
Inline Actions

Last comment promise... but I discovered that renaming to newNoContentView() ... renders something! with:

protected function newNoContentView(array $items) {
  $object = $this->getProfileObject();
  $builtins = $this->getBuiltinProfileItems($object);
  if (count($items) <= count($builtins)) {
    return $this->newEmptyView(
      pht('New Portal'),
      pht('Use "Edit Menu" to add menu items to this portal.'));
  } else {
    return $this->newEmptyView(
      pht('No Portal Content'),
      pht(
        'None of the visible menu items in this portal can render any '.
        'content.'),
    );
  }
}

With:

  • line 35 from newNoMenuItemsView to newNoContentView
  • line newNoContentView to be adjusted to avoid newNoContentView() (to avoid recursions)

In this case the test plan would be:

  • having a portal with nothing: renders first message
  • having a portal with just one "Divider": renders second message
  • having a portal with something useful (e.g. a Dashboard): shows the Dashboard
valerio.bozzolan: Last comment promise... but I discovered that renaming to `newNoContentView()` ... renders…
$object = $this->getProfileObject(); $object = $this->getProfileObject();
$builtins = $this->getBuiltinProfileItems($object); $builtins = $this->getBuiltinProfileItems($object);
if (count($items) <= count($builtins)) { if (count($items) <= count($builtins)) {
return $this->newEmptyView( return $this->newEmptyView(
pht('New Portal'), pht('New Portal'),
pht('Use "Edit Menu" to add menu items to this portal.')); pht('Use "Edit Menu" to add menu items to this portal.'));
} else { } else {
return $this->newNoContentView( return $this->newEmptyView(
pht('No Portal Content'), pht('No Portal Content'),
pht( pht(
'None of the visible menu items in this portal can render any '. 'None of the visible menu items in this portal can render any '.
'content.')); 'content.'));
} }
} }
} }
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0