diff options
author | Fatih Acet <acetfatih@gmail.com> | 2016-12-12 17:44:10 +0000 |
---|---|---|
committer | Fatih Acet <acetfatih@gmail.com> | 2016-12-12 17:44:10 +0000 |
commit | 407adb9e9529d4aa769dbd1fbb3932684e036c3a (patch) | |
tree | d365ae989e3ee58da711da1542877d358287d63a | |
parent | 3445136b9b0b8367b151170509fabe613389a50d (diff) | |
parent | 52e0c4ba916d2cbc9bdb0fa0782c6b705c03c5a6 (diff) | |
download | gitlab-ce-407adb9e9529d4aa769dbd1fbb3932684e036c3a.tar.gz |
Merge branch '25483-broken-tabs' into 'master'
Fix TypeError: Cannot read property 'initTabs'
## What does this MR do?
Adds a default value to the `options` argument in order to prevent errors when `Pipelines` is initialised without arguments.
Adds tests to guarantee this does not happen again.
Adds back removed pipeline class to make the pipeline graph visible, `js-pipeline-graph`
## Why was this MR needed?
In the places where `Pipelines` is initialised without arguments it throws an error: `TypeError: Cannot read property 'initTabs'`
## Does this MR meet the acceptance criteria?
- [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- Tests
- [x] Added for this feature/bug
- [ ] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Closes #25483
Closes #25493
See merge request !8009
9 files changed, 57 insertions, 10 deletions
diff --git a/app/assets/javascripts/pipelines.js.es6 b/app/assets/javascripts/pipelines.js.es6 index 72c6c4a1fcd..fb95648e1c7 100644 --- a/app/assets/javascripts/pipelines.js.es6 +++ b/app/assets/javascripts/pipelines.js.es6 @@ -4,7 +4,7 @@ ((global) => { class Pipelines { - constructor(options) { + constructor(options = {}) { if (options.initTabs && options.tabsOptions) { new global.LinkedTabs(options.tabsOptions); @@ -14,9 +14,11 @@ } addMarginToBuildColumns() { - this.pipelineGraph = document.querySelector('.pipeline-graph'); - const secondChildBuildNodes = document.querySelector('.pipeline-graph').querySelectorAll('.build:nth-child(2)'); - for (buildNodeIndex in secondChildBuildNodes) { + this.pipelineGraph = document.querySelector('.js-pipeline-graph'); + + const secondChildBuildNodes = this.pipelineGraph.querySelectorAll('.build:nth-child(2)'); + + for (const buildNodeIndex in secondChildBuildNodes) { const buildNode = secondChildBuildNodes[buildNodeIndex]; const firstChildBuildNode = buildNode.previousElementSibling; if (!firstChildBuildNode || !firstChildBuildNode.matches('.build')) continue; @@ -28,6 +30,7 @@ const columnBuilds = previousColumn.querySelectorAll('.build'); if (columnBuilds.length === 1) previousColumn.classList.add('no-margin'); } + this.pipelineGraph.classList.remove('hidden'); } } diff --git a/app/views/projects/ci/builds/_build_pipeline.html.haml b/app/views/projects/ci/builds/_build_pipeline.html.haml index 423a1282eb2..ad1a7360a8b 100644 --- a/app/views/projects/ci/builds/_build_pipeline.html.haml +++ b/app/views/projects/ci/builds/_build_pipeline.html.haml @@ -1,10 +1,10 @@ - is_playable = subject.playable? && can?(current_user, :update_build, @project) - if is_playable - = link_to play_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, data: { toggle: 'tooltip', title: "#{subject.name} - play", container: '.pipeline-graph', placement: 'bottom' } do + = link_to play_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, data: { toggle: 'tooltip', title: "#{subject.name} - play", container: '.js-pipeline-graph', placement: 'bottom' } do = ci_icon_for_status('play') .ci-status-text= subject.name - elsif can?(current_user, :read_build, @project) - = link_to namespace_project_build_path(subject.project.namespace, subject.project, subject), data: { toggle: 'tooltip', title: "#{subject.name} - #{subject.status}", container: '.pipeline-graph', placement: 'bottom' } do + = link_to namespace_project_build_path(subject.project.namespace, subject.project, subject), data: { toggle: 'tooltip', title: "#{subject.name} - #{subject.status}", container: '.js-pipeline-graph', placement: 'bottom' } do %span{class: "ci-status-icon ci-status-icon-#{subject.status}"} = ci_icon_for_status(subject.status) .ci-status-text= subject.name diff --git a/app/views/projects/commit/_pipeline.html.haml b/app/views/projects/commit/_pipeline.html.haml index c7b5c1124b3..08d3443b3d0 100644 --- a/app/views/projects/commit/_pipeline.html.haml +++ b/app/views/projects/commit/_pipeline.html.haml @@ -24,7 +24,7 @@ in = time_interval_in_words pipeline.duration - .row-content-block.build-content.middle-block.hidden + .row-content-block.build-content.middle-block.js-pipeline-graph.hidden = render "projects/pipelines/graph", pipeline: pipeline - if pipeline.yaml_errors.present? diff --git a/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml b/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml index 7b82d913d29..1bba0443154 100644 --- a/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml +++ b/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml @@ -1,4 +1,4 @@ -%a{ data: { toggle: 'tooltip', title: "#{subject.name} - #{subject.status}", container: '.pipeline-graph', placement: 'bottom' } } +%a{ data: { toggle: 'tooltip', title: "#{subject.name} - #{subject.status}", container: '.js-pipeline-graph', placement: 'bottom' } } - if subject.target_url = link_to subject.target_url do %span{class: "ci-status-icon ci-status-icon-#{subject.status}"} diff --git a/app/views/projects/pipelines/_with_tabs.html.haml b/app/views/projects/pipelines/_with_tabs.html.haml index 739e5930822..88af41aa835 100644 --- a/app/views/projects/pipelines/_with_tabs.html.haml +++ b/app/views/projects/pipelines/_with_tabs.html.haml @@ -12,7 +12,7 @@ .tab-content #js-tab-pipeline.tab-pane - .build-content.middle-block + .build-content.middle-block.js-pipeline-graph = render "projects/pipelines/graph", pipeline: pipeline #js-tab-builds.tab-pane diff --git a/changelogs/unreleased/25483-broken-tabs.yml b/changelogs/unreleased/25483-broken-tabs.yml new file mode 100644 index 00000000000..d6c92014bea --- /dev/null +++ b/changelogs/unreleased/25483-broken-tabs.yml @@ -0,0 +1,4 @@ +--- +title: Fix TypeError: Cannot read property 'initTabs' on commit builds tab +merge_request: 8009 +author: diff --git a/spec/javascripts/fixtures/pipeline_graph.html.haml b/spec/javascripts/fixtures/pipeline_graph.html.haml new file mode 100644 index 00000000000..deca50ceaa7 --- /dev/null +++ b/spec/javascripts/fixtures/pipeline_graph.html.haml @@ -0,0 +1,15 @@ +%div.pipeline-visualization.js-pipeline-graph + %ul.stage-column-list + %li.stage-column + .stage-name + %a{:href => "/"} + Test + .builds-container + %ul + %li.build + .curve + .build-content + %a + %svg + .ci-status-text + stop_review diff --git a/spec/javascripts/pipelines_spec.js.es6 b/spec/javascripts/pipelines_spec.js.es6 new file mode 100644 index 00000000000..85c9cf4b4f1 --- /dev/null +++ b/spec/javascripts/pipelines_spec.js.es6 @@ -0,0 +1,25 @@ +//= require pipelines + +(() => { + describe('Pipelines', () => { + fixture.preload('pipeline_graph'); + + beforeEach(() => { + fixture.load('pipeline_graph'); + }); + + it('should be defined', () => { + expect(window.gl.Pipelines).toBeDefined(); + }); + + it('should create a `Pipelines` instance without options', () => { + expect(() => { new window.gl.Pipelines(); }).not.toThrow(); //eslint-disable-line + }); + + it('should create a `Pipelines` instance with options', () => { + const pipelines = new window.gl.Pipelines({ foo: 'bar' }); + + expect(pipelines.pipelineGraph).toBeDefined(); + }); + }); +})(); diff --git a/spec/views/projects/pipelines/show.html.haml_spec.rb b/spec/views/projects/pipelines/show.html.haml_spec.rb index bf027499c94..a066ea078e6 100644 --- a/spec/views/projects/pipelines/show.html.haml_spec.rb +++ b/spec/views/projects/pipelines/show.html.haml_spec.rb @@ -28,7 +28,7 @@ describe 'projects/pipelines/show' do it 'shows a graph with grouped stages' do render - expect(rendered).to have_css('.pipeline-graph') + expect(rendered).to have_css('.js-pipeline-graph') expect(rendered).to have_css('.grouped-pipeline-dropdown') # stages |