Merge branch 'nfriend-deduplicate-dora-flag-names' into 'master'

Deduplicate DORA chart visibility flag names

See merge request gitlab-org/gitlab!59291

app/assets/javascripts/projects/pipelines/charts/components/app.vue

@@ -14,11 +14,7 @@ export default {
       import('ee_component/projects/pipelines/charts/components/lead_time_charts.vue'),
   },
   inject: {
-    shouldRenderDeploymentFrequencyCharts: {
-      type: Boolean,
-      default: false,
-    },
-    shouldRenderLeadTimeCharts: {
+    shouldRenderDoraCharts: {
       type: Boolean,
       default: false,
     },
@@ -32,12 +28,8 @@ export default {
     charts() {
       const chartsToShow = ['pipelines'];
 
-      if (this.shouldRenderDeploymentFrequencyCharts) {
-        chartsToShow.push('deployments');
-      }
-
-      if (this.shouldRenderLeadTimeCharts) {
-        chartsToShow.push('lead-time');
+      if (this.shouldRenderDoraCharts) {
+        chartsToShow.push('deployments', 'lead-time');
       }
 
       return chartsToShow;
@@ -69,12 +61,14 @@ export default {
       <gl-tab :title="__('Pipelines')">
         <pipeline-charts />
       </gl-tab>
-      <gl-tab v-if="shouldRenderDeploymentFrequencyCharts" :title="__('Deployments')">
-        <deployment-frequency-charts />
-      </gl-tab>
-      <gl-tab v-if="shouldRenderLeadTimeCharts" :title="__('Lead Time')">
-        <lead-time-charts />
-      </gl-tab>
+      <template v-if="shouldRenderDoraCharts">
+        <gl-tab :title="__('Deployments')">
+          <deployment-frequency-charts />
+        </gl-tab>
+        <gl-tab :title="__('Lead Time')">
+          <lead-time-charts />
+        </gl-tab>
+      </template>
     </gl-tabs>
     <pipeline-charts v-else />
   </div>

app/assets/javascripts/projects/pipelines/charts/index.js

@@ -13,10 +13,7 @@ const apolloProvider = new VueApollo({
 const mountPipelineChartsApp = (el) => {
   const { projectPath } = el.dataset;
 
-  const shouldRenderDeploymentFrequencyCharts = parseBoolean(
-    el.dataset.shouldRenderDeploymentFrequencyCharts,
-  );
-  const shouldRenderLeadTimeCharts = parseBoolean(el.dataset.shouldRenderLeadTimeCharts);
+  const shouldRenderDoraCharts = parseBoolean(el.dataset.shouldRenderDoraCharts);
 
   return new Vue({
     el,
@@ -27,8 +24,7 @@ const mountPipelineChartsApp = (el) => {
     apolloProvider,
     provide: {
       projectPath,
-      shouldRenderDeploymentFrequencyCharts,
-      shouldRenderLeadTimeCharts,
+      shouldRenderDoraCharts,
     },
     render: (createElement) => createElement(ProjectPipelinesCharts, {}),
   });

app/helpers/graph_helper.rb

@@ -23,11 +23,7 @@ module GraphHelper
     ratio.to_i
   end
 
-  def should_render_deployment_frequency_charts
-    false
-  end
-
-  def should_render_lead_time_charts
+  def should_render_dora_charts
     false
   end
 end

app/views/projects/pipelines/charts.html.haml

 - page_title _('CI/CD Analytics')
 
 #js-project-pipelines-charts-app{ data: { project_path: @project.full_path,
-  should_render_deployment_frequency_charts: should_render_deployment_frequency_charts.to_s,
-  should_render_lead_time_charts: should_render_lead_time_charts.to_s } }
+  should_render_dora_charts: should_render_dora_charts.to_s } }

ee/app/helpers/ee/graph_helper.rb

@@ -4,15 +4,8 @@ module EE
   module GraphHelper
     extend ::Gitlab::Utils::Override
 
-    override :should_render_deployment_frequency_charts
-    def should_render_deployment_frequency_charts
-      return false unless @project.feature_available?(:dora4_analytics)
-
-      can?(current_user, :read_dora4_analytics, @project)
-    end
-
-    override :should_render_lead_time_charts
-    def should_render_lead_time_charts
+    override :should_render_dora_charts
+    def should_render_dora_charts
       return false unless @project.feature_available?(:dora4_analytics)
 
       can?(current_user, :read_dora4_analytics, @project)

ee/spec/helpers/ee/graph_helper_spec.rb

@@ -15,37 +15,13 @@ RSpec.describe EE::GraphHelper do
     allow(self).to receive(:can?).with(current_user, :read_dora4_analytics, project).and_return(is_user_authorized)
   end
 
-  describe '#should_render_deployment_frequency_charts' do
+  describe '#should_render_dora_charts' do
     shared_examples 'returns true' do
-      it { expect(should_render_deployment_frequency_charts).to be(true) }
+      it { expect(should_render_dora_charts).to be(true) }
     end
 
     shared_examples 'returns false' do
-      it { expect(should_render_deployment_frequency_charts).to be(false) }
-    end
-
-    it_behaves_like 'returns true'
-
-    context 'when the feature is not available' do
-      let(:is_feature_licensed) { false }
-
-      it_behaves_like 'returns false'
-    end
-
-    context 'when the user does not have permission' do
-      let(:is_user_authorized) { false }
-
-      it_behaves_like 'returns false'
-    end
-  end
-
-  describe '#should_render_lead_time_charts' do
-    shared_examples 'returns true' do
-      it { expect(should_render_lead_time_charts).to be(true) }
-    end
-
-    shared_examples 'returns false' do
-      it { expect(should_render_lead_time_charts).to be(false) }
+      it { expect(should_render_dora_charts).to be(false) }
     end
 
     it_behaves_like 'returns true'

spec/frontend/projects/pipelines/charts/components/app_spec.js

@@ -22,8 +22,7 @@ describe('ProjectsPipelinesChartsApp', () => {
         {},
         {
           provide: {
-            shouldRenderDeploymentFrequencyCharts: true,
-            shouldRenderLeadTimeCharts: true,
+            shouldRenderDoraCharts: true,
           },
           stubs: {
             DeploymentFrequencyCharts: DeploymentFrequencyChartsStub,
@@ -41,39 +40,35 @@ describe('ProjectsPipelinesChartsApp', () => {
 
   const findGlTabs = () => wrapper.find(GlTabs);
   const findAllGlTabs = () => wrapper.findAll(GlTab);
+  const findGlTabAtIndex = (index) => findAllGlTabs().at(index);
   const findLeadTimeCharts = () => wrapper.find(LeadTimeChartsStub);
   const findDeploymentFrequencyCharts = () => wrapper.find(DeploymentFrequencyChartsStub);
   const findPipelineCharts = () => wrapper.find(PipelineCharts);
 
-  const expectCorrectTabs = ({ pipelines, leadTime, deploymentFreqency }) => {
-    it('renders the expected tabs', () => {
-      expect(findGlTabs().exists()).toBe(true);
-
-      const allTabTitles = findAllGlTabs().wrappers.map((w) => w.attributes('title'));
+  describe('when all charts are available', () => {
+    beforeEach(() => {
+      createComponent();
+    });
 
-      if (pipelines) {
-        expect(allTabTitles).toContain('Pipelines');
-        expect(findPipelineCharts().exists()).toBe(true);
-      }
+    it('renders tabs', () => {
+      expect(findGlTabs().exists()).toBe(true);
 
-      if (deploymentFreqency) {
-        expect(allTabTitles).toContain('Deployments');
-        expect(findDeploymentFrequencyCharts().exists()).toBe(true);
-      }
+      expect(findGlTabAtIndex(0).attributes('title')).toBe('Pipelines');
+      expect(findGlTabAtIndex(1).attributes('title')).toBe('Deployments');
+      expect(findGlTabAtIndex(2).attributes('title')).toBe('Lead Time');
+    });
 
-      if (leadTime) {
-        expect(allTabTitles).toContain('Lead Time');
-        expect(findLeadTimeCharts().exists()).toBe(true);
-      }
+    it('renders the pipeline charts', () => {
+      expect(findPipelineCharts().exists()).toBe(true);
     });
-  };
 
-  describe('when all charts are available', () => {
-    beforeEach(() => {
-      createComponent();
+    it('renders the deployment frequency charts', () => {
+      expect(findDeploymentFrequencyCharts().exists()).toBe(true);
     });
 
-    expectCorrectTabs({ pipelines: true, deploymentFreqency: true, leadTime: true });
+    it('renders the lead time charts', () => {
+      expect(findLeadTimeCharts().exists()).toBe(true);
+    });
 
     it('sets the tab and url when a tab is clicked', async () => {
       let chartsPath;
@@ -131,7 +126,7 @@ describe('ProjectsPipelinesChartsApp', () => {
         expect(name).toBe('chart');
         return chart ? [chart] : [];
       });
-      createComponent({ provide: { shouldRenderDeploymentFrequencyCharts: true } });
+      createComponent();
       expect(findGlTabs().attributes('value')).toBe(tab);
     });
 
@@ -151,7 +146,7 @@ describe('ProjectsPipelinesChartsApp', () => {
         return [];
       });
 
-      createComponent({ provide: { shouldRenderDeploymentFrequencyCharts: true } });
+      createComponent();
 
       expect(findGlTabs().attributes('value')).toBe('0');
 
@@ -168,30 +163,9 @@ describe('ProjectsPipelinesChartsApp', () => {
     });
   });
 
-  describe('when shouldRenderDeploymentFrequencyCharts is false', () => {
-    beforeEach(() => {
-      createComponent({ provide: { shouldRenderDeploymentFrequencyCharts: false } });
-    });
-
-    expectCorrectTabs({ pipelines: true, deploymentFreqency: false, leadTime: true });
-  });
-
-  describe('when shouldRenderLeadTimeCharts is false', () => {
-    beforeEach(() => {
-      createComponent({ provide: { shouldRenderLeadTimeCharts: false } });
-    });
-
-    expectCorrectTabs({ pipelines: true, deploymentFreqency: true, leadTime: false });
-  });
-
-  describe('when shouldRenderDeploymentFrequencyCharts and shouldRenderLeadTimeCharts are false', () => {
+  describe('when the dora charts are not available', () => {
     beforeEach(() => {
-      createComponent({
-        provide: {
-          shouldRenderDeploymentFrequencyCharts: false,
-          shouldRenderLeadTimeCharts: false,
-        },
-      });
+      createComponent({ provide: { shouldRenderDoraCharts: false } });
     });
 
     it('does not render tabs', () => {

spec/helpers/graph_helper_spec.rb

@@ -16,7 +16,7 @@ RSpec.describe GraphHelper do
     end
   end
 
-  describe '#should_render_deployment_frequency_charts' do
+  describe '#should_render_dora_charts' do
     let(:project) { create(:project, :private) }
 
     before do
@@ -24,19 +24,7 @@ RSpec.describe GraphHelper do
     end
 
     it 'always returns false' do
-      expect(should_render_deployment_frequency_charts).to be(false)
-    end
-  end
-
-  describe '#should_render_lead_time_charts' do
-    let(:project) { create(:project, :private) }
-
-    before do
-      self.instance_variable_set(:@project, project)
-    end
-
-    it 'always returns false' do
-      expect(should_render_lead_time_charts).to be(false)
+      expect(should_render_dora_charts).to be(false)
     end
   end
 end