From 368cd284f9fb8ef81fab27ca4a4db295332650b5 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Tue, 19 Aug 2025 17:15:21 +0100 Subject: [PATCH 1/2] Tidy --- src/utils/aotf.js | 58 +++-------------------------------- src/utils/uid.js | 17 +++++----- tests/e2e/specs/aotf.cy.js | 16 +++++----- tests/e2e/specs/menu.cy.js | 10 +++--- tests/unit/utils/aotf.spec.js | 20 ------------ 5 files changed, 27 insertions(+), 94 deletions(-) diff --git a/src/utils/aotf.js b/src/utils/aotf.js index ca39c7bef..ba523624a 100644 --- a/src/utils/aotf.js +++ b/src/utils/aotf.js @@ -50,9 +50,9 @@ import { import { Alert } from '@/model/Alert.model' import { store } from '@/store/index' -import { Tokens } from '@/utils/uid' +import { detokenise, Tokens } from '@/utils/uid' import { WorkflowState, WorkflowStateNames } from '@/model/WorkflowState.model' -import { isBoolean } from 'lodash-es' +import { isBoolean, startCase } from 'lodash-es' /** @typedef {import('@apollo/client').ApolloClient} ApolloClient */ /** @typedef {import('graphql').IntrospectionInputType} IntrospectionInputType */ @@ -199,20 +199,6 @@ export const primaryMutations = { // handle families the same as tasks primaryMutations.family = primaryMutations[cylcObjects.Namespace] -/** - * Cylc "objects" in hierarchy order. - * - * Note, this is the order they would appear in a tree representation. - */ -const identifierOrder = [ - cylcObjects.User, - cylcObjects.Workflow, - cylcObjects.CyclePoint, - cylcObjects.Namespace, - // cylcObjects.Task, - cylcObjects.Job -] - /** * Mapping of mutation argument types to Cylc "objects" (workflow, cycle, * task etc.). @@ -248,14 +234,7 @@ export const mutationMapping = { * Mutation argument types which are derived from more than one token. */ export const compoundFields = { - WorkflowID: (tokens) => { - if (tokens[cylcObjects.User]) { - return `~${tokens[cylcObjects.User]}/${tokens[cylcObjects.Workflow]}` - } - // don't provide user if not specified - // (will fallback to the UIs user) - return tokens[cylcObjects.Workflow] - }, + WorkflowID: (tokens) => detokenise(tokens, { workflow: true }), NamespaceIDGlob: (tokens) => ( // expand unspecified fields to '*' (tokens[cylcObjects.CyclePoint] || '*') + @@ -358,33 +337,6 @@ export function tokenise (id) { return ret } -/** - * Return the lowest token in the hierarchy. - * - * @param {Object} tokens - * @returns {String} - * */ -export function getType (tokens) { - let last = null - let item = null - for (const key of identifierOrder) { - item = tokens[key] - if (!item) { - break - } - last = key - } - return last -} - -/** - * Convert camel case to words. - */ -export function camelToWords (camel) { - const result = (camel || '').replace(/([A-Z])/g, ' $1') - return result.charAt(0).toUpperCase() + result.slice(1) -} - /** * Find the GraphQL object with the given name. * @@ -446,7 +398,7 @@ export function extractFields (type, fields, types) { */ export function processMutations (mutations, types) { for (const mutation of mutations) { - mutation._title = camelToWords(mutation.name) + mutation._title = startCase(mutation.name) mutation._icon = getMutationIcon(mutation.name) mutation._shortDescription = getMutationShortDesc(mutation.description) mutation._help = getMutationExtendedDesc(mutation.description) @@ -549,7 +501,7 @@ export function processArguments (mutation, types) { } pointer = pointer.ofType } - arg._title = camelToWords(arg.name) + arg._title = startCase(arg.name) arg._cylcObject = cylcObject arg._cylcType = cylcType arg._multiple = multiple diff --git a/src/utils/uid.js b/src/utils/uid.js index 9f730a27d..02092ea15 100644 --- a/src/utils/uid.js +++ b/src/utils/uid.js @@ -125,7 +125,10 @@ const RELATIVE_ID = new RegExp(` const JOB_ID = /^(\d+|NN)$/ -function detokenise (tokens, workflow = true, relative = true) { +export function detokenise ( + tokens, + { workflow, relative } = { workflow: true, relative: true } +) { let parts = [] let ret = '' @@ -136,10 +139,8 @@ function detokenise (tokens, workflow = true, relative = true) { if (tokens.workflow) { parts.push(tokens.workflow) } - if (parts) { - ret = parts.join('/') - parts = [] - } + ret = parts.join('/') + parts = [] } if (relative) { @@ -196,7 +197,7 @@ export class Tokens { let task let job - if (id === null) { + if (id == null) { throw new Error(`Invalid ID ${id}`) } @@ -268,8 +269,8 @@ export class Tokens { throw new Error(`Invalid job ID: ${this.job}`) } - this.workflowID = detokenise(this, true, false) - this.relativeID = detokenise(this, false, true) + this.workflowID = detokenise(this, { workflow: true }) + this.relativeID = detokenise(this, { relative: true }) } set (fields) { diff --git a/tests/e2e/specs/aotf.cy.js b/tests/e2e/specs/aotf.cy.js index adbad4c9c..378dda5b6 100644 --- a/tests/e2e/specs/aotf.cy.js +++ b/tests/e2e/specs/aotf.cy.js @@ -295,18 +295,18 @@ describe('Api On The Fly', () => { // it should list the one default workflow mutation // (see workflowService.primaryMutations) .get('.c-mutation-menu-list:first') - .find('.v-list-item') - .should('have.length', 2) // +1 because of the "see more" button + .find('.c-mutation') + .should('have.length', 1) // toggle the menu to "see more" items - .find('#less-more-button') + .get('#less-more-button') .click() // it should now list the five workflow mutations .get('.c-mutation-menu-list:first') - .find('.v-list-item') - .should('have.length', 6) // +1 because of the "see less" button + .find('.c-mutation') + .should('have.length', 5) // should have unauthorised mutation disabled .get('.c-mutation-menu-list:first') - .find('.v-list-item:nth-child(4)') + .find('.c-mutation:nth-child(4)') .should('exist') .should('have.class', 'v-list-item--disabled') // toggle the menu to "see less" items @@ -315,8 +315,8 @@ describe('Api On The Fly', () => { // it should list the one default workflow mutation // (see workflowService.primaryMutations) .get('.c-mutation-menu-list:first') - .find('.v-list-item') - .should('have.length', 2) // +1 because of the "see more" button + .find('.c-mutation') + .should('have.length', 1) }) }) }) diff --git a/tests/e2e/specs/menu.cy.js b/tests/e2e/specs/menu.cy.js index 48085bde8..477700945 100644 --- a/tests/e2e/specs/menu.cy.js +++ b/tests/e2e/specs/menu.cy.js @@ -16,8 +16,8 @@ */ describe('Command Menu component', () => { - const collapsedWorkflowMenuLength = 5 // (mutations + "show more" btn) - const expandedWorkflowMenuLength = 22 + const collapsedWorkflowMenuLength = 4 + const expandedWorkflowMenuLength = 21 beforeEach(() => { cy.visit('/#/workspace/one') @@ -34,7 +34,7 @@ describe('Command Menu component', () => { // the menu should now be open .get('.c-mutation-menu-list:first') .should('be.visible') - .children() + .children('.c-mutation') .should('have.length', collapsedWorkflowMenuLength) .get('.c-mutation-menu') .should('be.visible') @@ -51,7 +51,7 @@ describe('Command Menu component', () => { .click() .get('.c-mutation-menu-list') .should('be.visible') - .children() + .children('.c-mutation') .should('have.length', expandedWorkflowMenuLength) // Should close when clicking outside of the menu // (click on hidden element to avoid clicking on anything unexpected) @@ -64,7 +64,7 @@ describe('Command Menu component', () => { .click() .get('.c-mutation-menu-list') .should('be.visible') - .children() + .children('.c-mutation') .should('have.length', collapsedWorkflowMenuLength) }) diff --git a/tests/unit/utils/aotf.spec.js b/tests/unit/utils/aotf.spec.js index 9bd43b55f..58ebf5997 100644 --- a/tests/unit/utils/aotf.spec.js +++ b/tests/unit/utils/aotf.spec.js @@ -40,26 +40,6 @@ describe('aotf (Api On The Fly)', () => { }) }) - describe('getType', () => { - it('should extract the type from tokens', () => { - const tokens = {} - expect(aotf.getType(tokens)).to.deep.equal(null) - tokens[aotf.cylcObjects.User] = 'a' - expect(aotf.getType(tokens)).to.deep.equal(aotf.cylcObjects.User) - tokens[aotf.cylcObjects.Workflow] = 'b' - expect(aotf.getType(tokens)).to.deep.equal(aotf.cylcObjects.Workflow) - tokens[aotf.cylcObjects.CyclePoint] = 'c' - expect(aotf.getType(tokens)).to.deep.equal(aotf.cylcObjects.CyclePoint) - }) - }) - - describe('camelToWords', () => { - it('should convert camel case to plain text', () => { - expect(aotf.camelToWords(null)).to.equal('') - expect(aotf.camelToWords('aBC')).to.equal('A B C') - }) - }) - describe('getStates', () => { it('gets valid states', () => { expect(aotf.getStates('Valid for: running, stopped workflows.')).to.deep.equal(['running', 'stopped']) From 5ffef0c72e377bf01335ee6c9e8660d5bd57f873 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Thu, 21 Aug 2025 02:52:15 +0100 Subject: [PATCH 2/2] Fix "Log" & "Info" commands showing for families --- src/components/cylc/commandMenu/Menu.vue | 7 +- src/utils/aotf.js | 155 ++++++++++------------- tests/unit/utils/aotf.spec.js | 40 +++--- 3 files changed, 88 insertions(+), 114 deletions(-) diff --git a/src/components/cylc/commandMenu/Menu.vue b/src/components/cylc/commandMenu/Menu.vue index 18ea399c4..fcb1c4eae 100644 --- a/src/components/cylc/commandMenu/Menu.vue +++ b/src/components/cylc/commandMenu/Menu.vue @@ -335,13 +335,8 @@ export default { // displayed in the menu (this is what the skeleton-loader is for) this.isLoadingMutations = false this.types = types - let type = this.node.type - if (type === 'family') { - // show the same mutation list for families as for tasks - type = 'task' - } this.mutations = filterAssociations( - type, + this.node.type, this.node.tokens, mutations, this.user.permissions diff --git a/src/utils/aotf.js b/src/utils/aotf.js index ba523624a..29ee48bdd 100644 --- a/src/utils/aotf.js +++ b/src/utils/aotf.js @@ -71,7 +71,7 @@ import { isBoolean, startCase } from 'lodash-es' * @property {GQLType} type * @property {?string} defaultValue * @property {string=} _title - * @property {string=} _cylcObject + * @property {string=} _cylcObjects * @property {string=} _cylcType * @property {boolean=} _required * @property {boolean=} _multiple @@ -161,8 +161,8 @@ export const cylcObjects = Object.freeze({ User: 'user', Workflow: 'workflow', CyclePoint: 'cycle', - Namespace: 'task', - // Task: 'task', + Family: 'family', + Task: 'task', Job: 'job' }) @@ -183,22 +183,27 @@ export const primaryMutations = { 'hold', 'release', 'trigger', - 'kill' + 'kill', + 'play', + ], + [cylcObjects.Family]: [ + 'hold', + 'release', + 'trigger', + 'kill', + 'set', ], - [cylcObjects.Namespace]: [ + [cylcObjects.Task]: [ 'hold', 'release', 'trigger', 'kill', + 'set', 'log', 'info', - 'set' - ] + ], } -// handle families the same as tasks -primaryMutations.family = primaryMutations[cylcObjects.Namespace] - /** * Mapping of mutation argument types to Cylc "objects" (workflow, cycle, * task etc.). @@ -207,29 +212,23 @@ primaryMutations.family = primaryMutations[cylcObjects.Namespace] * auto-populate the input element in the mutation form based on the object * that was clicked on. * - * object: [[typeName: String, impliesMultiple: Boolean]] */ -export const mutationMapping = { - [cylcObjects.User]: [], - [cylcObjects.Workflow]: [ - ['WorkflowID', false] - ], - [cylcObjects.CyclePoint]: [ - ['CyclePoint', false], - ['CyclePointGlob', true] - ], - [cylcObjects.Namespace]: [ - ['NamespaceName', false], - ['NamespaceIDGlob', true] - ], - // [cylcObjects.Task]: [ - // ['TaskID', false] - // ], - [cylcObjects.Job]: [ - ['JobID', false] - ] +const mutationMapping = { + WorkflowID: [cylcObjects.Workflow], + CyclePoint: [cylcObjects.CyclePoint], + CyclePointGlob: [cylcObjects.CyclePoint], + NamespaceName: [cylcObjects.Task, cylcObjects.Family], + NamespaceIDGlob: [cylcObjects.Task, cylcObjects.Family], + // TaskID: [cylcObjects.Task], + JobID: [cylcObjects.Job], } +/** Argument types that imply multiple values. */ +const impliesMultiple = Object.freeze([ + 'CyclePointGlob', + 'NamespaceIDGlob', +]) + /** * Mutation argument types which are derived from more than one token. */ @@ -239,13 +238,13 @@ export const compoundFields = { // expand unspecified fields to '*' (tokens[cylcObjects.CyclePoint] || '*') + '/' + - (tokens[cylcObjects.Namespace] || '*') + (tokens[cylcObjects.Task] || '*') ), TaskID: (tokens) => ( // expand unspecified fields to '*' (tokens[cylcObjects.CyclePoint] || '*') + '/' + - tokens[cylcObjects.Namespace] + tokens[cylcObjects.Task] ) } @@ -284,7 +283,7 @@ export const dummyMutations = [ This only applies for the cycle point of the chosen task/family instance.`, args: [], - _appliesTo: [cylcObjects.Namespace, cylcObjects.CyclePoint], + _appliesTo: [cylcObjects.Task, cylcObjects.Family, cylcObjects.CyclePoint], _requiresInfo: true, _validStates: [WorkflowState.RUNNING.name, WorkflowState.PAUSED.name], _dialogWidth: '1200px', @@ -293,7 +292,7 @@ export const dummyMutations = [ name: 'log', description: 'View the logs.', args: [], - _appliesTo: [cylcObjects.Workflow, cylcObjects.Namespace, cylcObjects.Job], + _appliesTo: [cylcObjects.Workflow, cylcObjects.Task, cylcObjects.Job], _requiresInfo: false, _validStates: WorkflowStateNames, }, @@ -301,7 +300,7 @@ export const dummyMutations = [ name: 'info', description: 'View task information.', args: [], - _appliesTo: [cylcObjects.Namespace], + _appliesTo: [cylcObjects.Task], _requiresInfo: false }, ] @@ -453,8 +452,8 @@ export function getMutationExtendedDesc (text) { * This adds some computed fields prefixed with an underscore for later use: * _title: * Human-readable name for the mutation. - * _cylcObject: - * The Cylc Object this field relates to if any (e.g. Cycle, Task etc.). + * _cylcObjects: + * The Cylc objects this field relates to if any (e.g. Cycle, Task etc.). * _cylcType: * The underlying GraphQL type that provides this relationship * (e.g. taskID). @@ -472,8 +471,8 @@ export function processArguments (mutation, types) { for (const arg of mutation.args) { let pointer = arg.type let multiple = false - let cylcObject = null - let cylcType = null + let cylcObjects + let cylcType const required = arg.type?.kind === 'NON_NULL' while (pointer) { // walk down the nested type tree @@ -481,28 +480,16 @@ export function processArguments (mutation, types) { multiple = true } else if (pointer.kind !== 'NON_NULL' && pointer.name) { cylcType = pointer.name - for (const objectName in mutationMapping) { - for (const [type, impliesMultiple] of mutationMapping[objectName]) { - if (pointer.name === type) { - cylcObject = objectName - if (impliesMultiple) { - multiple = true - } - break - } - } - if (cylcObject) { - break - } - } - if (cylcObject) { + cylcObjects = mutationMapping[cylcType] + multiple ||= impliesMultiple.includes(cylcType) + if (cylcObjects) { break } } pointer = pointer.ofType } arg._title = startCase(arg.name) - arg._cylcObject = cylcObject + arg._cylcObjects = cylcObjects arg._cylcType = cylcType arg._multiple = multiple arg._required = required @@ -585,12 +572,12 @@ export function filterAssociations (cylcObject, tokens, mutations, permissions) let requiresInfo = mutation._requiresInfo ?? false let applies = mutation._appliesTo?.includes(cylcObject) for (const arg of mutation.args) { - if (arg._cylcObject) { - if (arg._cylcObject === cylcObject) { + if (arg._cylcObjects) { + if (arg._cylcObjects.includes(cylcObject)) { // this is the object type we are filtering for applies = true } - if (arg._required && !tokens[arg._cylcObject]) { + if (arg._required && !arg._cylcObjects.some((t) => tokens[t])) { // this cannot be satisfied by the context requiresInfo = true } @@ -807,39 +794,31 @@ export function constructQueryStr (query) { * */ export function getMutationArgsFromTokens (mutation, tokens) { const argspec = {} - let value for (const arg of mutation.args) { - if (arg._cylcObject) { - const alternate = alternateFields[arg._cylcType] - for (let token in tokens) { - if ([token, alternate].includes(arg._cylcObject)) { - if (arg.name === 'cutoff') { - // Work around for a field we don't want filled in, see: - // * https://github.com/cylc/cylc-ui/issues/1222 - // * https://github.com/cylc/cylc-ui/issues/1225 - // TODO: Once #1225 is done the field type can be safely changed in - // the schema without creating a compatibility issue with the UIS. - continue - } - if (arg._cylcObject === alternate) { - token = alternate - } - if (arg._cylcType in compoundFields) { - value = compoundFields[arg._cylcType](tokens) - } else { - value = tokens[token] - } - if (arg._multiple) { - value = [value] - } - argspec[arg.name] = value - break - } + if ( + arg._cylcObjects && + // Work around for a field we don't want filled in, see: + // * https://github.com/cylc/cylc-ui/issues/1222 + // * https://github.com/cylc/cylc-ui/issues/1225 + // TODO: Once #1225 is done the field type can be safely changed in + // the schema without creating a compatibility issue with the UIS. + arg.name !== 'cutoff' + ) { + let value + if (arg._cylcType in compoundFields) { + value = compoundFields[arg._cylcType](tokens) + } else { + const alternate = alternateFields[arg._cylcType] + const token = arg._cylcObjects.includes(alternate) + ? alternate + : arg._cylcObjects.find((t) => tokens[t]) + value = tokens[token] + } + if (value) { + argspec[arg.name] = arg._multiple ? [value] : value } } - if (!argspec[arg.name]) { - argspec[arg.name] = arg._default - } + argspec[arg.name] ||= arg._default } return argspec } diff --git a/tests/unit/utils/aotf.spec.js b/tests/unit/utils/aotf.spec.js index 58ebf5997..ce0d5307c 100644 --- a/tests/unit/utils/aotf.spec.js +++ b/tests/unit/utils/aotf.spec.js @@ -33,7 +33,7 @@ describe('aotf (Api On The Fly)', () => { expect(aotf.tokenise('~a/b')).to.deep.equal(tokens) tokens[aotf.cylcObjects.CyclePoint] = 'c' expect(aotf.tokenise('~a/b//c')).to.deep.equal(tokens) - tokens[aotf.cylcObjects.Namespace] = 'd' + tokens[aotf.cylcObjects.Task] = 'd' expect(aotf.tokenise('~a/b//c/d')).to.deep.equal(tokens) tokens[aotf.cylcObjects.Job] = '01' expect(aotf.tokenise('~a/b//c/d/01')).to.deep.equal(tokens) @@ -83,8 +83,8 @@ describe('aotf (Api On The Fly)', () => { { ...input.args[0], _title: 'Foo Bar', - _cylcObject: null, - _cylcType: null, + _cylcObjects: undefined, + _cylcType: undefined, _multiple: false, _required: false, _default: 42 @@ -124,7 +124,7 @@ describe('aotf (Api On The Fly)', () => { { ...input.args[0], _title: 'Foo Bar', - _cylcObject: aotf.cylcObjects.Workflow, + _cylcObjects: [aotf.cylcObjects.Workflow], _cylcType: 'WorkflowID', _multiple: true, // because of the LIST _required: true, // because of the NON_NULL @@ -146,12 +146,12 @@ describe('aotf (Api On The Fly)', () => { args: [ { name: 'arg1', - _cylcObject: aotf.cylcObjects.Workflow, + _cylcObjects: [aotf.cylcObjects.Workflow], _required: true }, { name: 'arg2', - _cylcObject: aotf.cylcObjects.User, + _cylcObjects: [aotf.cylcObjects.User], _required: false } ] @@ -162,7 +162,7 @@ describe('aotf (Api On The Fly)', () => { args: [ { name: 'arg1', - _cylcObject: aotf.cylcObjects.User, + _cylcObjects: [aotf.cylcObjects.User], _required: false } ] @@ -173,12 +173,12 @@ describe('aotf (Api On The Fly)', () => { args: [ { name: 'arg1', - _cylcObject: aotf.cylcObjects.Workflow, + _cylcObjects: [aotf.cylcObjects.Workflow], _required: true }, { name: 'arg2', - _cylcObject: null, + _cylcObjects: undefined, _required: true } ] @@ -191,7 +191,7 @@ describe('aotf (Api On The Fly)', () => { expect( aotf.filterAssociations( // filter by the "namespace" object - aotf.cylcObjects.Namespace, + aotf.cylcObjects.Task, tokens, mutations, permissions @@ -224,12 +224,12 @@ describe('aotf (Api On The Fly)', () => { args: [ { name: 'arg1', - _cylcObject: aotf.cylcObjects.Workflow, + _cylcObjects: [aotf.cylcObjects.Workflow], _required: true }, { name: 'arg2', - _cylcObject: null, + _cylcObjects: undefined, _required: false } ] @@ -240,12 +240,12 @@ describe('aotf (Api On The Fly)', () => { args: [ { name: 'arg1', - _cylcObject: aotf.cylcObjects.Workflow, + _cylcObjects: [aotf.cylcObjects.Workflow], _required: true }, { name: 'arg2', - _cylcObject: null, + _cylcObjects: undefined, _required: true } ] @@ -257,12 +257,12 @@ describe('aotf (Api On The Fly)', () => { args: [ { name: 'arg1', - _cylcObject: aotf.cylcObjects.Workflow, + _cylcObjects: [aotf.cylcObjects.Workflow], _required: true }, { name: 'arg2', - _cylcObject: aotf.cylcObjects.CyclePoint, + _cylcObjects: [aotf.cylcObjects.CyclePoint], _required: true } ] @@ -304,7 +304,7 @@ describe('aotf (Api On The Fly)', () => { it('should filter by permissions', () => { const args = [{ name: 'arg1', - _cylcObject: aotf.cylcObjects.Workflow, + _cylcObjects: [aotf.cylcObjects.Workflow], _required: true }] const mutations = [ @@ -641,14 +641,14 @@ describe('aotf (Api On The Fly)', () => { { name: 'arg1', _cylcType: 'WorkflowID', - _cylcObject: aotf.cylcObjects.Workflow, + _cylcObjects: [aotf.cylcObjects.Workflow], _multiple: true, _default: null }, { name: 'arg2', - _cylcType: null, - _cylcObject: null, + _cylcType: undefined, + _cylcObjects: undefined, _multiple: false, _default: 42 }