From 4f12c0e620cf29dc7497aad920e854f4a9b125f5 Mon Sep 17 00:00:00 2001 From: Gavin Reynolds Date: Wed, 3 Jan 2024 11:31:14 +0000 Subject: [PATCH 1/5] Fix #380 by clearing incident checkboxes as actions are requested, rather than completed and potentially filtered out, to avoid react-table's toggleAllRowsSelected behaviour of only working on currently filtered rows Signed-off-by: Gavin Reynolds --- cypress/e2e/Incidents/incidents.spec.js | 3 +++ .../IncidentTable/IncidentTableComponent.jsx | 14 +++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/cypress/e2e/Incidents/incidents.spec.js b/cypress/e2e/Incidents/incidents.spec.js index aa0e78ed..3bcc3ec2 100644 --- a/cypress/e2e/Incidents/incidents.spec.js +++ b/cypress/e2e/Incidents/incidents.spec.js @@ -230,6 +230,9 @@ describe('Manage Open Incidents', { failFast: { enabled: true } }, () => { selectIncident(0); cy.get('#incident-action-resolve-button').click(); checkActionAlertsModalContent('have been resolved'); + // and now show all resolved status (#380) + cy.get('.query-status-resolved-button').check({ force: true }); + waitForIncidentTable(); }); it('Update priority of singular incidents', () => { diff --git a/src/components/IncidentTable/IncidentTableComponent.jsx b/src/components/IncidentTable/IncidentTableComponent.jsx index c98595d4..a37835c6 100644 --- a/src/components/IncidentTable/IncidentTableComponent.jsx +++ b/src/components/IncidentTable/IncidentTableComponent.jsx @@ -441,11 +441,15 @@ const IncidentTableComponent = () => { // Handle deselecting rows after incident action has completed useEffect(() => { // TODO: Get user feedback on this workflow - if (incidentActionsStatus === 'ACTION_COMPLETED') { - toggleAllRowsSelected(false); - } else if ( - !incidentActionsStatus.includes('TOGGLE') - && incidentActionsStatus.includes('COMPLETED') + // toggleAllRowsSelected only works on the currently filtered rows, therefore + // to fix #380 we can't wait for the incidentActionsStatus to be COMPLETED + // + // Workarounds using the stateReducer to clear selectedRowIds also don't appear to work on filtered out rows + // Therefore, clearing the checkbox in the UI before it is filtered out is the best we can do for now + if ( + incidentActionsStatus === 'ACTION_COMPLETED' + || (!incidentActionsStatus.includes('TOGGLE') + && (incidentActionsStatus.includes('REQUESTED') || incidentActionsStatus.includes('COMPLETED'))) ) { toggleAllRowsSelected(false); } From ce2227ac2f42d24c145a293f38c6ffab7b51820a Mon Sep 17 00:00:00 2001 From: Gavin Reynolds Date: Wed, 3 Jan 2024 16:06:15 +0000 Subject: [PATCH 2/5] Clarify if conditional for deselection after incident action Signed-off-by: Gavin Reynolds --- .../IncidentTable/IncidentTableComponent.jsx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/components/IncidentTable/IncidentTableComponent.jsx b/src/components/IncidentTable/IncidentTableComponent.jsx index a37835c6..8259792c 100644 --- a/src/components/IncidentTable/IncidentTableComponent.jsx +++ b/src/components/IncidentTable/IncidentTableComponent.jsx @@ -438,7 +438,7 @@ const IncidentTableComponent = () => { return () => {}; }, [selectedFlatRows]); - // Handle deselecting rows after incident action has completed + // Handle deselecting rows after incident action has been requested useEffect(() => { // TODO: Get user feedback on this workflow // toggleAllRowsSelected only works on the currently filtered rows, therefore @@ -446,21 +446,21 @@ const IncidentTableComponent = () => { // // Workarounds using the stateReducer to clear selectedRowIds also don't appear to work on filtered out rows // Therefore, clearing the checkbox in the UI before it is filtered out is the best we can do for now - if ( - incidentActionsStatus === 'ACTION_COMPLETED' - || (!incidentActionsStatus.includes('TOGGLE') - && (incidentActionsStatus.includes('REQUESTED') || incidentActionsStatus.includes('COMPLETED'))) - ) { + const isActionRequested = incidentActionsStatus === 'ACTION_REQUESTED'; + const isToggleAction = incidentActionsStatus.includes('TOGGLE'); + const isRequestedOrCompleted = incidentActionsStatus.includes('REQUESTED') || incidentActionsStatus.includes('COMPLETED'); + + if (isActionRequested || (!isToggleAction && isRequestedOrCompleted)) { toggleAllRowsSelected(false); } }, [incidentActionsStatus]); - // deselect rows after response play has completed + // deselect rows after response play has requested // not adding this to the above useEffect because I don't want to // take a chance of deselecting too many times useEffect(() => { // TODO: Get user feedback on this workflow - if (responsePlaysStatus === 'RUN_RESPONSE_PLAY_COMPLETED') { + if (responsePlaysStatus.includes('RUN_RESPONSE_PLAY_REQUESTED') || responsePlaysStatus.includes('RUN_RESPONSE_PLAY_COMPLETED')) { toggleAllRowsSelected(false); } }, [responsePlaysStatus]); From 26450a71ee212003c777edbec904c238457dcc2b Mon Sep 17 00:00:00 2001 From: Gavin Reynolds Date: Mon, 8 Jan 2024 12:54:46 +0000 Subject: [PATCH 3/5] use should instead of then, as should is automatically retried Signed-off-by: Gavin Reynolds --- cypress/e2e/Incidents/incidents.spec.js | 4 ++-- cypress/support/util/common.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cypress/e2e/Incidents/incidents.spec.js b/cypress/e2e/Incidents/incidents.spec.js index 3bcc3ec2..cb831ba5 100644 --- a/cypress/e2e/Incidents/incidents.spec.js +++ b/cypress/e2e/Incidents/incidents.spec.js @@ -49,7 +49,7 @@ describe('Manage Open Incidents', { failFast: { enabled: true } }, () => { it('Select all incidents', () => { selectAllIncidents(); - cy.get('.selected-incidents-badge').then(($el) => { + cy.get('.selected-incidents-badge').should(($el) => { const text = $el.text(); const incidentNumbers = text.split(' ')[0].split('/'); expect(incidentNumbers[0]).to.equal(incidentNumbers[1]); @@ -59,7 +59,7 @@ describe('Manage Open Incidents', { failFast: { enabled: true } }, () => { // Shift-select multiple incidents selectIncident(0); selectIncident(4, true); - cy.get('.selected-incidents-badge').then(($el) => { + cy.get('.selected-incidents-badge').should(($el) => { const text = $el.text(); const incidentNumbers = text.split(' ')[0].split('/'); expect(incidentNumbers[0]).to.equal('5'); diff --git a/cypress/support/util/common.js b/cypress/support/util/common.js index 0846b4c3..af58b0d1 100644 --- a/cypress/support/util/common.js +++ b/cypress/support/util/common.js @@ -49,7 +49,7 @@ export const selectAllIncidents = () => { }; export const checkNoIncidentsSelected = () => { - cy.get('.selected-incidents-badge').then(($el) => { + cy.get('.selected-incidents-badge').should(($el) => { const text = $el.text(); const incidentNumbers = text.split(' ')[0].split('/'); expect(incidentNumbers[0]).to.equal('0'); From 314b593ed6734fd4908b2422f54ede2b455cff9e Mon Sep 17 00:00:00 2001 From: Gavin Reynolds Date: Tue, 9 Jan 2024 13:40:52 +0000 Subject: [PATCH 4/5] eslint format Signed-off-by: Gavin Reynolds --- cypress/support/util/common.js | 5 ++++- src/components/IncidentTable/IncidentTableComponent.jsx | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/cypress/support/util/common.js b/cypress/support/util/common.js index af58b0d1..f8ca5d7d 100644 --- a/cypress/support/util/common.js +++ b/cypress/support/util/common.js @@ -29,7 +29,10 @@ export const waitForIncidentTable = () => { export const waitForAlerts = () => { // eslint-disable-next-line cypress/no-unnecessary-waiting cy.wait(3000); // Required for query debounce - cy.get('.selected-incidents-ctr', { timeout: 60000 }).should('not.include.text', 'Fetching Alerts'); + cy.get('.selected-incidents-ctr', { timeout: 60000 }).should( + 'not.include.text', + 'Fetching Alerts', + ); }; export const selectIncident = (incidentIdx = 0, shiftKey = false) => { diff --git a/src/components/IncidentTable/IncidentTableComponent.jsx b/src/components/IncidentTable/IncidentTableComponent.jsx index 8259792c..c19d4432 100644 --- a/src/components/IncidentTable/IncidentTableComponent.jsx +++ b/src/components/IncidentTable/IncidentTableComponent.jsx @@ -460,7 +460,10 @@ const IncidentTableComponent = () => { // take a chance of deselecting too many times useEffect(() => { // TODO: Get user feedback on this workflow - if (responsePlaysStatus.includes('RUN_RESPONSE_PLAY_REQUESTED') || responsePlaysStatus.includes('RUN_RESPONSE_PLAY_COMPLETED')) { + if ( + responsePlaysStatus.includes('RUN_RESPONSE_PLAY_REQUESTED') + || responsePlaysStatus.includes('RUN_RESPONSE_PLAY_COMPLETED') + ) { toggleAllRowsSelected(false); } }, [responsePlaysStatus]); From 4d464213f65badc3c8925ef8904af3f119feb4af Mon Sep 17 00:00:00 2001 From: Gavin Reynolds Date: Tue, 9 Jan 2024 13:53:34 +0000 Subject: [PATCH 5/5] Address flaky responder test Signed-off-by: Gavin Reynolds --- cypress/e2e/Incidents/incidents.spec.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cypress/e2e/Incidents/incidents.spec.js b/cypress/e2e/Incidents/incidents.spec.js index cb831ba5..14507255 100644 --- a/cypress/e2e/Incidents/incidents.spec.js +++ b/cypress/e2e/Incidents/incidents.spec.js @@ -174,7 +174,7 @@ describe('Manage Open Incidents', { failFast: { enabled: true } }, () => { checkActionAlertsModalContent('Requested additional response for incident(s)'); cy.get(`@selectedIncidentId_${incidentIdx}`).then((incidentId) => { checkIncidentCellContent(incidentId, 'Responders', 'UA'); - checkPopoverContent(incidentId, 'Responders', 'user_a1@example.com'); + checkPopoverContent(incidentId, 'Responders', 'User A'); checkIncidentCellContent(incidentId, 'Latest Log Entry Type', 'responder_request'); }); @@ -185,7 +185,7 @@ describe('Manage Open Incidents', { failFast: { enabled: true } }, () => { checkActionAlertsModalContent('Requested additional response for incident(s)'); cy.get(`@selectedIncidentId_${incidentIdx}`).then((incidentId) => { checkIncidentCellContent(incidentId, 'Responders', 'UA'); - checkPopoverContent(incidentId, 'Responders', 'user_a1@example.com'); + checkPopoverContent(incidentId, 'Responders', 'User A'); checkIncidentCellContent(incidentId, 'Latest Log Entry Type', 'responder_request'); }); }); @@ -199,7 +199,9 @@ describe('Manage Open Incidents', { failFast: { enabled: true } }, () => { checkActionAlertsModalContent('Requested additional response for incident(s)'); cy.get(`@selectedIncidentId_${incidentIdx}`).then((incidentId) => { checkIncidentCellContent(incidentId, 'Responders', 'UA'); - checkPopoverContent(incidentId, 'Responders', 'user_a1@example.com'); + checkIncidentCellContent(incidentId, 'Responders', 'UB'); + checkPopoverContent(incidentId, 'Responders', 'User A'); + checkPopoverContent(incidentId, 'Responders', 'User B'); checkIncidentCellContent(incidentId, 'Latest Log Entry Type', 'responder_request'); }); });