Page MenuHomePhabricator

PhabricatorGuideQuickStartModule duplicates quickstart entries in cluster mode
Closed, ResolvedPublic

Description

PhabricatorGuideQuickStartModule adds the list of quickstart actions for the /guides/quickstart/ endpoint. However, the logic in this module is buggy; I created some repos in a new free Phacility instance earlier (omg exciting) -- on the right, the "Recent Activity" suggests I look at the QuickStart. When I did, I see this:

pasted_file (604×879 px, 72 KB)

The buggy code is highlighted here and inline for posterity:

$title = pht('Explore Applications');
$href = PhabricatorEnv::getURI('/applications/');
$icon = 'fa-globe';
$icon_bg = 'bg-sky';
$description =
  pht('See all the applications included in Phabricator.');

$item = id(new PhabricatorGuideItemView())
  ->setTitle($title)
  ->setHref($href)
  ->setIcon($icon)
  ->setIconBackground($icon_bg)
  ->setDescription($description);
$guide_items->addItem($item);

if (!$instance) {
  $title = pht('Invite Collaborators');
  $people_check = id(new PhabricatorPeopleQuery())
    ->setViewer($viewer)
    ->execute();
  $people = count($people_check);
  $href = PhabricatorEnv::getURI('/people/invite/send/');
  if ($people > 1) {
    $icon = 'fa-check';
    $icon_bg = 'bg-green';
    $description = pht(
      'Your invitations have been accepted. You will not be alone on '.
      'this journey.');
  } else {
    $icon = 'fa-group';
    $icon_bg = 'bg-sky';
    $description =
      pht('Invite the rest of your team to get started on Phabricator.');
  }
}

$item = id(new PhabricatorGuideItemView())
  ->setTitle($title)
  ->setHref($href)
  ->setIcon($icon)
  ->setIconBackground($icon_bg)
  ->setDescription($description);
$guide_items->addItem($item);

Where $instance = PhabricatorEnv::getEnvConfig('cluster.instance'); -- that is, if the current instance is part of a cluster, then the if will be skipped, and the prior entry for Explore Applications will be added again.

I'd offer a patch but I'm not sure how you'd like to fix it. I mean, you can just move the ->addItem into the if block, which will just fix the duplicate problem, but it's unclear to me why it even matters if this is in clustered mode? Also, it would be nice if this worked on Phacility anyway, because this quick start entry makes sense there -- it could link back to admin.phacility.com, under InstancesOverview

Note: I have tagged this under Phacility because this is where I encountered it, and also because most users running in clustered mode (strictly speaking) are likely using Phacility as well.

Version information: Phacility cluster, at the time of submission.