[OGUI-1877] Add visual elements to clearly decouple TST detector from rest of them

Control/public/common/detectorLock/detectionLockActionButton.js

@@ -12,23 +12,46 @@
  * or submit itself to any jurisdiction.
 */
 
-import {h} from '/js/src/index.js';
-import {DetectorLockState} from './../enums/DetectorLockState.enum.js';
+import { h } from '/js/src/index.js';
+import { DetectorLockState } from './../enums/DetectorLockState.enum.js';
+import { DetectorLockAction } from '../enums/DetectorLockAction.enum.js';
 
 /**
- * Button with action to force take/release lock for a detector
+ * Button with action to no-force/force take/release lock for a detector
  * @param {Lock} lockModel - model of the lock service 
- * @param {String} detector - detector name
- * @param {DetectorLockState} lockState - lock state of the detector
+ * @param {String} detector - detector name in prefix format, e.g. "ITS", "MFT", "TST"
+ * @param {object} lockState - lock state of the detector
+ * @param {DetectorLockState} lockState.state - state of the lock
  * @param {DetectorLockAction} action - action to be performed
+ * @param {boolean} shouldForce - if the action should be forced or not
  * @param {String} label - button label to be displayed to the user
- * @return {vnode}
+ * @returns {vnode}
  */
 export const detectorLockActionButton = (
   lockModel, detector, lockState, action, shouldForce = false, label = `${action}`
 ) => {
+  const isFree = lockState?.state === DetectorLockState.FREE;
+  const isReleaseAction = action === DetectorLockAction.RELEASE;
+  const isTakeAction = action === DetectorLockAction.TAKE;
+
+  let isDisabled = false;
+  let titleAndAriaLabel = `${action} lock for ${detector}`;
+
+  if (isFree && isReleaseAction) {
+    titleAndAriaLabel = `Cannot release lock for ${detector} - lock is not taken`;
+    isDisabled = true;
+  } else if (isFree && isTakeAction && shouldForce) {
+    titleAndAriaLabel = `Cannot force take lock for ${detector} - lock is already free`;
+    isDisabled = true;
+  } else if (shouldForce) {
+    titleAndAriaLabel = `Force ${action} lock for ${detector}`;
+  }
+
   return h('button.btn.btn-sm.btn-danger', {
-    disabled: lockState?.state === DetectorLockState.FREE,
+    disabled: isDisabled,
+    title: titleAndAriaLabel,
+    'aria-label': titleAndAriaLabel,
+    'aria-disabled': isDisabled ? 'true' : 'false',
     onclick: () => lockModel.actionOnLock(detector, action, shouldForce)
   }, label);
 };

Control/public/common/detectorUtils.js

@@ -0,0 +1,35 @@
+/**
+ * @license
+ * Copyright 2019-2020 CERN and copyright holders of ALICE O2.
+ * See http://alice-o2.web.cern.ch/copyright for details of the copyright holders.
+ * All rights not expressly granted are reserved.
+ *
+ * This software is distributed under the terms of the GNU General Public
+ * License v3 (GPL Version 3), copied verbatim in the file "COPYING".
+ *
+ * In applying this license CERN does not waive the privileges and immunities
+ * granted to it by virtue of its status as an Intergovernmental Organization
+ * or submit itself to any jurisdiction.
+*/
+export const TST_DETECTOR_NAME = 'TST';
+
+/**
+ * Method to return a detector list with TST detector at the end if it exists
+ * @param {String[]} detectorList - list of detector names
+ * @return {String[]} reordered list of detector names
+ */
+export const getDetectorListWithTstAtEnd = (detectorList) => {
+  let hasTstDetector = false;
+  const detectorsWithoutTst = detectorList.filter((detector) => {
+    if (detector === TST_DETECTOR_NAME) {
+      hasTstDetector = true;
+      return false;
+    }
+    return true;
+  });
+  const detectorsWithTst = detectorList.filter(detector => detector.toLocaleUpperCase().includes(TST_DETECTOR_NAME));
+  return [
+    ...detectorsWithoutTst,
+    ...(hasTstDetector ? detectorsWithTst : [])
+  ];
+}

Control/public/lock/lockButton.js

@@ -22,12 +22,11 @@ import {DetectorLockAction} from './../common/enums/DetectorLockAction.enum.js';
  * - see who owns the lock
  * 
  * When the user releases a lock, the detector also has to be unselected from the workflow.
- * @param {Model} model - root model of the application
+ * @param {LockModel} lockModel - model of the lock state and actions
  * @param {String} detector - detector name
  * @param {Object} lockState - lock state of the detector
  */
-export const detectorLockButton = (model, detector, lockState, isIcon = false) => {
-  const lockModel = model.lock;
+export const detectorLockButton = (lockModel, detector, lockState, isIcon = false) => {
   const isDetectorLockTaken = lockModel.isLocked(detector);
 
   let detectorLockHandler = null;
@@ -36,10 +35,7 @@ export const detectorLockButton = (model, detector, lockState, isIcon = false) =
   if (isDetectorLockTaken) {
     if (lockModel.isLockedByCurrentUser(detector)) {
       detectorLockButtonClass = '.success';
-      detectorLockHandler = () => {
-        lockModel.actionOnLock(detector, DetectorLockAction.RELEASE, false);
-        model.workflow.flpSelection.unselectDetector(detector);
-      };
+      detectorLockHandler = () => lockModel.actionOnLock(detector, DetectorLockAction.RELEASE, false);
     } else {
       detectorLockButtonClass = '.warning.disabled.disabled-item';
     }

Control/public/lock/lockPage.js

@@ -21,6 +21,7 @@ import errorPage from './../common/errorPage.js';
 import loading from './../common/loading.js';
 import {DetectorLockAction} from '../common/enums/DetectorLockAction.enum.js';
 import {isUserAllowedRole} from './../common/userRole.js';
+import { getDetectorListWithTstAtEnd, TST_DETECTOR_NAME } from '../common/detectorUtils.js';
 
 const LOCK_TABLE_HEADER_KEYS = ['Detector', 'Owner'];
 const DETECTOR_ALL = 'ALL';
@@ -31,16 +32,12 @@ const DETECTOR_ALL = 'ALL';
 
 /**
  * Header of the lock page
- * @param {Object} model
  * @return {vnode}
  */
-export const header = (model) => [
+export const header = () => [
   h('.w-100.text-center', [
     h('h4', 'Locks')
   ]),
-  model.detectors.selected === 'GLOBAL' && h('.flex-row.text-right', {
-    style: 'position: absolute; right: 0px;'
-  })
 ];
 
 /**
@@ -49,8 +46,8 @@ export const header = (model) => [
  * @return {vnode}
  */
 export const content = (model) => {
-  const padlockState = model.lock.padlockState;
-  const lock = model.lock;
+  const {lock: lockModel, detectors: detectorsService} = model;
+  const { padlockState } = lockModel;
   return [
     detectorHeader(model),
     h('.text-center.scroll-y.absolute-fill', {style: 'top: 40px'}, [
@@ -59,19 +56,28 @@ export const content = (model) => {
         Loading: () => loading(3),
         Failure: (error) => errorPage(error),
         Success: (detectorsLocksState) => h('.flex-column', [
-          h('.flex-row.g2.pv2', [
-            isUserAllowedRole(ROLES.Admin) && [
-              detectorLockActionButton(lock, DETECTOR_ALL, {}, DetectorLockAction.RELEASE, true, 'Force Release ALL'),
-              detectorLockActionButton(lock, DETECTOR_ALL, {}, DetectorLockAction.TAKE, true, 'Force Take ALL'),
-            ],
-            isUserAllowedRole(ROLES.Global) && [
-              detectorLockActionButton(lock, DETECTOR_ALL, {}, DetectorLockAction.RELEASE, false, 'Release ALL*'),
-              detectorLockActionButton(lock, DETECTOR_ALL, {}, DetectorLockAction.TAKE, false, 'Take ALL*'),
-            ],
-          ]),
-          h('small.text-left.ph2',
-            'Note: Release/Take all will only affect the detectors you have access to and detectors that are available.'
-          ),
+          detectorsService.isGlobalView() && [
+            h('.flex-row.g2.p2', [
+              isUserAllowedRole(ROLES.Admin) && [
+                h('strong', 'Admin actions: '),
+                detectorLockActionButton(
+                  lockModel, DETECTOR_ALL, {}, DetectorLockAction.RELEASE, true, 'Force Release ALL'
+                ),
+                detectorLockActionButton(
+                  lockModel, DETECTOR_ALL, {}, DetectorLockAction.TAKE, true, 'Force Take ALL'
+                ),
+              ],
+              isUserAllowedRole(ROLES.Global) && [
+                h('strong', 'Global actions: '),
+                detectorLockActionButton(
+                  lockModel, DETECTOR_ALL, {}, DetectorLockAction.RELEASE, false, 'Release ALL*'
+                ),
+                detectorLockActionButton(
+                  lockModel, DETECTOR_ALL, {}, DetectorLockAction.TAKE, false, 'Take ALL*'
+                ),
+              ],
+            ]),
+          ],
           detectorLocksTable(model, detectorsLocksState)
         ])
       })
@@ -86,18 +92,24 @@ export const content = (model) => {
  * @return {vnode}
  */
 const detectorLocksTable = (model, detectorLocksState) => {
-  const {detectors} = model;
+  const { detectors: detectorsService, lock: lockModel } = model;
   const isUserGlobal = isUserAllowedRole(ROLES.Global);
-
-  const detectorRows = Object.keys(detectorLocksState)
-    .filter((detector) => {
+  const detectorKeysWithTstLast = getDetectorListWithTstAtEnd(Object.keys(detectorLocksState));
+  const detectorRows = detectorKeysWithTstLast
+    .filter((detectorName) => {
       const isSelectedDetectorViewGlobalOrCurrent = (
-        detectors.selected === 'GLOBAL' || detectors.selected === detector
+        detectorsService.isGlobalView() || detectorsService.selected === detectorName
       );
-      const isUserAllowedDetector = detectors.authed.includes(detector);
+      const isUserAllowedDetector = detectorsService.authed.includes(detectorName);
       return (isUserGlobal && isSelectedDetectorViewGlobalOrCurrent) || isUserAllowedDetector;
     })
-    .map((detector) => detectorLockRow(model, detector, detectorLocksState[detector]))
+    .map((detectorName) => {
+      if (detectorName.toLocaleUpperCase().includes(TST_DETECTOR_NAME)) {
+        return [emptyRowSeparator(), detectorLockRow(lockModel, detectorName, detectorLocksState[detectorName])];
+      } else {
+        return detectorLockRow(lockModel, detectorName, detectorLocksState[detectorName])
+      }
+    });
   return h('table.table.table-sm',
     h('thead',
       h('tr',
@@ -121,26 +133,32 @@ const detectorLocksTable = (model, detectorLocksState) => {
 
 /**
  * Build a vnode for a row in the detector lock table which contains state of the lock and owner
- * @param {Model} model - root model of the application
+ * @param {LockModel} lockModel - model of the lock state and actions
  * @param {String} detector - detector name
  * @param {DetectorLock} lockState - state of the lock  {owner: {fullName: String}, isLocked: Boolean
  * @return {vnode}
  */
-const detectorLockRow = (model, detector, lockState) => {
+const detectorLockRow = (lockModel, detector, lockState) => {
   const ownerName = lockState?.owner?.fullName || '-';
   return h('tr', {
     id: `detector-row-${detector}`,
   }, [
     h('td',
       h('.flex-row.g2.items-center.f5', [
-        detectorLockButton(model, detector, lockState),
+        detectorLockButton(lockModel, detector, lockState),
         detector
       ])
     ),
     h('td', ownerName),
     isUserAllowedRole(ROLES.Global) && h('td', [
-      detectorLockActionButton(model.lock, detector, lockState, DetectorLockAction.RELEASE, true, 'Force Release'),
-      detectorLockActionButton(model.lock, detector, lockState, DetectorLockAction.TAKE, true, 'Force Take')
+      detectorLockActionButton(lockModel, detector, lockState, DetectorLockAction.RELEASE, true, 'Force Release'),
+      detectorLockActionButton(lockModel, detector, lockState, DetectorLockAction.TAKE, true, 'Force Take')
     ])
   ]);
 };
+
+/**
+ * Empty table row separator vnode
+ * @return {vnode}
+ */
+const emptyRowSeparator = () => h('tr', h('td', {colspan: 3}, h('hr')));

Control/public/services/DetectorService.js

@@ -76,6 +76,14 @@ export default class DetectorService extends Observable {
     return this._selected && this._selected !== 'GLOBAL';
   }
 
+  /**
+   * Checks if the user selected detector view is GLOBAL
+   * @returns {boolean}
+   */
+  isGlobalView() {
+    return this._selected === 'GLOBAL';
+  }
+
   /**
    * Update selection for detector view in LocalStorage
    * Format: {SELECTED: <string>}

Control/public/workflow/Workflow.js

@@ -32,7 +32,7 @@ export default class Workflow extends Observable {
   constructor(model) {
     super();
     this.model = model;
-    this.flpSelection = new FlpSelection(this);
+    this.flpSelection = new FlpSelection(this, model.lock);
     this.flpSelection.bubbleTo(this);
 
     this.form = new WorkflowForm();

Control/public/workflow/panels/flps/FlpSelection.js

@@ -21,8 +21,9 @@ export default class FlpSelection extends Observable {
   /**
    * Initialize FLPs Selection Component
    * @param {Object} workflow
+   * @param {Lock} lockModel - model of the lock state which will be used to automatically unselect detectors that are no longer locked by the current user
    */
-  constructor(workflow) {
+  constructor(workflow, lockModel) {
     super();
     this.loader = workflow.model.loader;
     this.workflow = workflow;
@@ -38,6 +39,9 @@ export default class FlpSelection extends Observable {
     this.unavailableDetectors = []; // detectors which are loaded from configuration but active in AliECS
     this.missingHosts = [];
     this.detectorViewConfigurationError = false;
+
+    // Observe lock changes to automatically unselect detectors when locks are released
+    lockModel.observe(() => this._handleLockChanges());
   }
 
   /**
@@ -111,13 +115,12 @@ export default class FlpSelection extends Observable {
   }
 
   /**
-   * Toggle selection of a detector. A detector can have one of the 3 states:
-   * * active
-   * * available
-   * * unavailable
+   * Toggle selection of a detector. A detector can have one of the states taken/released:
+   * This toggle will also notify the observer by default unless specified otherwise via shouldNotify
    * @param {String} name
+   * @param {boolean} shouldNotify - specify if the method should trigger a notification at the end of the process. 
    */
-  toggleDetectorSelection(name) {
+  toggleDetectorSelection(name, shouldNotify = true) {
     const indexUnavailable = this.unavailableDetectors.indexOf(name)
     if (indexUnavailable >= 0) {
       this.unavailableDetectors.splice(indexUnavailable, 1);
@@ -131,7 +134,9 @@ export default class FlpSelection extends Observable {
         this.setHostsForDetector(name, true);
       }
     }
-    this.notify();
+    if (shouldNotify) {
+      this.notify();
+    }
   }
 
   /**
@@ -162,15 +167,7 @@ export default class FlpSelection extends Observable {
   isDetectorActive(name) {
     return this.activeDetectors.isSuccess() && this.activeDetectors.payload.detectors.includes(name)
   }
-  /**
-   * Unselects given detector and its FLPs
-   * @param {string} name
-   */
-  unselectDetector(name) {
-    if (this.selectedDetectors.includes(name)) {
-      this.toggleDetectorSelection(name);
-    }
-  }
+
   /**
    * Toggle the selection of an FLP from the form host
    * The user can also use SHIFT key to select between 2 FLP machines, thus
@@ -303,4 +300,24 @@ export default class FlpSelection extends Observable {
     }
     this.notify();
   }
+
+  /**
+   * Handler for lock state changes so that it automatically unselects detectors that are no longer locked by the current user.
+   * This method will be called either by:
+   * * changes triggered by the user on the lock button of a detector panel/locks page
+   * * changes triggered by a different user and received via websocket subscription to lock changes
+   * In all cases, if a detector is no longer locked by the current user, it will be unselected from the workflow and its hosts will be removed from the form selection.
+   * @private
+   * @returns {void}
+   */
+  _handleLockChanges() {
+    this.selectedDetectors
+      .filter((detector) => !this.workflow.model.lock.isLockedByCurrentUser(detector))
+      .forEach((detector) => {
+        if (this.selectedDetectors.includes(detector)) {
+          this.toggleDetectorSelection(detector, false);
+        }
+      });
+    this.notify();
+  }
 }