diff options
-rw-r--r-- | Gemfile | 2 | ||||
-rw-r--r-- | Gemfile.lock | 12 | ||||
-rw-r--r-- | app/assets/javascripts/merge_request.js.coffee | 79 | ||||
-rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 17 | ||||
-rw-r--r-- | app/views/projects/merge_requests/_new_submit.html.haml | 6 | ||||
-rw-r--r-- | app/views/projects/merge_requests/_show.html.haml | 6 | ||||
-rw-r--r-- | config/routes.rb | 1 | ||||
-rw-r--r-- | spec/routing/project_routing_spec.rb | 32 |
8 files changed, 96 insertions, 59 deletions
@@ -186,7 +186,7 @@ gem 'charlock_holmes' gem "sass-rails", '~> 4.0.2' gem "coffee-rails" gem "uglifier" -gem 'turbolinks' +gem 'turbolinks', '~> 2.5.0' gem 'jquery-turbolinks' gem 'select2-rails' diff --git a/Gemfile.lock b/Gemfile.lock index 7dbc3b4ffa9..f479ca7ed53 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -101,13 +101,13 @@ GEM coderay (1.1.0) coercible (1.0.0) descendants_tracker (~> 0.0.1) - coffee-rails (4.0.1) + coffee-rails (4.1.0) coffee-script (>= 2.2.0) railties (>= 4.0.0, < 5.0) - coffee-script (2.2.0) + coffee-script (2.4.1) coffee-script-source execjs - coffee-script-source (1.6.3) + coffee-script-source (1.9.1.1) colored (1.2) colorize (0.5.8) columnize (0.9.0) @@ -418,7 +418,7 @@ GEM quiet_assets (1.0.2) railties (>= 3.1, < 5.0) racc (1.4.10) - rack (1.5.2) + rack (1.5.3) rack-accept (0.4.5) rack (>= 0.4) rack-attack (4.3.0) @@ -633,7 +633,7 @@ GEM multi_json (~> 1.7) twitter-stream (~> 0.1) tins (0.13.1) - turbolinks (2.0.0) + turbolinks (2.5.3) coffee-rails twitter-stream (0.1.16) eventmachine (>= 0.12.8) @@ -806,7 +806,7 @@ DEPENDENCIES test_after_commit thin tinder (~> 1.9.2) - turbolinks + turbolinks (~> 2.5.0) uglifier underscore-rails (~> 1.4.4) unf diff --git a/app/assets/javascripts/merge_request.js.coffee b/app/assets/javascripts/merge_request.js.coffee index 3937c428e24..a5a2caf1263 100644 --- a/app/assets/javascripts/merge_request.js.coffee +++ b/app/assets/javascripts/merge_request.js.coffee @@ -26,7 +26,7 @@ class @MergeRequest @commits_loaded = @opts.commits_loaded or false this.bindEvents() - this.activateTabFromHash() + this.activateTabFromPath() this.initMergeWidget() this.$('.show-all-commits').on 'click', => @@ -82,19 +82,16 @@ class @MergeRequest bindEvents: -> this.$('.merge-request-tabs a[data-toggle="tab"]').on 'shown.bs.tab', (e) => $target = $(e.target) - - # Nothing else to be done if we're on the first tab - return if $target.data('action') == 'notes' - - # Persist current tab selection via URL - href = $target.attr('href') - if href.substr(0,1) == '#' - location.replace("#!#{href.substr(1)}") + tab_action = $target.data('action') # Lazy-load diffs - if $target.data('action') == 'diffs' + if tab_action == 'diffs' this.loadDiff() unless @diffs_loaded - $('.diff-header').trigger("sticky_kit:recalc") + $('.diff-header').trigger('sticky_kit:recalc') + + # Skip tab-persisting behavior on MergeRequests#new + unless @opts.action == 'new' + @setCurrentAction(tab_action) this.$('.accept_merge_request').on 'click', -> $('.automerge_widget.can_be_merged').hide() @@ -112,27 +109,51 @@ class @MergeRequest this.$('.remove_source_branch_in_progress').hide() this.$('.remove_source_branch_widget.failed').show() - # Activates a tab section based on the `#!` URL hash + # Activate a tab based on the current URL path # - # If no hash value is present (i.e., on the initial page load), the first tab - # is selected by default. + # If the current action is 'show' or 'new' (i.e., initial page load), + # activates the first tab, otherwise activates the tab corresponding to the + # current action (diffs, commits). + activateTabFromPath: -> + if @opts.action == 'show' || @opts.action == 'new' + this.$('.merge-request-tabs a[data-toggle="tab"]:first').tab('show') + else + this.$(".merge-request-tabs a[data-action='#{@opts.action}']").tab('show') + + # Replaces the current Merge Request-specific action in the URL with a new one # - # ... unless the current controller action is `diffs`, in which case that tab - # is selected instead. Fun, right? + # If the action is "notes", the URL is reset to the standard + # `MergeRequests#show` route. # - # Note: We use a `#!` instead of a standard URL hash for two reasons: + # Examples: # - # 1. Prevents the hash acting like an anchor and scrolling the page. - # 2. Prevents mutating browser history. - activateTabFromHash: -> - # Correct the hash if we came here directly via the `/diffs` path - if location.hash == '' and @opts.action == 'diffs' - location.replace('#!diffs') - - if location.hash == '' - this.$('.merge-request-tabs a[data-toggle="tab"]:first').tab('show') - else if location.hash.substr(0,2) == '#!' - this.$(".merge-request-tabs a[href='##{location.hash.substr(2)}']").tab("show") + # location.pathname # => "/namespace/project/merge_requests/1" + # setCurrentAction('diffs') + # location.pathname # => "/namespace/project/merge_requests/1/diffs" + # + # location.pathname # => "/namespace/project/merge_requests/1/diffs" + # setCurrentAction('notes') + # location.pathname # => "/namespace/project/merge_requests/1" + # + # location.pathname # => "/namespace/project/merge_requests/1/diffs" + # setCurrentAction('commits') + # location.pathname # => "/namespace/project/merge_requests/1/commits" + setCurrentAction: (action) -> + # Normalize action, just to be safe + action = 'notes' if action == 'show' + + # Remove a trailing '/commits' or '/diffs' + new_state = location.pathname.replace(/\/(commits|diffs)\/?$/, '') + + # Append the new action if we're on a tab other than 'notes' + unless action == 'notes' + new_state += "/#{action}" + + # Replace the current history state with the new one without breaking + # Turbolinks' history. + # + # See https://github.com/rails/turbolinks/issues/363 + history.replaceState {turbolinks: true, url: new_state}, '', new_state showState: (state) -> $('.automerge_widget').hide() @@ -161,7 +182,7 @@ class @MergeRequest loadDiff: (event) -> $.ajax type: 'GET' - url: this.$('.merge-request-tabs .diffs-tab a').data('source') + ".json" + url: this.$('.merge-request-tabs .diffs-tab a').attr('href') + ".json" beforeSend: => this.$('.mr-loading-status .loading').show() complete: => diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index c7467e9b2f5..71d3051ab88 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -2,10 +2,13 @@ require 'gitlab/satellite/satellite' class Projects::MergeRequestsController < Projects::ApplicationController before_action :module_enabled - before_action :merge_request, only: [:edit, :update, :show, :diffs, :automerge, :automerge_check, :ci_status, :toggle_subscription] - before_action :closes_issues, only: [:edit, :update, :show, :diffs] - before_action :validates_merge_request, only: [:show, :diffs] - before_action :define_show_vars, only: [:show, :diffs] + before_action :merge_request, only: [ + :edit, :update, :show, :diffs, :commits, :automerge, :automerge_check, + :ci_status, :toggle_subscription + ] + before_action :closes_issues, only: [:edit, :update, :show, :diffs, :commits] + before_action :validates_merge_request, only: [:show, :diffs, :commits] + before_action :define_show_vars, only: [:show, :diffs, :commits] # Allow read any merge_request before_action :authorize_read_merge_request! @@ -27,7 +30,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_requests = @merge_requests.full_search(terms) end end - + @merge_requests = @merge_requests.page(params[:page]).per(PER_PAGE) respond_to do |format| @@ -67,6 +70,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController end end + def commits + render 'show' + end + def new params[:merge_request] ||= ActionController::Parameters.new(source_project: @project) @merge_request = MergeRequests::BuildService.new(project, current_user, merge_request_params).execute diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml index e83b7649928..9a2edbf0a8c 100644 --- a/app/views/projects/merge_requests/_new_submit.html.haml +++ b/app/views/projects/merge_requests/_new_submit.html.haml @@ -20,12 +20,12 @@ .mr-compare.merge-request %ul.nav.nav-tabs.merge-request-tabs %li.commits-tab - = link_to '#commits', data: {action: 'commits', toggle: 'tab'} do + = link_to url_for(params), data: {target: '#commits', action: 'commits', toggle: 'tab'} do = icon('history') Commits %span.badge= @commits.size %li.diffs-tab - = link_to '#diffs', data: {action: 'diffs', toggle: 'tab'} do + = link_to url_for(params), data: {target: '#diffs', action: 'diffs', toggle: 'tab'} do = icon('list-alt') Changes %span.badge= @diffs.size @@ -56,7 +56,7 @@ :javascript var merge_request merge_request = new MergeRequest({ - action: 'diffs', + action: 'new', diffs_loaded: true, commits_loaded: true }); diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 0d894e360ea..bf056462b70 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -38,17 +38,17 @@ - if @commits.present? %ul.nav.nav-tabs.merge-request-tabs %li.notes-tab - = link_to '#notes', data: {action: 'notes', toggle: 'tab'} do + = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: {target: '#notes', action: 'notes', toggle: 'tab'} do = icon('comments') Discussion %span.badge= @merge_request.mr_and_commit_notes.user.count %li.commits-tab - = link_to '#commits', data: {action: 'commits', toggle: 'tab'} do + = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: {target: '#commits', action: 'commits', toggle: 'tab'} do = icon('history') Commits %span.badge= @commits.size %li.diffs-tab - = link_to '#diffs', data: {source: diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), action: 'diffs', toggle: 'tab'} do + = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: {target: '#diffs', action: 'diffs', toggle: 'tab'} do = icon('list-alt') Changes %span.badge= @merge_request.diffs.size diff --git a/config/routes.rb b/config/routes.rb index bf2cb6421c5..3f8f920963d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -450,6 +450,7 @@ Gitlab::Application.routes.draw do resources :merge_requests, constraints: { id: /\d+/ }, except: [:destroy] do member do get :diffs + get :commits post :automerge get :automerge_check get :ci_status diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 042352311da..3a0d9b88d75 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -208,23 +208,31 @@ describe Projects::RefsController, 'routing' do end end -# diffs_project_merge_request GET /:project_id/merge_requests/:id/diffs(.:format) projects/merge_requests#diffs -# automerge_project_merge_request POST /:project_id/merge_requests/:id/automerge(.:format) projects/merge_requests#automerge -# automerge_check_project_merge_request GET /:project_id/merge_requests/:id/automerge_check(.:format) projects/merge_requests#automerge_check -# branch_from_project_merge_requests GET /:project_id/merge_requests/branch_from(.:format) projects/merge_requests#branch_from -# branch_to_project_merge_requests GET /:project_id/merge_requests/branch_to(.:format) projects/merge_requests#branch_to -# project_merge_requests GET /:project_id/merge_requests(.:format) projects/merge_requests#index -# POST /:project_id/merge_requests(.:format) projects/merge_requests#create -# new_project_merge_request GET /:project_id/merge_requests/new(.:format) projects/merge_requests#new -# edit_project_merge_request GET /:project_id/merge_requests/:id/edit(.:format) projects/merge_requests#edit -# project_merge_request GET /:project_id/merge_requests/:id(.:format) projects/merge_requests#show -# PUT /:project_id/merge_requests/:id(.:format) projects/merge_requests#update -# DELETE /:project_id/merge_requests/:id(.:format) projects/merge_requests#destroy +# diffs_namespace_project_merge_request GET /:namespace_id/:project_id/merge_requests/:id/diffs(.:format) projects/merge_requests#diffs +# commits_namespace_project_merge_request GET /:namespace_id/:project_id/merge_requests/:id/commits(.:format) projects/merge_requests#commits +# automerge_namespace_project_merge_request POST /:namespace_id/:project_id/merge_requests/:id/automerge(.:format) projects/merge_requests#automerge +# automerge_check_namespace_project_merge_request GET /:namespace_id/:project_id/merge_requests/:id/automerge_check(.:format) projects/merge_requests#automerge_check +# ci_status_namespace_project_merge_request GET /:namespace_id/:project_id/merge_requests/:id/ci_status(.:format) projects/merge_requests#ci_status +# toggle_subscription_namespace_project_merge_request POST /:namespace_id/:project_id/merge_requests/:id/toggle_subscription(.:format) projects/merge_requests#toggle_subscription +# branch_from_namespace_project_merge_requests GET /:namespace_id/:project_id/merge_requests/branch_from(.:format) projects/merge_requests#branch_from +# branch_to_namespace_project_merge_requests GET /:namespace_id/:project_id/merge_requests/branch_to(.:format) projects/merge_requests#branch_to +# update_branches_namespace_project_merge_requests GET /:namespace_id/:project_id/merge_requests/update_branches(.:format) projects/merge_requests#update_branches +# namespace_project_merge_requests GET /:namespace_id/:project_id/merge_requests(.:format) projects/merge_requests#index +# POST /:namespace_id/:project_id/merge_requests(.:format) projects/merge_requests#create +# new_namespace_project_merge_request GET /:namespace_id/:project_id/merge_requests/new(.:format) projects/merge_requests#new +# edit_namespace_project_merge_request GET /:namespace_id/:project_id/merge_requests/:id/edit(.:format) projects/merge_requests#edit +# namespace_project_merge_request GET /:namespace_id/:project_id/merge_requests/:id(.:format) projects/merge_requests#show +# PATCH /:namespace_id/:project_id/merge_requests/:id(.:format) projects/merge_requests#update +# PUT /:namespace_id/:project_id/merge_requests/:id(.:format) projects/merge_requests#update describe Projects::MergeRequestsController, 'routing' do it 'to #diffs' do expect(get('/gitlab/gitlabhq/merge_requests/1/diffs')).to route_to('projects/merge_requests#diffs', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1') end + it 'to #commits' do + expect(get('/gitlab/gitlabhq/merge_requests/1/commits')).to route_to('projects/merge_requests#commits', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1') + end + it 'to #automerge' do expect(post('/gitlab/gitlabhq/merge_requests/1/automerge')).to route_to( 'projects/merge_requests#automerge', |