Store job expansion in redux on status page

Currently the status page will forget the individual expansion
selections when navigating away and back, or also occasionally
when components re-render in such a way that a queue item component
is replaced in the DOM.

To make the page more stable and intuitive, this stores the expansion
state for jobs in redux.  It is keyed by the stable queue id, so that
even as queue items move around, the expansion will be remembered.

It will be remembered when navigating around the web ui, but reloading
the page will cause it to be forgotten.

Currently any change to the redux store will also trigger a complete
re-render of the status page due to the mapStateToProps function in
the PipelineSummary component.  To make this more efficient, that
is transformed to use the useSelector hook so that components are
not re-rendered unless the content actually changes.

Note that this is not a complete solution: a further change is needed
in order to remember the pipeline expansion.  Currently in order to
see this change in action after navigating away, the containing
pipeline must be manually expanded again.

Change-Id: Ic5b72f124512c7e5c0bf45d76a46a44210811f30

web/src/actions/status.js

@@ -22,7 +22,10 @@ export const requestStatus = () => ({
   type: STATUS_FETCH_REQUEST
 })
 
-export const receiveStatus = json => ({
+// TODO: If we wanted to spend the CPU cycles to do it, we could check
+// here if any state.statusExpansion.expandedJobs are no longer
+// present in the queues and then remove them.
+const receiveStatus = (json) => ({
   type: STATUS_FETCH_SUCCESS,
   status: json,
   receivedAt: Date.now()

web/src/actions/statusExpansion.js

@@ -0,0 +1,44 @@
+// Copyright 2024 Acme Gating, LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may
+// not use this file except in compliance with the License. You may obtain
+// a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations
+// under the License.
+
+export const STATUSEXPANSION_EXPAND_JOBS = 'STATUSEXPANSION_EXPAND_JOBS'
+export const STATUSEXPANSION_COLLAPSE_JOBS = 'STATUSEXPANSION_COLLAPSE_JOBS'
+export const STATUSEXPANSION_CLEANUP_JOBS = 'STATUSEXPANSION_CLEANUP_JOBS'
+
+export const expandJobsAction = (key) => ({
+  type: STATUSEXPANSION_EXPAND_JOBS,
+  key: key,
+})
+
+export const collapseJobsAction = (key) => ({
+  type: STATUSEXPANSION_COLLAPSE_JOBS,
+  key: key,
+})
+
+export const cleanupJobsAction = (key) => ({
+  type: STATUSEXPANSION_CLEANUP_JOBS,
+  key: key,
+})
+
+export const expandJobs = (key) => (dispatch) => {
+  dispatch(expandJobsAction(key))
+}
+
+export const collapseJobs = (key) => (dispatch) => {
+  dispatch(collapseJobsAction(key))
+}
+
+export const cleanupJobs = (key) => (dispatch) => {
+  dispatch(cleanupJobsAction(key))
+}

web/src/containers/status/QueueItem.jsx

@@ -12,10 +12,10 @@
 // License for the specific language governing permissions and limitations
 // under the License.
 
-import React, { useState, useEffect } from 'react'
+import React, { useState } from 'react'
 import PropTypes from 'prop-types'
 import { useHistory, useLocation } from 'react-router-dom'
-import { connect, useDispatch } from 'react-redux'
+import { connect, useDispatch, useSelector } from 'react-redux'
 
 import {
   Button,
@@ -60,6 +60,7 @@ import { dequeue, dequeue_ref, promote } from '../../api'
 import { addDequeueError, addPromoteError } from '../../actions/adminActions'
 import { addNotification } from '../../actions/notifications'
 import { fetchStatusIfNeeded } from '../../actions/status'
+import { expandJobs, collapseJobs } from '../../actions/statusExpansion'
 
 function FilterDropdown({ item, pipeline }) {
   const [isFilterDropdownOpen, setIsFilterDropdownOpen] = useState(false)
@@ -326,20 +327,22 @@ function QueueItem({ item, pipeline, tenant, user, jobsExpanded }) {
   const [isAdminActionsOpen, setIsAdminActionsOpen] = useState(false)
   const [isDequeueModalOpen, setIsDequeueModalOpen] = useState(false)
   const [isPromoteModalOpen, setIsPromoteModalOpen] = useState(false)
-  const [isJobsExpanded, setIsJobsExpanded] = useState(jobsExpanded)
   const [isSkippedJobsExpanded, setIsSkippedJobsExpanded] = useState(false)
 
+  const expansionKey = item.id
+  const expandedJobs = useSelector(state => state.statusExpansion.expandedJobs[expansionKey])
+
   const skippedjobs = item.jobs.filter(j => getJobStrResult(j) === 'skipped')
   const jobs = item.jobs.filter(j => getJobStrResult(j) !== 'skipped')
-
-  useEffect(() => {
-    setIsJobsExpanded(jobsExpanded)
-  }, [jobsExpanded])
-
   const dispatch = useDispatch()
+  const isJobsExpanded = expandedJobs === undefined ? jobsExpanded : expandedJobs
 
   const onJobsToggle = (isExpanded) => {
-    setIsJobsExpanded(isExpanded)
+    if (isExpanded) {
+      dispatch(expandJobs(expansionKey))
+    } else {
+      dispatch(collapseJobs(expansionKey))
+    }
   }
 
   const onSkippedJobsToggle = () => {

web/src/pages/PipelineOverview.jsx

@@ -12,9 +12,9 @@
 // License for the specific language governing permissions and limitations
 // under the License.
 
-import React, { useCallback, useEffect, useState } from 'react'
+import React, { useCallback, useEffect, useState, useMemo } from 'react'
 
-import { connect } from 'react-redux'
+import { useSelector, useDispatch } from 'react-redux'
 import { withRouter, useLocation, useHistory } from 'react-router-dom'
 import PropTypes from 'prop-types'
 import * as moment_tz from 'moment-timezone'
@@ -162,9 +162,33 @@ PipelineGallery.propTypes = {
   onClearFilters: PropTypes.func,
 }
 
-function PipelineOverviewPage({
-  pipelines, stats, isFetching, tenant, darkMode, autoReload, timezone, fetchStatusIfNeeded
-}) {
+function getPipelines(status, location) {
+  let pipelines = []
+  let stats = {}
+  if (status) {
+    const filters = getFiltersFromUrl(location, filterCategories)
+    // we need to work on a copy of the state..pipelines, because when mutating
+    // the original, we couldn't reset or change the filters without reloading
+    // from the backend first.
+    pipelines = global.structuredClone(status.pipelines)
+    pipelines = filterPipelines(pipelines, filters, filterCategories, true)
+
+    pipelines = pipelines.map(ppl => (
+      countPipelineItems(ppl)
+    ))
+    stats = {
+      trigger_event_queue: status.trigger_event_queue,
+      management_event_queue: status.management_event_queue,
+      last_reconfigured: status.last_reconfigured,
+    }
+  }
+  return {
+    pipelines,
+    stats,
+  }
+}
+
+function PipelineOverviewPage() {
   const [showAllPipelines, setShowAllPipelines] = useState(
     localStorage.getItem('zuul_show_all_pipelines') === 'true')
   const [expandAll, setExpandAll] = useState(
@@ -178,6 +202,16 @@ function PipelineOverviewPage({
 
   const isDocumentVisible = useDocumentVisibility()
 
+  const status = useSelector((state) => state.status.status)
+  const {pipelines, stats} = useMemo(() => getPipelines(status, location), [status, location])
+
+  const isFetching = useSelector((state) => state.status.isFetching)
+  const tenant = useSelector((state) => state.tenant)
+  const darkMode = useSelector((state) => state.preferences.darkMode)
+  const autoReload = useSelector((state) => state.preferences.autoReload)
+  const timezone = useSelector((state) => state.timezone)
+  const dispatch = useDispatch()
+
   const onShowAllPipelinesToggle = (isChecked) => {
     setShowAllPipelines(isChecked)
     localStorage.setItem('zuul_show_all_pipelines', isChecked.toString())
@@ -204,12 +238,12 @@ function PipelineOverviewPage({
   const updateData = useCallback((tenant) => {
     if (tenant.name) {
       setIsReloading(true)
-      fetchStatusIfNeeded(tenant)
+      dispatch(fetchStatusIfNeeded(tenant))
         .then(() => {
           setIsReloading(false)
         })
     }
-  }, [setIsReloading, fetchStatusIfNeeded])
+  }, [setIsReloading, dispatch])
 
   useEffect(() => {
     document.title = 'Zuul Status'
@@ -289,60 +323,6 @@ function PipelineOverviewPage({
       </PageSection>
     </>
   )
-
 }
 
-PipelineOverviewPage.propTypes = {
-  pipelines: PropTypes.array,
-  stats: PropTypes.object,
-  isFetching: PropTypes.bool,
-  tenant: PropTypes.object,
-  preferences: PropTypes.object,
-  darkMode: PropTypes.bool,
-  autoReload: PropTypes.bool.isRequired,
-  timezone: PropTypes.string,
-  fetchStatusIfNeeded: PropTypes.func,
-}
-
-function mapStateToProps(state, ownProps) {
-  let pipelines = []
-  let stats = {}
-  if (state.status.status) {
-    const filters = getFiltersFromUrl(ownProps.location, filterCategories)
-    // we need to work on a copy of the state..pipelines, because when mutating
-    // the original, we couldn't reset or change the filters without reloading
-    // from the backend first.
-    pipelines = global.structuredClone(state.status.status.pipelines)
-    pipelines = filterPipelines(pipelines, filters, filterCategories, true)
-
-    pipelines = pipelines.map(ppl => (
-      countPipelineItems(ppl)
-    ))
-    stats = {
-      trigger_event_queue: state.status.status.trigger_event_queue,
-      management_event_queue: state.status.status.management_event_queue,
-      last_reconfigured: state.status.status.last_reconfigured,
-    }
-  }
-
-  // TODO (felix): Here we could also order the pipelines by any
-  // criteria (e.g. the pipeline_type) in case we want that. Currently
-  // they are ordered in the way they are defined in the zuul config.
-  // The sorting could also be done via the filter toolbar.
-  return {
-    pipelines,
-    stats,
-    isFetching: state.status.isFetching,
-    tenant: state.tenant,
-    darkMode: state.preferences.darkMode,
-    autoReload: state.preferences.autoReload,
-    timezone: state.timezone,
-  }
-}
-
-const mapDispatchToProps = { fetchStatusIfNeeded }
-
-export default connect(
-  mapStateToProps,
-  mapDispatchToProps,
-)(withRouter(PipelineOverviewPage))
+export default withRouter(PipelineOverviewPage)

web/src/reducers/index.js

@@ -35,6 +35,7 @@ import projects from './projects'
 import preferences from './preferences'
 import semaphores from './semaphores'
 import status from './status'
+import statusExpansion from './statusExpansion'
 import tenant from './tenant'
 import tenants from './tenants'
 import tenantStatus from './tenantStatus'
@@ -63,6 +64,7 @@ const reducers = {
   projects,
   semaphores,
   status,
+  statusExpansion,
   tenant,
   tenants,
   timezone,

web/src/reducers/statusExpansion.js

@@ -0,0 +1,45 @@
+// Copyright 2024 Acme Gating, LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may
+// not use this file except in compliance with the License. You may obtain
+// a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations
+// under the License.
+
+import {
+  STATUSEXPANSION_EXPAND_JOBS,
+  STATUSEXPANSION_COLLAPSE_JOBS,
+  STATUSEXPANSION_CLEANUP_JOBS,
+} from '../actions/statusExpansion'
+
+export default (state = {
+  expandedJobs: {},
+}, action) => {
+  switch (action.type) {
+    case STATUSEXPANSION_EXPAND_JOBS:
+      return {
+        ...state,
+        expandedJobs: {...state.expandedJobs, [action.key]: true}
+      }
+    case STATUSEXPANSION_COLLAPSE_JOBS:
+      return {
+        ...state,
+        expandedJobs: {...state.expandedJobs, [action.key]: false}
+      }
+    case STATUSEXPANSION_CLEANUP_JOBS:
+      // eslint-disable-next-line
+      const {[action.key]:unused, ...newJobs } = state.expandedJobs
+      return {
+        ...state,
+        expandedJobs: newJobs,
+      }
+    default:
+      return state
+  }
+}