From c68b85a9c2b6175225326ea962a1710d37b4c872 Mon Sep 17 00:00:00 2001 From: jasquat Date: Wed, 28 Dec 2022 12:27:37 -0500 Subject: [PATCH] fixed some cypress tests and fixed issue where an invalid date caused the page to constantly reload w/ burnettk --- flask-bpmn/src/flask_bpmn/api/api_error.py | 3 +- .../bin/spiffworkflow-realm.json | 2 +- .../routes/process_api_blueprint.py | 1 - .../cypress/e2e/process_instances.cy.js | 3 + .../cypress/e2e/tasks.cy.js | 2 +- .../components/ProcessInstanceListTable.tsx | 56 +++++++++---------- spiffworkflow-frontend/src/helpers.tsx | 15 +++++ .../src/routes/ProcessInstanceList.tsx | 2 + 8 files changed, 52 insertions(+), 32 deletions(-) diff --git a/flask-bpmn/src/flask_bpmn/api/api_error.py b/flask-bpmn/src/flask_bpmn/api/api_error.py index 8586f476..ed792e7e 100644 --- a/flask-bpmn/src/flask_bpmn/api/api_error.py +++ b/flask-bpmn/src/flask_bpmn/api/api_error.py @@ -176,7 +176,8 @@ def handle_exception(exception: Exception) -> flask.wrappers.Response: id = capture_exception(exception) if isinstance(exception, ApiError): - current_app.logger.info(f"Sending ApiError exception to sentry: {exception} with error code {exception.error_code}") + current_app.logger.info( + f"Sending ApiError exception to sentry: {exception} with error code {exception.error_code}") organization_slug = current_app.config.get("SENTRY_ORGANIZATION_SLUG") project_slug = current_app.config.get("SENTRY_PROJECT_SLUG") diff --git a/spiffworkflow-backend/bin/spiffworkflow-realm.json b/spiffworkflow-backend/bin/spiffworkflow-realm.json index 0df83605..e31942cf 100644 --- a/spiffworkflow-backend/bin/spiffworkflow-realm.json +++ b/spiffworkflow-backend/bin/spiffworkflow-realm.json @@ -2876,4 +2876,4 @@ "clientPolicies" : { "policies" : [ ] } -} \ No newline at end of file +} diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py index 374c90ac..ae784ba0 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py @@ -135,7 +135,6 @@ def permissions_check(body: Dict[str, Dict[str, list[str]]]) -> flask.wrappers.R status_code=400, ) ) - response_dict: dict[str, dict[str, bool]] = {} requests_to_check = body["requests_to_check"] diff --git a/spiffworkflow-frontend/cypress/e2e/process_instances.cy.js b/spiffworkflow-frontend/cypress/e2e/process_instances.cy.js index 55c1712d..e582dcbb 100644 --- a/spiffworkflow-frontend/cypress/e2e/process_instances.cy.js +++ b/spiffworkflow-frontend/cypress/e2e/process_instances.cy.js @@ -168,6 +168,8 @@ describe('process-instances', () => { it('can filter', () => { cy.getBySel('process-instance-list-link').click(); + cy.getBySel('process-instance-list-all').click(); + cy.contains('All Process Instances'); cy.assertAtLeastOneItemInPaginatedResults(); const statusSelect = '#process-instance-status-select'; @@ -175,6 +177,7 @@ describe('process-instances', () => { if (!['all', 'waiting'].includes(processStatus)) { cy.get(statusSelect).click(); cy.get(statusSelect).contains(processStatus).click(); + cy.get(statusSelect).click(); cy.getBySel('filter-button').click(); // FIXME: wait a little bit for the useEffects to be able to fully set processInstanceFilters cy.wait(1000); diff --git a/spiffworkflow-frontend/cypress/e2e/tasks.cy.js b/spiffworkflow-frontend/cypress/e2e/tasks.cy.js index 8d722e14..73e9ef3b 100644 --- a/spiffworkflow-frontend/cypress/e2e/tasks.cy.js +++ b/spiffworkflow-frontend/cypress/e2e/tasks.cy.js @@ -92,7 +92,7 @@ describe('tasks', () => { cy.contains('Tasks').should('exist'); // FIXME: this will probably need a better way to link to the proper form that we want - cy.contains('Complete Task').click(); + cy.contains('Go').click(); submitInputIntoFormField( 'get_user_generated_number_four', diff --git a/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx b/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx index d961627a..553524f4 100644 --- a/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx +++ b/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx @@ -40,6 +40,7 @@ import { getProcessModelFullIdentifierFromSearchParams, modifyProcessIdentifierForPathParam, refreshAtInterval, + setErrorMessageSafely, } from '../helpers'; import PaginationForTable from './PaginationForTable'; @@ -130,11 +131,11 @@ export default function ProcessInstanceListTable({ const [endFromTimeInvalid, setEndFromTimeInvalid] = useState(false); const [endToTimeInvalid, setEndToTimeInvalid] = useState(false); - const setErrorMessage = (useContext as any)(ErrorContext)[1]; + const [errorMessage, setErrorMessage] = (useContext as any)(ErrorContext); const processInstancePathPrefix = variant === 'all' - ? '/admin/process-instances' + ? '/admin/process-instances/all' : '/admin/process-instances/for-me'; const [processStatusAllOptions, setProcessStatusAllOptions] = useState( @@ -428,8 +429,11 @@ export default function ProcessInstanceListTable({ } }; - // TODO: after factoring this out page hangs when invalid date ranges and applying the filter - const calculateStartAndEndSeconds = () => { + // jasquat/burnettk - 2022-12-28 do not check the validity of the dates when rendering components to avoid the page being + // re-rendered while the user is still typing. NOTE that we also prevented rerendering + // with the use of the setErrorMessageSafely function. we are not sure why the context not + // changing still causes things to rerender when we call its setter without our extra check. + const calculateStartAndEndSeconds = (validate: boolean = true) => { const startFromSeconds = convertDateAndTimeStringsToSeconds( startFromDate, startFromTime || '00:00:00' @@ -447,29 +451,25 @@ export default function ProcessInstanceListTable({ endToTime || '00:00:00' ); let valid = true; - if (isTrueComparison(startFromSeconds, '>', startToSeconds)) { - setErrorMessage({ - message: '"Start date from" cannot be after "start date to"', - }); - valid = false; - } - if (isTrueComparison(endFromSeconds, '>', endToSeconds)) { - setErrorMessage({ - message: '"End date from" cannot be after "end date to"', - }); - valid = false; - } - if (isTrueComparison(startFromSeconds, '>', endFromSeconds)) { - setErrorMessage({ - message: '"Start date from" cannot be after "end date from"', - }); - valid = false; - } - if (isTrueComparison(startToSeconds, '>', endToSeconds)) { - setErrorMessage({ - message: '"Start date to" cannot be after "end date to"', - }); - valid = false; + + if (validate) { + let message = ''; + if (isTrueComparison(startFromSeconds, '>', startToSeconds)) { + message = '"Start date from" cannot be after "start date to"'; + } + if (isTrueComparison(endFromSeconds, '>', endToSeconds)) { + message = '"End date from" cannot be after "end date to"'; + } + if (isTrueComparison(startFromSeconds, '>', endFromSeconds)) { + message = '"Start date from" cannot be after "end date from"'; + } + if (isTrueComparison(startToSeconds, '>', endToSeconds)) { + message = '"Start date to" cannot be after "end date to"'; + } + if (message !== '') { + valid = false; + setErrorMessageSafely(message, errorMessage, setErrorMessage); + } } return { @@ -657,7 +657,7 @@ export default function ProcessInstanceListTable({ startToSeconds, endFromSeconds, endToSeconds, - } = calculateStartAndEndSeconds(); + } = calculateStartAndEndSeconds(false); if (!valid || !reportMetadata) { return null; diff --git a/spiffworkflow-frontend/src/helpers.tsx b/spiffworkflow-frontend/src/helpers.tsx index d0f8e6f1..d91f0543 100644 --- a/spiffworkflow-frontend/src/helpers.tsx +++ b/spiffworkflow-frontend/src/helpers.tsx @@ -8,6 +8,7 @@ import { DEFAULT_PER_PAGE, DEFAULT_PAGE, } from './components/PaginationForTable'; +import { ErrorForDisplay } from './interfaces'; // https://www.30secondsofcode.org/js/s/slugify export const slugifyString = (str: any) => { @@ -238,3 +239,17 @@ export const getBpmnProcessIdentifiers = (rootBpmnElement: any) => { childProcesses.push(rootBpmnElement.businessObject.id); return childProcesses; }; + +// Setting the error message state to the same string is still considered a change +// and re-renders the page so check the message first to avoid that. +export const setErrorMessageSafely = ( + newErrorMessageString: string, + oldErrorMessage: ErrorForDisplay, + errorMessageSetter: any +) => { + if (oldErrorMessage && oldErrorMessage.message === newErrorMessageString) { + return null; + } + errorMessageSetter({ message: newErrorMessageString }); + return null; +}; diff --git a/spiffworkflow-frontend/src/routes/ProcessInstanceList.tsx b/spiffworkflow-frontend/src/routes/ProcessInstanceList.tsx index 51c09981..a18f48c8 100644 --- a/spiffworkflow-frontend/src/routes/ProcessInstanceList.tsx +++ b/spiffworkflow-frontend/src/routes/ProcessInstanceList.tsx @@ -67,6 +67,7 @@ export default function ProcessInstanceList({ variant }: OwnProps) { { navigate('/admin/process-instances/for-me'); }} @@ -76,6 +77,7 @@ export default function ProcessInstanceList({ variant }: OwnProps) { { navigate('/admin/process-instances/all'); }}