Serialize and deserialize pipeline header data
Problem
We want to avoid the explicit string check of computeMinutes === '0.0' within the pipelineDetailsHeader(which was flagged during code review).
Proposal
We can make sure the data is serialized to json in ruby and then deserializing using js. This should also allow us to remove the conversations from strings to booleans as well.
Something roughly like this full diff should work:
diff --git a/app/assets/javascripts/ci/pipeline_details/header/pipeline_details_header.vue b/app/assets/javascripts/ci/pipeline_details/header/pipeline_details_header.vue
index 3a6a655bfa6c..78e7bbb9f64d 100644
--- a/app/assets/javascripts/ci/pipeline_details/header/pipeline_details_header.vue
+++ b/app/assets/javascripts/ci/pipeline_details/header/pipeline_details_header.vue
@@ -307,7 +307,7 @@ export default {
return cancelable && userPermissions.updatePipeline;
},
showComputeMinutes() {
- return this.isFinished && this.computeMinutes !== '0.0';
+ return this.isFinished && this.computeMinutes;
},
},
methods: {
diff --git a/app/assets/javascripts/ci/pipeline_details/pipeline_details_header.js b/app/assets/javascripts/ci/pipeline_details/pipeline_details_header.js
index 067ec3f305e4..1912769d0ba2 100644
--- a/app/assets/javascripts/ci/pipeline_details/pipeline_details_header.js
+++ b/app/assets/javascripts/ci/pipeline_details/pipeline_details_header.js
@@ -1,6 +1,5 @@
import Vue from 'vue';
import VueApollo from 'vue-apollo';
-import { parseBoolean } from '~/lib/utils/common_utils';
import PipelineDetailsHeader from './header/pipeline_details_header.vue';
Vue.use(VueApollo);
@@ -32,7 +31,7 @@ export const createPipelineDetailsHeaderApp = (elSelector, apolloProvider, graph
detached,
stuck,
refText,
- } = el.dataset;
+ } = JSON.parse(el.dataset.pipelineDetails);
// eslint-disable-next-line no-new
new Vue({
@@ -58,15 +57,15 @@ export const createPipelineDetailsHeaderApp = (elSelector, apolloProvider, graph
failureReason,
refText,
badges: {
- schedule: parseBoolean(schedule),
- child: parseBoolean(child),
- latest: parseBoolean(latest),
- mergeTrainPipeline: parseBoolean(mergeTrainPipeline),
- invalid: parseBoolean(invalid),
- failed: parseBoolean(failed),
- autoDevops: parseBoolean(autoDevops),
- detached: parseBoolean(detached),
- stuck: parseBoolean(stuck),
+ schedule,
+ child,
+ latest,
+ mergeTrainPipeline,
+ invalid,
+ failed,
+ autoDevops,
+ detached,
+ stuck,
},
},
});
diff --git a/app/views/projects/pipelines/show.html.haml b/app/views/projects/pipelines/show.html.haml
index 435edde319b0..f901cd0ce702 100644
--- a/app/views/projects/pipelines/show.html.haml
+++ b/app/views/projects/pipelines/show.html.haml
@@ -10,7 +10,7 @@
- add_page_startup_graphql_call('pipelines/get_pipeline_details', { projectPath: @project.full_path, iid: @pipeline.iid })
.js-pipeline-container{ data: { controller_action: "#{controller.action_name}" } }
- #js-pipeline-details-header-vue{ data: js_pipeline_details_header_data(@project, @pipeline) }
+ #js-pipeline-details-header-vue{ data: js_pipeline_details_header_data(@project, @pipeline).to_json }
= render_if_exists 'projects/pipelines/cc_validation_required_alert', pipeline: @pipeline
Hordur also has a small correction to that snippet here: #414844 (comment 1601238524)
Edited by Allison Browne