From 5de4c8e78a96433482063a53dfce0056902da654 Mon Sep 17 00:00:00 2001 From: Lucas Di Pentima Date: Wed, 6 Apr 2022 18:08:27 -0300 Subject: [PATCH] 18881: Improves process filtering by status. Adds tests. Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima --- src/store/processes/process.ts | 44 ++++++++++--------- .../project-panel-middleware-service.ts | 36 ++++++--------- .../resource-type-filters.test.ts | 21 ++++++++- .../resource-type-filters.ts | 32 ++++++++------ 4 files changed, 76 insertions(+), 57 deletions(-) diff --git a/src/store/processes/process.ts b/src/store/processes/process.ts index 37cdd2b3..b72a0c2b 100644 --- a/src/store/processes/process.ts +++ b/src/store/processes/process.ts @@ -92,36 +92,40 @@ export const getProcessStatusColor = (status: string, { customs }: ArvadosTheme) export const getProcessStatus = ({ containerRequest, container }: Process): ProcessStatus => { switch (true) { + case containerRequest.state === ContainerRequestState.FINAL && + container?.state !== ContainerState.COMPLETE: + // Request was finalized before its container started (or the + // container was cancelled) + return ProcessStatus.CANCELLED; + case containerRequest.state === ContainerRequestState.UNCOMMITTED: return ProcessStatus.DRAFT; - case containerRequest.priority === 0: - return ProcessStatus.ONHOLD; - - case container && container.state === ContainerState.COMPLETE && container.exitCode === 0: - return ProcessStatus.COMPLETED; + case container?.state === ContainerState.COMPLETE: + if (container?.exitCode === 0) { + return ProcessStatus.COMPLETED; + } + return ProcessStatus.FAILED; - case container && container.state === ContainerState.CANCELLED: + case container?.state === ContainerState.CANCELLED: return ProcessStatus.CANCELLED; - case container && (container.state === ContainerState.QUEUED || - container.state === ContainerState.LOCKED): + case container?.state === ContainerState.QUEUED || + container?.state === ContainerState.LOCKED: + if (containerRequest.priority === 0) { + return ProcessStatus.ONHOLD; + } return ProcessStatus.QUEUED; - case container && container.state === ContainerState.RUNNING && - !!container.runtimeStatus.error: - return ProcessStatus.FAILING; - - case container && container.state === ContainerState.RUNNING && - !!container.runtimeStatus.warning: - return ProcessStatus.WARNING; - - case container && container.state === ContainerState.RUNNING: + case container?.state === ContainerState.RUNNING: + if (!!container?.runtimeStatus.error) { + return ProcessStatus.FAILING; + } + if (!!container?.runtimeStatus.warning) { + return ProcessStatus.WARNING; + } return ProcessStatus.RUNNING; - case container && container.state === ContainerState.COMPLETE && container.exitCode !== 0: - return ProcessStatus.FAILED; - default: return ProcessStatus.UNKNOWN; } diff --git a/src/store/project-panel/project-panel-middleware-service.ts b/src/store/project-panel/project-panel-middleware-service.ts index be569b49..ccfa4fff 100644 --- a/src/store/project-panel/project-panel-middleware-service.ts +++ b/src/store/project-panel/project-panel-middleware-service.ts @@ -17,7 +17,11 @@ import { OrderBuilder, OrderDirection } from "services/api/order-builder"; import { FilterBuilder, joinFilters } from "services/api/filter-builder"; import { GroupContentsResource, GroupContentsResourcePrefix } from "services/groups-service/groups-service"; import { updateFavorites } from "store/favorites/favorites-actions"; -import { IS_PROJECT_PANEL_TRASHED, projectPanelActions, getProjectPanelCurrentUuid } from 'store/project-panel/project-panel-action'; +import { + IS_PROJECT_PANEL_TRASHED, + projectPanelActions, + getProjectPanelCurrentUuid +} from 'store/project-panel/project-panel-action'; import { Dispatch, MiddlewareAPI } from "redux"; import { ProjectResource } from "models/project"; import { updateResources } from "store/resources/resources-actions"; @@ -29,7 +33,10 @@ import { ListResults } from 'services/common-service/common-service'; import { loadContainers } from 'store/processes/processes-actions'; import { ResourceKind } from 'models/resource'; import { getSortColumn } from "store/data-explorer/data-explorer-reducer"; -import { serializeResourceTypeFilters, ProcessStatusFilter } from 'store/resource-type-filters/resource-type-filters'; +import { + serializeResourceTypeFilters, + buildProcessStatusFilters +} from 'store/resource-type-filters/resource-type-filters'; import { updatePublicFavorites } from 'store/public-favorites/public-favorites-actions'; export class ProjectPanelMiddlewareService extends DataExplorerMiddlewareService { @@ -116,27 +123,10 @@ export const getFilters = (dataExplorer: DataExplorer) => { .getFilters(); // Filter by container status - const fb = new FilterBuilder(); - switch (activeStatusFilter) { - case ProcessStatusFilter.COMPLETED: { - fb.addEqual('container.state', 'Complete', GroupContentsResourcePrefix.PROCESS); - fb.addEqual('container.exit_code', '0', GroupContentsResourcePrefix.PROCESS); - break; - } - case ProcessStatusFilter.FAILED: { - fb.addEqual('container.state', 'Complete', GroupContentsResourcePrefix.PROCESS); - fb.addDistinct('container.exit_code', '0', GroupContentsResourcePrefix.PROCESS); - break; - } - case ProcessStatusFilter.CANCELLED: - case ProcessStatusFilter.LOCKED: - case ProcessStatusFilter.QUEUED: - case ProcessStatusFilter.RUNNING: { - fb.addEqual('container.state', activeStatusFilter, GroupContentsResourcePrefix.PROCESS); - break; - } - } - const statusFilters = fb.getFilters(); + const statusFilters = buildProcessStatusFilters( + new FilterBuilder(), + activeStatusFilter || '', + GroupContentsResourcePrefix.PROCESS).getFilters(); return joinFilters( statusFilters, diff --git a/src/store/resource-type-filters/resource-type-filters.test.ts b/src/store/resource-type-filters/resource-type-filters.test.ts index 71b00b2e..698515bd 100644 --- a/src/store/resource-type-filters/resource-type-filters.test.ts +++ b/src/store/resource-type-filters/resource-type-filters.test.ts @@ -2,10 +2,29 @@ // // SPDX-License-Identifier: AGPL-3.0 -import { getInitialResourceTypeFilters, serializeResourceTypeFilters, ObjectTypeFilter, CollectionTypeFilter, ProcessTypeFilter, GroupTypeFilter } from './resource-type-filters'; +import { getInitialResourceTypeFilters, serializeResourceTypeFilters, ObjectTypeFilter, CollectionTypeFilter, ProcessTypeFilter, GroupTypeFilter, buildProcessStatusFilters, ProcessStatusFilter } from './resource-type-filters'; import { ResourceKind } from 'models/resource'; import { deselectNode } from 'models/tree'; import { pipe } from 'lodash/fp'; +import { FilterBuilder } from 'services/api/filter-builder'; + +describe("buildProcessStatusFilters", () => { + [ + [ProcessStatusFilter.ALL, ""], + [ProcessStatusFilter.ONHOLD, `["state","!=","Final"],["priority","=","0"],["container.state","in",["Queued","Locked"]]`], + [ProcessStatusFilter.COMPLETED, `["container.state","=","Complete"],["container.exit_code","=","0"]`], + [ProcessStatusFilter.FAILED, `["container.state","=","Complete"],["container.exit_code","!=","0"]`], + [ProcessStatusFilter.QUEUED, `["container.state","=","Queued"],["priority","!=","0"]`], + [ProcessStatusFilter.CANCELLED, `["container.state","=","Cancelled"]`], + [ProcessStatusFilter.RUNNING, `["container.state","=","Running"]`], + ].forEach(([status, expected]) => { + it(`can filter "${status}" processes`, () => { + const filters = buildProcessStatusFilters(new FilterBuilder(), status); + expect(filters.getFilters()) + .toEqual(expected); + }) + }); +}); describe("serializeResourceTypeFilters", () => { it("should serialize all filters", () => { diff --git a/src/store/resource-type-filters/resource-type-filters.ts b/src/store/resource-type-filters/resource-type-filters.ts index e42a16d8..a39807d5 100644 --- a/src/store/resource-type-filters/resource-type-filters.ts +++ b/src/store/resource-type-filters/resource-type-filters.ts @@ -11,6 +11,7 @@ import { getSelectedNodes } from 'models/tree'; import { CollectionType } from 'models/collection'; import { GroupContentsResourcePrefix } from 'services/groups-service/groups-service'; import { ContainerState } from 'models/container'; +import { ContainerRequestState } from 'models/container-request'; export enum ProcessStatusFilter { ALL = 'All', @@ -18,7 +19,7 @@ export enum ProcessStatusFilter { FAILED = 'Failed', COMPLETED = 'Completed', CANCELLED = 'Cancelled', - LOCKED = 'Locked', + ONHOLD = 'On hold', QUEUED = 'Queued' } @@ -95,12 +96,12 @@ export const getInitialProcessStatusFilters = pipe( (): DataTableFilters => createTree(), pipe( initFilter(ProcessStatusFilter.ALL, '', true), + initFilter(ProcessStatusFilter.ONHOLD, '', false), + initFilter(ProcessStatusFilter.QUEUED, '', false), initFilter(ProcessStatusFilter.RUNNING, '', false), - initFilter(ProcessStatusFilter.FAILED, '', false), initFilter(ProcessStatusFilter.COMPLETED, '', false), initFilter(ProcessStatusFilter.CANCELLED, '', false), - initFilter(ProcessStatusFilter.QUEUED, '', false), - initFilter(ProcessStatusFilter.LOCKED, '', false), + initFilter(ProcessStatusFilter.FAILED, '', false), ), ); @@ -272,27 +273,32 @@ export const serializeSimpleObjectTypeFilters = (filters: Tree { +export const buildProcessStatusFilters = ( fb: FilterBuilder, activeStatusFilter: string, resourcePrefix?: string ): FilterBuilder => { switch (activeStatusFilter) { + case ProcessStatusFilter.ONHOLD: { + fb.addDistinct('state', ContainerRequestState.FINAL, resourcePrefix); + fb.addEqual('priority', '0', resourcePrefix); + fb.addIn('container.state', [ContainerState.QUEUED, ContainerState.LOCKED], resourcePrefix); + break; + } case ProcessStatusFilter.COMPLETED: { - fb.addEqual('container.state', ContainerState.COMPLETE); - fb.addEqual('container.exit_code', '0'); + fb.addEqual('container.state', ContainerState.COMPLETE, resourcePrefix); + fb.addEqual('container.exit_code', '0', resourcePrefix); break; } case ProcessStatusFilter.FAILED: { - fb.addEqual('container.state', ContainerState.COMPLETE); - fb.addDistinct('container.exit_code', '0'); + fb.addEqual('container.state', ContainerState.COMPLETE, resourcePrefix); + fb.addDistinct('container.exit_code', '0', resourcePrefix); break; } case ProcessStatusFilter.QUEUED: { - fb.addEqual('container.state', ContainerState.QUEUED); - fb.addDistinct('container.priority', '0'); + fb.addEqual('container.state', ContainerState.QUEUED, resourcePrefix); + fb.addDistinct('priority', '0', resourcePrefix); break; } case ProcessStatusFilter.CANCELLED: - case ProcessStatusFilter.LOCKED: case ProcessStatusFilter.RUNNING: { - fb.addEqual('container.state', activeStatusFilter); + fb.addEqual('container.state', activeStatusFilter, resourcePrefix); break; } } -- 2.30.2