Move check for data to action

To make things a little cleaner, and the component
a little more lean, I moved the check for if we have
the full report/test suite data already to the fetch
test suite action. We have easier access to the state
there, and it makes more sense to be there.

app/assets/javascripts/pipelines/components/test_reports/test_reports.vue

@@ -41,9 +41,7 @@ export default {
       this.setSelectedSuiteIndex(index);
 
       // Fetch the test suite when the user clicks to see more details
-      if (!this.hasFullReport && !this.getSelectedSuite(index).hasFullSuite) {
-        this.fetchTestSuite(index);
-      }
+      this.fetchTestSuite(index);
     },
     beforeEnterTransition() {
       document.documentElement.style.overflowX = 'hidden';
@@ -71,7 +69,7 @@ export default {
       @after-leave="afterLeaveTransition"
     >
       <div v-if="showSuite" key="detail" class="w-100 position-absolute slide-enter-to-element">
-        <test-summary :report="getSelectedSuite()" show-back @on-back-click="summaryBackClick" />
+        <test-summary :report="getSelectedSuite" show-back @on-back-click="summaryBackClick" />
 
         <test-suite-table />
       </div>

app/assets/javascripts/pipelines/stores/test_reports/actions.js

@@ -34,6 +34,12 @@ export const fetchSummary = ({ state, commit, dispatch }) => {
 };
 
 export const fetchTestSuite = ({ state, commit, dispatch }, index) => {
+  const { hasFullSuite } = state.testReports?.test_suites?.[index] || {};
+  // We don't need to fetch the suite if we have the information already
+  if (state.hasFullReport || hasFullSuite) {
+    return false;
+  }
+
   dispatch('toggleLoading');
 
   const { name = '', build_ids = [] } = state.testReports?.test_suites?.[index] || {};

app/assets/javascripts/pipelines/stores/test_reports/getters.js

@@ -9,13 +9,10 @@ export const getTestSuites = state => {
   }));
 };
 
-// We want to use this to get the selected suite based on state
-// but we also want to use this when selecting a state
-// so we can make it return a method to call
-export const getSelectedSuite = state => (index = state.selectedSuiteIndex) =>
-  state.testReports?.test_suites?.[index] || {};
+export const getSelectedSuite = state =>
+  state.testReports?.test_suites?.[state.selectedSuiteIndex] || {};
 
 export const getSuiteTests = state => {
-  const { test_cases: testCases = [] } = getSelectedSuite(state)();
+  const { test_cases: testCases = [] } = getSelectedSuite(state);
   return testCases.sort(sortTestCases).map(addIconStatus);
 };

spec/frontend/pipelines/test_reports/stores/actions_spec.js

@@ -141,6 +141,24 @@ describe('Actions TestReports Store', () => {
         },
       );
     });
+
+    describe('when we already have the suite data', () => {
+      it('should not fetch suite', done => {
+        const index = 0;
+        testReports.test_suites[0].hasFullSuite = true;
+
+        testAction(actions.fetchTestSuite, index, { ...state, testReports }, [], [], done);
+      });
+    });
+
+    describe('when we already have the full report data', () => {
+      it('should not fetch suite', done => {
+        const index = 0;
+        testReports.hasFullReport = true;
+
+        testAction(actions.fetchTestSuite, index, { ...state, testReports }, [], [], done);
+      });
+    });
   });
 
   describe('fetch full report', () => {

spec/frontend/pipelines/test_reports/stores/getters_spec.js

@@ -51,20 +51,11 @@ describe('Getters TestReports Store', () => {
     it('should return the selected suite', () => {
       setupState();
 
-      const selectedSuite = getters.getSelectedSuite(state)();
+      const selectedSuite = getters.getSelectedSuite(state);
       const expected = testReports.test_suites[state.selectedSuiteIndex];
 
       expect(selectedSuite).toEqual(expected);
     });
-
-    it('should return the suite at the given index', () => {
-      setupState();
-
-      const selectedSuite = getters.getSelectedSuite(state)(0);
-      const expected = testReports.test_suites[0];
-
-      expect(selectedSuite).toEqual(expected);
-    });
   });
 
   describe('getSuiteTests', () => {

spec/frontend/pipelines/test_reports/test_reports_spec.js

@@ -91,46 +91,14 @@ describe('Test reports app', () => {
   });
 
   describe('when a suite is clicked', () => {
-    describe('when the full test report has already been received', () => {
-      beforeEach(() => {
-        createComponent({ hasFullReport: true });
-        testSummaryTable().vm.$emit('row-click', 0);
-      });
-
-      it('should only call setSelectedSuiteIndex', () => {
-        expect(actionSpies.setSelectedSuiteIndex).toHaveBeenCalled();
-        expect(actionSpies.fetchTestSuite).not.toHaveBeenCalled();
-      });
+    beforeEach(() => {
+      createComponent({ hasFullReport: true });
+      testSummaryTable().vm.$emit('row-click', 0);
     });
 
-    describe('when the full test report has not been received', () => {
-      describe('when the full suite has already been received', () => {
-        beforeEach(() => {
-          const mockState = { hasFullReport: false, testReports };
-          mockState.testReports.test_suites[0].hasFullSuite = true;
-          createComponent(mockState);
-          testSummaryTable().vm.$emit('row-click', 0);
-        });
-
-        it('should only call setSelectedSuiteIndex', () => {
-          expect(actionSpies.setSelectedSuiteIndex).toHaveBeenCalled();
-          expect(actionSpies.fetchTestSuite).not.toHaveBeenCalled();
-        });
-      });
-
-      describe('when the full suite has not been received', () => {
-        beforeEach(() => {
-          const mockState = { hasFullReport: false, testReports };
-          mockState.testReports.test_suites[0].hasFullSuite = false;
-          createComponent(mockState);
-          testSummaryTable().vm.$emit('row-click', 0);
-        });
-
-        it('should call setSelectedSuiteIndex and fetchTestSuite', () => {
-          expect(actionSpies.setSelectedSuiteIndex).toHaveBeenCalled();
-          expect(actionSpies.fetchTestSuite).toHaveBeenCalled();
-        });
-      });
+    it('should call setSelectedSuiteIndex and fetchTestSuite', () => {
+      expect(actionSpies.setSelectedSuiteIndex).toHaveBeenCalled();
+      expect(actionSpies.fetchTestSuite).toHaveBeenCalled();
     });
   });