diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -70,6 +70,7 @@ 'rsrc/css/application/feed/feed.css' => '0d17c209', 'rsrc/css/application/files/global-drag-and-drop.css' => '697324ad', 'rsrc/css/application/flag/flag.css' => '5337623f', + 'rsrc/css/application/harbormaster/harbormaster.css' => 'cec833b7', 'rsrc/css/application/herald/herald-test.css' => '2b7d0f54', 'rsrc/css/application/herald/herald.css' => '59d48f01', 'rsrc/css/application/maniphest/batch-editor.css' => '8f380ebc', @@ -521,6 +522,7 @@ 'diviner-shared-css' => '38813222', 'font-source-sans-pro' => '225851dd', 'global-drag-and-drop-css' => '697324ad', + 'harbormaster-css' => 'cec833b7', 'herald-css' => '59d48f01', 'herald-rule-editor' => '4173dbd8', 'herald-test-css' => '2b7d0f54', diff --git a/src/applications/harbormaster/controller/HarbormasterPlanOrderController.php b/src/applications/harbormaster/controller/HarbormasterPlanOrderController.php --- a/src/applications/harbormaster/controller/HarbormasterPlanOrderController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanOrderController.php @@ -1,8 +1,5 @@ getStepImplementation(); - $settings = $implementation->getSettings(); - foreach ($implementation->getSettingDefinitions() as $name => $opt) { - switch ($opt['type']) { - case BuildStepImplementation::SETTING_TYPE_ARTIFACT: - $value = $settings[$name]; - $filter = $opt['artifact_type']; - $available_artifacts = - BuildStepImplementation::getAvailableArtifacts( - $plan, - $reordered_steps, - $step, - $filter); - $artifact_found = false; - foreach ($available_artifacts as $key => $type) { - if ($key === $value) { - $artifact_found = true; - } - } - if (!$artifact_found) { - $step->setDetail($name, null); - } - break; - } - $step->save(); - } - } + // NOTE: Reordering steps may invalidate artifacts. This is fine; the UI + // will show that there are ordering issues. // Force the page to re-render. return id(new AphrontRedirectResponse()); diff --git a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php --- a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php @@ -50,7 +50,16 @@ $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb(pht("Plan %d", $id)); - $step_list = $this->buildStepList($plan); + list($step_list, $has_any_conflicts) = $this->buildStepList($plan); + + if ($has_any_conflicts) { + $box->setFormErrors( + array( + pht( + 'This build plan has conflicts in one or more build steps. '. + 'Examine the step list and resolve the listed errors.'), + )); + } return $this->buildApplicationPage( array( @@ -91,6 +100,8 @@ 'listID' => $list_id, 'orderURI' => '/harbormaster/plan/order/'.$plan->getID().'/', )); + + $has_any_conflicts = false; foreach ($steps as $step) { $implementation = null; try { @@ -121,27 +132,14 @@ ->setObjectName("Step ".$i++) ->setHeader($implementation->getName()); - if (!$implementation->validateSettings()) { - $item - ->setBarColor('red') - ->addAttribute(pht('This step is not configured correctly.')); - } else { - $item->addAttribute($implementation->getDescription()); - } + $item->addAttribute($implementation->getDescription()); + + $step_id = $step->getID(); + $edit_uri = $this->getApplicationURI("step/edit/{$step_id}/"); + $delete_uri = $this->getApplicationURI("step/delete/{$step_id}/"); if ($can_edit) { - $edit_uri = $this->getApplicationURI("step/edit/".$step->getID()."/"); - $item - ->setHref($edit_uri) - ->addAction( - id(new PHUIListItemView()) - ->setIcon('delete') - ->addSigil('harbormaster-build-step-delete') - ->setWorkflow(true) - ->setRenderNameAsTooltip(true) - ->setName(pht("Delete")) - ->setHref( - $this->getApplicationURI("step/delete/".$step->getID()."/"))); + $item->setHref($edit_uri); $item->setGrippable(true); $item->addSigil('build-step'); $item->setMetadata( @@ -150,10 +148,60 @@ )); } + $item + ->setHref($edit_uri) + ->addAction( + id(new PHUIListItemView()) + ->setIcon('delete') + ->addSigil('harbormaster-build-step-delete') + ->setWorkflow(true) + ->setDisabled(!$can_edit) + ->setHref( + $this->getApplicationURI("step/delete/".$step->getID()."/"))); + + $inputs = $step->getStepImplementation()->getArtifactInputs(); + $outputs = $step->getStepImplementation()->getArtifactOutputs(); + + $has_conflicts = false; + if ($inputs || $outputs) { + $available_artifacts = BuildStepImplementation::loadAvailableArtifacts( + $plan, + $step, + null); + + list($inputs_ui, $has_conflicts) = $this->buildArtifactList( + $inputs, + 'in', + pht('Input Artifacts'), + $available_artifacts); + + list($outputs_ui) = $this->buildArtifactList( + $outputs, + 'out', + pht('Output Artifacts'), + array()); + + $item->appendChild( + phutil_tag( + 'div', + array( + 'class' => 'harbormaster-artifact-io', + ), + array( + $inputs_ui, + $outputs_ui, + ))); + } + + if ($has_conflicts) { + $has_any_conflicts = true; + $item->setBarColor('red'); + } + $step_list->addItem($item); } - return $step_list; + return array($step_list, $has_any_conflicts); } private function buildActionList(HarbormasterBuildPlan $plan) { @@ -233,4 +281,102 @@ } + private function buildArtifactList( + array $artifacts, + $kind, + $name, + array $available_artifacts) { + $has_conflicts = false; + + if (!$artifacts) { + return array(null, $has_conflicts); + } + + + $this->requireResource('harbormaster-css'); + + $header = phutil_tag( + 'div', + array( + 'class' => 'harbormaster-artifact-summary-header', + ), + $name); + + $is_input = ($kind == 'in'); + + $list = new PHUIStatusListView(); + foreach ($artifacts as $artifact) { + $error = null; + + $key = idx($artifact, 'key'); + if (!strlen($key)) { + $bound = phutil_tag('em', array(), pht('(null)')); + if ($is_input) { + // This is an unbound input. For now, all inputs are always required. + $icon = 'warning-red'; + $icon_label = pht('Required Input'); + $has_conflicts = true; + $error = pht('This input is required, but not configured.'); + } else { + // This is an unnamed output. Outputs do not necessarily need to be + // named. + $icon = 'open'; + $icon_label = pht('Unused Output'); + } + } else { + $bound = phutil_tag('strong', array(), $key); + if ($is_input) { + if (isset($available_artifacts[$key])) { + if ($available_artifacts[$key] == idx($artifact, 'type')) { + $icon = 'accept-green'; + $icon_label = pht('Valid Input'); + } else { + $icon = 'warning-red'; + $icon_label = pht('Bad Input Type'); + $has_conflicts = true; + $error = pht( + 'This input is bound to the wrong artifact type. It is bound '. + 'to a "%s" artifact, but should be bound to a "%s" artifact.', + $available_artifacts[$key], + idx($artifact, 'type')); + } + } else { + $icon = 'question-red'; + $icon_label = pht('Unknown Input'); + $has_conflicts = true; + $error = pht( + 'This input is bound to an artifact ("%s") which does not exist '. + 'at this stage in the build process.', + $key); + } + } else { + $icon = 'down-green'; + $icon_label = pht('Valid Output'); + } + } + + if ($error) { + $note = array( + phutil_tag('strong', array(), pht('ERROR:')), + ' ', + $error); + } else { + $note = $bound; + } + + $list->addItem( + id(new PHUIStatusItemView()) + ->setIcon($icon, $icon_label) + ->setTarget($artifact['name']) + ->setNote($note)); + } + + $ui = array( + $header, + $list, + ); + + return array($ui, $has_conflicts); + } + } diff --git a/src/applications/harbormaster/step/BuildStepImplementation.php b/src/applications/harbormaster/step/BuildStepImplementation.php --- a/src/applications/harbormaster/step/BuildStepImplementation.php +++ b/src/applications/harbormaster/step/BuildStepImplementation.php @@ -50,6 +50,10 @@ return $this->settings; } + public function getSetting($key, $default = null) { + return idx($this->settings, $key, $default); + } + /** * Validate the current settings of this build step. */ @@ -103,7 +107,11 @@ * * @return array The mappings of artifact names to their types. */ - public function getArtifactMappings() { + public function getArtifactInputs() { + return array(); + } + + public function getArtifactOutputs() { return array(); } @@ -141,9 +149,10 @@ $previous_implementations[] = $build_step->getStepImplementation(); } - $artifact_arrays = mpull($previous_implementations, 'getArtifactMappings'); + $artifact_arrays = mpull($previous_implementations, 'getArtifactOutputs'); $artifacts = array(); foreach ($artifact_arrays as $array) { + $array = ipull($array, 'type', 'key'); foreach ($array as $name => $type) { if ($type !== $artifact_type && $artifact_type !== null) { continue; diff --git a/src/applications/harbormaster/step/CommandBuildStepImplementation.php b/src/applications/harbormaster/step/CommandBuildStepImplementation.php --- a/src/applications/harbormaster/step/CommandBuildStepImplementation.php +++ b/src/applications/harbormaster/step/CommandBuildStepImplementation.php @@ -90,6 +90,16 @@ return true; } + public function getArtifactInputs() { + return array( + array( + 'name' => pht('Run on Host'), + 'key' => $this->getSetting('hostartifact'), + 'type' => HarbormasterBuildArtifact::TYPE_HOST, + ), + ); + } + public function getSettingDefinitions() { return array( 'command' => array( diff --git a/src/applications/harbormaster/step/LeaseHostBuildStepImplementation.php b/src/applications/harbormaster/step/LeaseHostBuildStepImplementation.php --- a/src/applications/harbormaster/step/LeaseHostBuildStepImplementation.php +++ b/src/applications/harbormaster/step/LeaseHostBuildStepImplementation.php @@ -51,13 +51,6 @@ $artifact->save(); } - public function getArtifactMappings() { - $settings = $this->getSettings(); - - return array( - $settings['name'] => HarbormasterBuildArtifact::TYPE_HOST); - } - public function validateSettings() { $settings = $this->getSettings(); @@ -71,6 +64,16 @@ return true; } + public function getArtifactOutputs() { + return array( + array( + 'name' => pht('Leased Host'), + 'key' => $this->getSetting('name'), + 'type' => HarbormasterBuildArtifact::TYPE_HOST, + ), + ); + } + public function getSettingDefinitions() { return array( 'name' => array( diff --git a/src/applications/harbormaster/step/PublishFragmentBuildStepImplementation.php b/src/applications/harbormaster/step/PublishFragmentBuildStepImplementation.php --- a/src/applications/harbormaster/step/PublishFragmentBuildStepImplementation.php +++ b/src/applications/harbormaster/step/PublishFragmentBuildStepImplementation.php @@ -73,6 +73,16 @@ return true; } + public function getArtifactInputs() { + return array( + array( + 'name' => pht('Publishes File'), + 'key' => $this->getSetting('artifact'), + 'type' => HarbormasterBuildArtifact::TYPE_FILE, + ), + ); + } + public function getSettingDefinitions() { return array( 'path' => array( diff --git a/src/applications/harbormaster/step/UploadArtifactBuildStepImplementation.php b/src/applications/harbormaster/step/UploadArtifactBuildStepImplementation.php --- a/src/applications/harbormaster/step/UploadArtifactBuildStepImplementation.php +++ b/src/applications/harbormaster/step/UploadArtifactBuildStepImplementation.php @@ -51,13 +51,6 @@ $artifact->save(); } - public function getArtifactMappings() { - $settings = $this->getSettings(); - - return array( - $settings['name'] => HarbormasterBuildArtifact::TYPE_FILE); - } - public function validateSettings() { $settings = $this->getSettings(); @@ -77,6 +70,26 @@ return true; } + public function getArtifactInputs() { + return array( + array( + 'name' => pht('Upload From Host'), + 'key' => $this->getSetting('hostartifact'), + 'type' => HarbormasterBuildArtifact::TYPE_HOST, + ), + ); + } + + public function getArtifactOutputs() { + return array( + array( + 'name' => pht('Uploaded File'), + 'key' => $this->getSetting('name'), + 'type' => HarbormasterBuildArtifact::TYPE_FILE, + ), + ); + } + public function getSettingDefinitions() { return array( 'path' => array( diff --git a/webroot/rsrc/css/application/harbormaster/harbormaster.css b/webroot/rsrc/css/application/harbormaster/harbormaster.css new file mode 100644 --- /dev/null +++ b/webroot/rsrc/css/application/harbormaster/harbormaster.css @@ -0,0 +1,19 @@ +/** + * @provides harbormaster-css + */ + +.harbormaster-artifact-io { + margin: 0 0 0 8px; + padding: 4px 8px; + border-width: 1px 0 0 1px; + border-style: solid; + box-shadow: inset 2px 2px 1px rgba(0, 0, 0, 0.075); + background: {$lightbluebackground}; + border-color: {$lightblueborder}; +} + +.harbormaster-artifact-summary-header { + font-weight: bold; + margin-bottom: 2px; + color: {$darkbluetext}; +}