Skip to content

Commit

Permalink
Linting best practices.
Browse files Browse the repository at this point in the history
  • Loading branch information
jmchilton committed Dec 2, 2024
1 parent 8792e68 commit 7eab266
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 4 deletions.
7 changes: 7 additions & 0 deletions client/src/components/Workflow/Editor/Lint.vue
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<button class="refactor-button ui-link" @click="onRefactor">Try to automatically fix issues.</button>
</template>
<LintSection
data-description="linting has annotation"
:okay="checkAnnotation"
success-message="This workflow is annotated. Ideally, this helps the executors of the workflow
understand the purpose and usage of the workflow."
Expand All @@ -12,6 +13,7 @@
attribute-link="Annotate your Workflow."
@onClick="onAttributes" />
<LintSection
data-description="linting has creator"
:okay="checkCreator"
success-message="This workflow defines creator information."
warning-message="This workflow does not specify creator(s). This is important metadata for workflows
Expand All @@ -20,6 +22,7 @@
attribute-link="Provide Creator Details."
@onClick="onAttributes" />
<LintSection
data-description="linting has license"
:okay="checkLicense"
success-message="This workflow defines a license."
warning-message="This workflow does not specify a license. This is important metadata for workflows
Expand All @@ -28,6 +31,7 @@
attribute-link="Specify a License."
@onClick="onAttributes" />
<LintSection
data-description="linting formal inputs"
success-message="Workflow parameters are using formal input parameters."
warning-message="This workflow uses legacy workflow parameters. They should be replaced with
formal workflow inputs. Formal input parameters make tracking workflow provenance, usage within subworkflows,
Expand All @@ -37,6 +41,7 @@
@onMouseLeave="onUnhighlight"
@onClick="onFixUntypedParameter" />
<LintSection
data-description="linting connected"
success-message="All non-optional inputs to workflow steps are connected to formal input parameters."
warning-message="Some non-optional inputs are not connected to formal workflow inputs. Formal input parameters
make tracking workflow provenance, usage within subworkflows, and executing the workflow via the API more robust:"
Expand All @@ -45,13 +50,15 @@
@onMouseLeave="onUnhighlight"
@onClick="onFixDisconnectedInput" />
<LintSection
data-description="linting input metadata"
success-message="All workflow inputs have labels and annotations."
warning-message="Some workflow inputs are missing labels and/or annotations:"
:warning-items="warningMissingMetadata"
@onMouseOver="onHighlight"
@onMouseLeave="onUnhighlight"
@onClick="openAndFocus" />
<LintSection
data-description="linting output labels"
success-message="This workflow has outputs and they all have valid labels."
warning-message="The following workflow outputs have no labels, they should be assigned a useful label or
unchecked in the workflow editor to mark them as no longer being a workflow output:"
Expand Down
12 changes: 10 additions & 2 deletions client/src/components/Workflow/Editor/LintSection.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<template>
<div class="mb-2">
<div class="mb-2" :data-lint-status="isOkay ? 'ok' : 'warning'">
<div v-if="isOkay">
<FontAwesomeIcon icon="check" class="text-success" />
<span>{{ successMessage }}</span>
Expand All @@ -16,7 +16,12 @@
@mouseover="onMouseOver(item)"
@focusout="onMouseLeave(item)"
@mouseleave="onMouseLeave(item)">
<a href="#" class="scrolls" @click="onClick(item)">
<a
href="#"
class="scrolls"
:data-item-index="idx"
v-bind="dataAttributes(item)"
@click="onClick(item)">
<FontAwesomeIcon v-if="item.autofix" icon="magic" class="mr-1" />
<FontAwesomeIcon v-else icon="search" class="mr-1" />
{{ item.stepLabel }}: {{ item.warningLabel }}
Expand All @@ -37,6 +42,8 @@ import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
import BootstrapVue from "bootstrap-vue";
import Vue from "vue";
import { dataAttributes } from "./modules/linting";
Vue.use(BootstrapVue);
library.add(faMagic);
Expand Down Expand Up @@ -80,6 +87,7 @@ export default {
},
},
methods: {
dataAttributes: dataAttributes,
onMouseOver(id) {
this.$emit("onMouseOver", id);
},
Expand Down
19 changes: 19 additions & 0 deletions client/src/components/Workflow/Editor/modules/linting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ interface LintState {
name?: string;
inputName?: string;
autofix?: boolean;
data?: Record<string, string>;
}

export function getDisconnectedInputs(
Expand Down Expand Up @@ -50,25 +51,43 @@ export function getMissingMetadata(steps: Steps) {
const noAnnotation = !step.annotation;
const noLabel = !step.label;
let warningLabel = null;
const data = {
"missing-label": "false",
"missing-annotation": "false",
};
if (noLabel && noAnnotation) {
warningLabel = "Missing a label and annotation";
data["missing-label"] = "true";
data["missing-annotation"] = "true";
} else if (noLabel) {
warningLabel = "Missing a label";
data["missing-label"] = "true";
} else if (noAnnotation) {
warningLabel = "Missing an annotation";
data["missing-annotation"] = "true";
}
if (warningLabel) {
inputs.push({
stepId: step.id,
stepLabel: step.label || step.content_id || step.name,
warningLabel: warningLabel,
data: data,
});
}
}
});
return inputs;
}

export function dataAttributes(action: LintState): Record<string, string> {
const result: Record<string, string> = {};
for (const [key, value] of Object.entries(action.data || {})) {
result[`data-${key}`] = value;
}

return result;
}

export function getUnlabeledOutputs(steps: Steps) {
const outputs: LintState[] = [];
Object.values(steps).forEach((step) => {
Expand Down
7 changes: 7 additions & 0 deletions client/src/utils/navigation/navigation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,13 @@ workflow_editor:
attributes: "#activity-workflow-editor-attributes"
changes: "#activity-workflow-undo-redo"
upgrade_all: "#activity-workflow-upgrade"
best_practices: "#activity-workflow-best-practices"

best_practices:
selectors:
section_input_metadata: '[data-description="linting input metadata"]'
item_input_metadata: '[data-description="linting input metadata"] [data-item-index="${index}"]'

comment:
selectors:
_: ".workflow-editor-comment"
Expand Down
10 changes: 10 additions & 0 deletions lib/galaxy/selenium/navigates_galaxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -1226,6 +1226,16 @@ def workflow_editor_set_tool_vesrion(self, version: str, node: Optional[EditorNo
editor.tool_version_button.wait_for_and_click()
assert self.select_dropdown_item(f"Switch to {version}"), "Switch to tool version dropdown item not found"

def workflow_editor_set_node_label(self, label: str, node: Optional[EditorNodeReference] = None):
self.workflow_editor_ensure_tool_form_open(node)
editor = self.components.workflow_editor
editor.label_input.wait_for_and_clear_and_send_keys(label)

def workflow_editor_set_node_annotation(self, annotation: str, node: Optional[EditorNodeReference] = None):
self.workflow_editor_ensure_tool_form_open(node)
editor = self.components.workflow_editor
editor.annotation_input.wait_for_and_clear_and_send_keys(annotation)

def workflow_editor_ensure_tool_form_open(self, node: Optional[EditorNodeReference] = None):
# if node is_empty just assume current tool step is open
editor = self.components.workflow_editor
Expand Down
36 changes: 34 additions & 2 deletions lib/galaxy_test/selenium/test_workflow_editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ def test_data_input(self):
name = self.workflow_create_new()
self.workflow_editor_add_input(item_name="data_input")
self.screenshot("workflow_editor_data_input_new")
editor.label_input.wait_for_and_send_keys("input1")
editor.annotation_input.wait_for_and_send_keys("my cool annotation")
self.workflow_editor_set_node_label("input1")
self.workflow_editor_set_node_annotation("my cool annotation")
self.sleep_for(self.wait_types.UX_RENDER)
self.screenshot("workflow_editor_data_input_filled_in")
self.workflow_editor_click_save()
Expand Down Expand Up @@ -465,6 +465,38 @@ def test_rendering_simple_nested_workflow(self):
self.workflow_editor_maximize_center_pane()
self.screenshot("workflow_editor_simple_nested")

@selenium_test
def test_best_practices_input_label(self):
editor = self.components.workflow_editor
annotation = "best_practices_input_label"
self.workflow_create_new(annotation=annotation)
self.workflow_editor_add_input(item_name="data_input")
editor.tool_bar.best_practices.wait_for_and_click()
best_practices = editor.best_practices
section_element = best_practices.section_input_metadata.wait_for_present()
assert section_element.get_attribute("data-lint-status") == "warning"
item = best_practices.item_input_metadata(index=0)
element = item.wait_for_present()
assert element.get_attribute("data-missing-label") == "true"
assert element.get_attribute("data-missing-annotation") == "true"

item.wait_for_and_click()
self.workflow_editor_set_node_label("best practice input")

# editor.tool_bar.best_practices.wait_for_and_click()
element = item.wait_for_present()
assert element.get_attribute("data-missing-label") == "false"
assert element.get_attribute("data-missing-annotation") == "true"

self.workflow_editor_set_node_annotation("informative annotation")

@retry_assertion_during_transitions
def assert_linting_input_metadata_okay():
section_element = best_practices.section_input_metadata.wait_for_present()
assert section_element.get_attribute("data-lint-status") == "ok"

assert_linting_input_metadata_okay()

@selenium_test
def test_rendering_rules_workflow_1(self):
self.open_in_workflow_editor(WORKFLOW_WITH_RULES_1)
Expand Down

0 comments on commit 7eab266

Please sign in to comment.