Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[24.2] Fix usability of workflow best practice attribute checking. #19230

Merged
merged 1 commit into from
Dec 8, 2024
Merged

[24.2] Fix usability of workflow best practice attribute checking. #19230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions client/src/components/Workflow/Editor/Index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,11 @@
:license="license"
:steps="steps"
:datatypes-mapper="datatypesMapper"
@onAttributes="showAttributes"
@onAttributes="
(e) => {
showAttributes(e);
}
"
@onHighlight="onHighlight"
@onUnhighlight="onUnhighlight"
@onRefactor="onAttemptRefactor"
Expand All @@ -75,6 +79,7 @@
v-else-if="isActiveSideBar('workflow-editor-attributes')"
:id="id"
:tags="tags"
:highlight="highlightAttribute"
:parameters="parameters"
:annotation="annotation"
:name="name"
Expand Down Expand Up @@ -296,10 +301,13 @@ export default {
parameters.value = getUntypedWorkflowParameters(steps.value);
}

function showAttributes() {
function showAttributes(args) {
ensureParametersSet();
stateStore.activeNodeId = null;
activityBar.value?.setActiveSideBar("workflow-editor-attributes");
if (args.highlight) {
this.highlightAttribute = args.highlight;
}
}

const name = ref("Unnamed Workflow");
Expand Down Expand Up @@ -521,6 +529,7 @@ export default {
refactorActions: [],
scrollToId: null,
highlightId: null,
highlightAttribute: null,
messageTitle: null,
messageBody: null,
messageIsError: false,
Expand Down
32 changes: 19 additions & 13 deletions client/src/components/Workflow/Editor/Lint.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,21 @@
:okay="checkAnnotation"
success-message="This workflow is annotated. Ideally, this helps the executors of the workflow
understand the purpose and usage of the workflow."
warning-message="This workflow is not annotated. Providing an annotation helps workflow executors
understand the purpose and usage of the workflow."
:warning-message="bestPracticeWarningAnnotation"
attribute-link="Annotate your Workflow."
@onClick="onAttributes" />
@onClick="onAttributes('annotation')" />
<LintSection
:okay="checkCreator"
success-message="This workflow defines creator information."
warning-message="This workflow does not specify creator(s). This is important metadata for workflows
that will be published and/or shared to help workflow executors know how to cite the
workflow authors."
:warning-message="bestPracticeWarningCreator"
attribute-link="Provide Creator Details."
@onClick="onAttributes" />
@onClick="onAttributes('creator')" />
<LintSection
:okay="checkLicense"
success-message="This workflow defines a license."
warning-message="This workflow does not specify a license. This is important metadata for workflows
that will be published and/or shared to help workflow executors understand how it
may be used."
:warning-message="bestPracticeWarningLicense"
attribute-link="Specify a License."
@onClick="onAttributes" />
@onClick="onAttributes('license')" />
<LintSection
success-message="Workflow parameters are using formal input parameters."
warning-message="This workflow uses legacy workflow parameters. They should be replaced with
Expand Down Expand Up @@ -79,6 +74,9 @@ import { DatatypesMapperModel } from "@/components/Datatypes/model";
import { useWorkflowStores } from "@/composables/workflowStores";

import {
bestPracticeWarningAnnotation,
bestPracticeWarningCreator,
bestPracticeWarningLicense,
fixAllIssues,
fixDisconnectedInput,
fixUnlabeledOutputs,
Expand Down Expand Up @@ -135,6 +133,13 @@ export default {
const { hasActiveOutputs } = storeToRefs(stepStore);
return { stores, connectionStore, stepStore, hasActiveOutputs, stateStore };
},
data() {
return {
bestPracticeWarningAnnotation: bestPracticeWarningAnnotation,
bestPracticeWarningCreator: bestPracticeWarningCreator,
bestPracticeWarningLicense: bestPracticeWarningLicense,
};
},
computed: {
showRefactor() {
// we could be even more precise here and check the inputs and such, because
Expand Down Expand Up @@ -177,8 +182,9 @@ export default {
},
},
methods: {
onAttributes() {
this.$emit("onAttributes");
onAttributes(highlight) {
const args = { highlight: highlight };
this.$emit("onAttributes", args);
},
onFixUntypedParameter(item) {
if (
Expand Down
107 changes: 102 additions & 5 deletions client/src/components/Workflow/Editor/WorkflowAttributes.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,61 @@
</b-list-group-item>
</b-list-group>
</div>
<div id="workflow-annotation-area" class="mt-2">
<div
id="workflow-annotation-area"
class="mt-2"
:class="{ 'bg-secondary': showAnnotationHightlight, 'highlight-attribute': showAnnotationHightlight }">
<b>Annotation</b>
<meta itemprop="description" :content="annotationCurrent" />
<b-textarea
id="workflow-annotation"
v-model="annotationCurrent"
@keyup="$emit('update:annotationCurrent', annotationCurrent)" />
<div class="form-text text-muted">These notes will be visible when this workflow is viewed.</div>
<b-popover
custom-class="best-practice-popover"
target="workflow-annotation"
boundary="window"
placement="right"
:show.sync="showAnnotationHightlight"
triggers="manual"
title="Best Practice"
:content="bestPracticeWarningAnnotation">
</b-popover>
</div>
<div id="workflow-license-area" class="mt-2">
<div
id="workflow-license-area"
class="mt-2"
:class="{ 'bg-secondary': showLicenseHightlight, 'highlight-attribute': showLicenseHightlight }">
<b>License</b>
<LicenseSelector :input-license="license" @onLicense="onLicense" />
<LicenseSelector id="license-selector" :input-license="license" @onLicense="onLicense" />
<b-popover
custom-class="best-practice-popover"
target="license-selector"
boundary="window"
placement="right"
:show.sync="showLicenseHightlight"
triggers="manual"
title="Best Practice"
:content="bestPracticeWarningLicense">
</b-popover>
</div>
<div id="workflow-creator-area" class="mt-2">
<div
id="workflow-creator-area"
class="mt-2"
:class="{ 'bg-secondary': showCreatorHightlight, 'highlight-attribute': showCreatorHightlight }">
<b>Creator</b>
<CreatorEditor :creators="creatorAsList" @onCreators="onCreator" />
<CreatorEditor id="creator-editor" :creators="creatorAsList" @onCreators="onCreator" />
<b-popover
custom-class="best-practice-popover"
target="creator-editor"
boundary="window"
placement="right"
:show.sync="showCreatorHightlight"
triggers="manual"
title="Best Practice"
:content="bestPracticeWarningCreator">
</b-popover>
</div>
<div class="mt-2">
<b>Tags</b>
Expand All @@ -60,13 +99,20 @@ import { format, parseISO } from "date-fns";

import { Services } from "@/components/Workflow/services";

import {
bestPracticeWarningAnnotation,
bestPracticeWarningCreator,
bestPracticeWarningLicense,
} from "./modules/linting";
import { UntypedParameters } from "./modules/parameters";

import LicenseSelector from "@/components/License/LicenseSelector.vue";
import ActivityPanel from "@/components/Panels/ActivityPanel.vue";
import CreatorEditor from "@/components/SchemaOrg/CreatorEditor.vue";
import StatelessTags from "@/components/TagsMultiselect/StatelessTags.vue";

const bestPracticeHighlightTime = 10000;

export default {
name: "WorkflowAttributes",
components: {
Expand All @@ -84,6 +130,10 @@ export default {
type: String,
default: null,
},
highlight: {
type: String,
default: null,
},
tags: {
type: Array,
required: true,
Expand Down Expand Up @@ -115,11 +165,17 @@ export default {
},
data() {
return {
bestPracticeWarningAnnotation: bestPracticeWarningAnnotation,
bestPracticeWarningCreator: bestPracticeWarningCreator,
bestPracticeWarningLicense: bestPracticeWarningLicense,
message: null,
messageVariant: null,
versionCurrent: this.version,
annotationCurrent: this.annotation,
nameCurrent: this.name,
showAnnotationHightlight: false,
showLicenseHightlight: false,
showCreatorHightlight: false,
};
},
computed: {
Expand Down Expand Up @@ -179,6 +235,36 @@ export default {
name() {
this.nameCurrent = this.name;
},
highlight: {
immediate: true,
handler(newHighlight, oldHighlight) {
if (newHighlight == oldHighlight) {
return;
}
if (newHighlight == "annotation") {
this.showAnnotationHightlight = true;
this.showCreatorHightlight = false;
this.showLicenseHightlight = false;
setTimeout(() => {
this.showAnnotationHightlight = false;
}, bestPracticeHighlightTime);
} else if (newHighlight == "creator") {
this.showAnnotationHightlight = false;
this.showCreatorHightlight = true;
this.showLicenseHightlight = false;
setTimeout(() => {
this.showCreatorHightlight = false;
}, bestPracticeHighlightTime);
} else if (newHighlight == "license") {
this.showAnnotationHightlight = false;
this.showCreatorHightlight = false;
this.showLicenseHightlight = true;
setTimeout(() => {
this.showLicenseHightlight = false;
}, bestPracticeHighlightTime);
}
},
},
},
created() {
this.services = new Services();
Expand Down Expand Up @@ -211,3 +297,14 @@ export default {
},
};
</script>

<style>
.highlight-attribute {
border: 1px outset;
padding: 10px;
}

.best-practice-popover {
max-width: 250px !important;
}
</style>
7 changes: 7 additions & 0 deletions client/src/components/Workflow/Editor/modules/linting.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ interface LintState {
autofix?: boolean;
}

export const bestPracticeWarningAnnotation =
"This workflow is not annotated. Providing an annotation helps workflow executors understand the purpose and usage of the workflow.";
export const bestPracticeWarningCreator =
"This workflow does not specify creator(s). This is important metadata for workflows that will be published and/or shared to help workflow executors know how to cite the workflow authors.";
export const bestPracticeWarningLicense =
"This workflow does not specify a license. This is important metadata for workflows that will be published and/or shared to help workflow executors understand how it may be used.";

export function getDisconnectedInputs(
steps: Steps = {},
datatypesMapper: DatatypesMapperModel,
Expand Down
Loading