From d92749989490289793e1e64fc6fff5673c41c75a Mon Sep 17 00:00:00 2001 From: Ciro Santilli Date: Sat, 4 Oct 2014 10:54:00 +0200 Subject: [PATCH 01/31] Remove unused Project#code function. --- app/models/project.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index d228da192e4..1c7fd27a38b 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -326,11 +326,6 @@ class Project < ActiveRecord::Base @ci_service ||= ci_services.select(&:activated?).first end - # For compatibility with old code - def code - path - end - def items_for(entity) case entity when 'issue' then From 6d4076fdc674cd5f9002e908e082d6c1c9dd1b90 Mon Sep 17 00:00:00 2001 From: Ciro Santilli Date: Tue, 7 Oct 2014 20:48:26 +0200 Subject: [PATCH 02/31] Disallow POST to compare: does not create objects --- config/routes.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 2534153758b..9273499f196 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -212,7 +212,8 @@ Gitlab::Application.routes.draw do end end - match "/compare/:from...:to" => "compare#show", as: "compare", via: [:get, :post], constraints: {from: /.+/, to: /.+/} + get '/compare/:from...:to' => 'compare#show', :as => 'compare', + :constraints => {from: /.+/, to: /.+/} resources :snippets, constraints: {id: /\d+/} do member do From f7274dd6a197da3501b7f3f5c7d298660f048fcc Mon Sep 17 00:00:00 2001 From: Ciro Santilli Date: Fri, 19 Sep 2014 10:33:41 +0200 Subject: [PATCH 03/31] Sort .gitignore. --- .gitignore | 60 +++++++++++++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/.gitignore b/.gitignore index 2c6b65b7b7d..7a7b5c93936 100644 --- a/.gitignore +++ b/.gitignore @@ -1,42 +1,42 @@ -.bundle -.rbx/ -db/*.sqlite3 -db/*.sqlite3-journal -log/*.log* -tmp/ -.sass-cache/ -coverage/* -backups/* +*.log *.swp -public/uploads/ -.ruby-version -.ruby-gemset -.rvmrc -.rbenv-version +.DS_Store +.bundle +.chef .directory -nohup.out -Vagrantfile +.envrc +.gitlab_shell_secret +.idea +.rbenv-version +.rbx/ +.ruby-gemset +.ruby-version +.rvmrc +.sass-cache/ +.secret .vagrant -config/gitlab.yml +Vagrantfile +backups/* +config/aws.yml config/database.yml +config/gitlab.yml config/initializers/omniauth.rb config/initializers/rack_attack.rb config/initializers/smtp_settings.rb -config/unicorn.rb config/resque.yml -config/aws.yml +config/unicorn.rb +coverage/* +db/*.sqlite3 +db/*.sqlite3-journal db/data.yml -.idea -.DS_Store -.chef -vendor/bundle/* -rails_best_practices_output.html doc/code/* -.secret -*.log -public/uploads.* -public/assets/ -.envrc dump.rdb +log/*.log* +nohup.out +public/assets/ +public/uploads.* +public/uploads/ +rails_best_practices_output.html tags -.gitlab_shell_secret +tmp/ +vendor/bundle/* From 2ff60660886008eef2a39bf1f2dcc249bbec8232 Mon Sep 17 00:00:00 2001 From: Miz Date: Thu, 11 Dec 2014 14:18:18 +0200 Subject: [PATCH 04/31] Add commit dates to repository-push email tempalte --- app/views/notify/repository_push_email.html.haml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/notify/repository_push_email.html.haml b/app/views/notify/repository_push_email.html.haml index 3cf50bf0826..d678147ec5d 100644 --- a/app/views/notify/repository_push_email.html.haml +++ b/app/views/notify/repository_push_email.html.haml @@ -6,7 +6,9 @@ - @commits.each do |commit| %li %strong #{link_to commit.short_id, project_commit_url(@project, commit)} - %span by #{commit.author_name} + %div + %span by #{commit.author_name} + %i at #{commit.committed_date.strftime("%Y-%m-%dT%H:%M:%SZ")} %pre #{commit.safe_message} %h4 Changes: From d6840542a7e96bbadf6ca221af489ec91d3fd747 Mon Sep 17 00:00:00 2001 From: Chulki Lee Date: Thu, 30 Oct 2014 10:18:32 -0700 Subject: [PATCH 05/31] ruby 2.1.5 in .ruby-version --- .ruby-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ruby-version b/.ruby-version index ac2cdeba013..cd57a8b95d6 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.1.3 +2.1.5 From 1fa19401e969f79cbd737c55e63249ca9355791c Mon Sep 17 00:00:00 2001 From: Jason Lippert Date: Mon, 8 Dec 2014 16:54:09 -0500 Subject: [PATCH 06/31] Teamcity interaction using 8.1 rest api --- CHANGELOG | 2 +- .../projects/services_controller.rb | 2 +- app/models/project.rb | 4 +- .../project_services/teamcity_service.rb | 116 ++++++++++++++++++ doc/project_services/project_services.md | 1 + features/project/service.feature | 7 ++ features/steps/project/services.rb | 20 +++ 7 files changed, 149 insertions(+), 3 deletions(-) create mode 100644 app/models/project_services/teamcity_service.rb diff --git a/CHANGELOG b/CHANGELOG index 4b78d1218ca..2bf5cb7ba32 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,7 +1,7 @@ v 7.7.0 - - - - + - Add Jetbrains Teamcity CI service (Jason Lippert) - - - diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb index c50a1f1e75b..ef4d2609147 100644 --- a/app/controllers/projects/services_controller.rb +++ b/app/controllers/projects/services_controller.rb @@ -42,7 +42,7 @@ class Projects::ServicesController < Projects::ApplicationController :title, :token, :type, :active, :api_key, :subdomain, :room, :recipients, :project_url, :webhook, :user_key, :device, :priority, :sound, :bamboo_url, :username, :password, - :build_key, :server + :build_key, :server, :teamcity_url, :build_type ) end end diff --git a/app/models/project.rb b/app/models/project.rb index 32b0145ca24..f0a49b633fe 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -66,6 +66,7 @@ class Project < ActiveRecord::Base has_one :slack_service, dependent: :destroy has_one :buildbox_service, dependent: :destroy has_one :bamboo_service, dependent: :destroy + has_one :teamcity_service, dependent: :destroy has_one :pushover_service, dependent: :destroy has_one :forked_project_link, dependent: :destroy, foreign_key: "forked_to_project_id" has_one :forked_from_project, through: :forked_project_link @@ -314,7 +315,8 @@ class Project < ActiveRecord::Base end def available_services_names - %w(gitlab_ci campfire hipchat pivotaltracker flowdock assembla emails_on_push gemnasium slack pushover buildbox bamboo) + %w(gitlab_ci campfire hipchat pivotaltracker flowdock assembla + emails_on_push gemnasium slack pushover buildbox bamboo teamcity) end def gitlab_ci? diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb new file mode 100644 index 00000000000..52b5862e4d1 --- /dev/null +++ b/app/models/project_services/teamcity_service.rb @@ -0,0 +1,116 @@ +class TeamcityService < CiService + include HTTParty + + prop_accessor :teamcity_url, :build_type, :username, :password + + validates :teamcity_url, presence: true, + format: { with: URI::regexp }, if: :activated? + validates :build_type, presence: true, if: :activated? + validates :username, presence: true, + if: ->(service) { service.password? }, if: :activated? + validates :password, presence: true, + if: ->(service) { service.username? }, if: :activated? + + attr_accessor :response + + after_save :compose_service_hook, if: :activated? + + def compose_service_hook + hook = service_hook || build_service_hook + hook.save + end + + def title + 'JetBrains TeamCity CI' + end + + def description + 'A continuous integration and build server' + end + + def help + 'The build configuration in Teamcity must use the build format '\ + 'number %build.vcs.number% '\ + 'you will also want to configure monitoring of all branches so merge '\ + 'requests build, that setting is in the vsc root advanced settings.' + end + + def to_param + 'teamcity' + end + + def fields + [ + { type: 'text', name: 'teamcity_url', + placeholder: 'TeamCity root URL like https://teamcity.example.com' }, + { type: 'text', name: 'build_type', + placeholder: 'Build configuration ID' }, + { type: 'text', name: 'username', + placeholder: 'A user with permissions to trigger a manual build' }, + { type: 'password', name: 'password' }, + ] + end + + def build_info(sha) + url = URI.parse("#{teamcity_url}/httpAuth/app/rest/builds/"\ + "branch:unspecified:any,number:#{sha}") + auth = { + username: username, + password: password, + } + @response = HTTParty.get("#{url}", verify: false, basic_auth: auth) + end + + def build_page(sha) + build_info(sha) if @response.nil? || !@response.code + + if @response.code != 200 + # If actual build link can't be determined, + # send user to build summary page. + "#{teamcity_url}/viewLog.html?buildTypeId=#{build_type}" + else + # If actual build link is available, go to build result page. + built_id = @response['build']['id'] + "#{teamcity_url}/viewLog.html?buildId=#{built_id}"\ + "&buildTypeId=#{build_type}" + end + end + + def commit_status(sha) + build_info(sha) if @response.nil? || !@response.code + return :error unless @response.code == 200 || @response.code == 404 + + status = if @response.code == 404 + 'Pending' + else + @response['build']['status'] + end + + if status.include?('SUCCESS') + 'success' + elsif status.include?('FAILURE') + 'failed' + elsif status.include?('Pending') + 'pending' + else + :error + end + end + + def execute(data) + auth = { + username: username, + password: password, + } + + branch = data[:ref] + + self.class.post("#{teamcity_url}/httpAuth/app/rest/buildQueue", + body: ""\ + ""\ + '', + headers: { 'Content-type' => 'application/xml' }, + basic_auth: auth + ) + end +end diff --git a/doc/project_services/project_services.md b/doc/project_services/project_services.md index 20a69a211dd..ec46af5fe3b 100644 --- a/doc/project_services/project_services.md +++ b/doc/project_services/project_services.md @@ -16,3 +16,4 @@ __Project integrations with external services for continuous integration and mor - PivotalTracker - Pushover - Slack +- TeamCity \ No newline at end of file diff --git a/features/project/service.feature b/features/project/service.feature index ed9e03b428d..85939a5c9ca 100644 --- a/features/project/service.feature +++ b/features/project/service.feature @@ -66,3 +66,10 @@ Feature: Project Services And I click Atlassian Bamboo CI service link And I fill Atlassian Bamboo CI settings Then I should see Atlassian Bamboo CI service settings saved + + Scenario: Activate jetBrains TeamCity CI service + When I visit project "Shop" services page + And I click jetBrains TeamCity CI service link + And I fill jetBrains TeamCity CI settings + Then I should see jetBrains TeamCity CI service settings saved + diff --git a/features/steps/project/services.rb b/features/steps/project/services.rb index 7a0b47a8fe5..09e86447058 100644 --- a/features/steps/project/services.rb +++ b/features/steps/project/services.rb @@ -15,6 +15,7 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps page.should have_content 'Assembla' page.should have_content 'Pushover' page.should have_content 'Atlassian Bamboo' + page.should have_content 'JetBrains TeamCity' end step 'I click gitlab-ci service link' do @@ -168,4 +169,23 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps find_field('Build key').value.should == 'KEY' find_field('Username').value.should == 'user' end + + step 'I click JetBrains TeamCity CI service link' do + click_link 'JetBrains TeamCity CI' + end + + step 'I fill JetBrains TeamCity CI settings' do + check 'Active' + fill_in 'Teamcity url', with: 'http://teamcity.example.com' + fill_in 'Build type', with: 'GitlabTest_Build' + fill_in 'Username', with: 'user' + fill_in 'Password', with: 'verySecret' + click_button 'Save' + end + + step 'I should see JetBrains TeamCity CI service settings saved' do + find_field('Teamcity url').value.should == 'http://teamcity.example.com' + find_field('Build type').value.should == 'GitlabTest_Build' + find_field('Username').value.should == 'user' + end end From e41dadcb33fda44ee274daa673bd933e13aa90eb Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Fri, 19 Dec 2014 16:15:29 +0200 Subject: [PATCH 07/31] Doorkeeper integration --- Gemfile | 2 + Gemfile.lock | 12 ++ .../oauth/applications_controller.rb | 25 +++ .../oauth/authorizations_controller.rb | 57 ++++++ .../authorized_applications_controller.rb | 8 + .../profiles/accounts_controller.rb | 2 + app/models/user.rb | 1 + .../oauth2/access_token_validation_service.rb | 41 ++++ .../applications/_delete_form.html.haml | 4 + .../doorkeeper/applications/_form.html.haml | 25 +++ .../doorkeeper/applications/edit.html.haml | 2 + .../doorkeeper/applications/index.html.haml | 16 ++ .../doorkeeper/applications/new.html.haml | 2 + .../doorkeeper/applications/show.html.haml | 21 +++ .../doorkeeper/authorizations/error.html.haml | 3 + .../doorkeeper/authorizations/new.html.haml | 28 +++ .../doorkeeper/authorizations/show.html.haml | 3 + .../_delete_form.html.haml | 4 + .../authorized_applications/index.html.haml | 16 ++ app/views/layouts/doorkeeper/admin.html.erb | 34 ++++ .../layouts/doorkeeper/application.html.erb | 23 +++ app/views/layouts/nav/_profile.html.haml | 2 +- app/views/profiles/accounts/show.html.haml | 35 ++++ config/initializers/doorkeeper.rb | 91 +++++++++ config/locales/doorkeeper.en.yml | 73 ++++++++ config/routes.rb | 5 + ...20141216155758_create_doorkeeper_tables.rb | 42 +++++ ...20141217125223_add_owner_to_application.rb | 7 + db/schema.rb | 45 ++++- features/profile/profile.feature | 14 ++ features/steps/profile/profile.rb | 50 +++++ lib/api/api.rb | 1 + lib/api/api_guard.rb | 175 ++++++++++++++++++ lib/api/helpers.rb | 2 +- spec/requests/api/api_helpers_spec.rb | 1 + spec/requests/api/doorkeeper_access_spec.rb | 31 ++++ 36 files changed, 900 insertions(+), 3 deletions(-) create mode 100644 app/controllers/oauth/applications_controller.rb create mode 100644 app/controllers/oauth/authorizations_controller.rb create mode 100644 app/controllers/oauth/authorized_applications_controller.rb create mode 100644 app/services/oauth2/access_token_validation_service.rb create mode 100644 app/views/doorkeeper/applications/_delete_form.html.haml create mode 100644 app/views/doorkeeper/applications/_form.html.haml create mode 100644 app/views/doorkeeper/applications/edit.html.haml create mode 100644 app/views/doorkeeper/applications/index.html.haml create mode 100644 app/views/doorkeeper/applications/new.html.haml create mode 100644 app/views/doorkeeper/applications/show.html.haml create mode 100644 app/views/doorkeeper/authorizations/error.html.haml create mode 100644 app/views/doorkeeper/authorizations/new.html.haml create mode 100644 app/views/doorkeeper/authorizations/show.html.haml create mode 100644 app/views/doorkeeper/authorized_applications/_delete_form.html.haml create mode 100644 app/views/doorkeeper/authorized_applications/index.html.haml create mode 100644 app/views/layouts/doorkeeper/admin.html.erb create mode 100644 app/views/layouts/doorkeeper/application.html.erb create mode 100644 config/initializers/doorkeeper.rb create mode 100644 config/locales/doorkeeper.en.yml create mode 100644 db/migrate/20141216155758_create_doorkeeper_tables.rb create mode 100644 db/migrate/20141217125223_add_owner_to_application.rb create mode 100644 lib/api/api_guard.rb create mode 100644 spec/requests/api/doorkeeper_access_spec.rb diff --git a/Gemfile b/Gemfile index ce9b83308f3..85e7bba444a 100644 --- a/Gemfile +++ b/Gemfile @@ -29,6 +29,8 @@ gem 'omniauth-twitter' gem 'omniauth-github' gem 'omniauth-shibboleth' gem 'omniauth-kerberos' +gem 'doorkeeper', '2.0.1' +gem "rack-oauth2", "~> 1.0.5" # Extracting information from a git repository # Provide access to Gitlab::Git library diff --git a/Gemfile.lock b/Gemfile.lock index cf96677f875..0d089305fe5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -37,6 +37,7 @@ GEM rake (>= 0.8.7) arel (5.0.1.20140414130214) asciidoctor (0.1.4) + attr_required (1.0.0) awesome_print (1.2.0) axiom-types (0.0.5) descendants_tracker (~> 0.0.1) @@ -107,6 +108,8 @@ GEM diff-lcs (1.2.5) diffy (3.0.3) docile (1.1.5) + doorkeeper (2.0.1) + railties (>= 3.1) dotenv (0.9.0) dropzonejs-rails (0.4.14) rails (> 3.1) @@ -250,6 +253,7 @@ GEM json (~> 1.8) multi_xml (>= 0.5.2) httpauth (0.2.1) + httpclient (2.5.3.3) i18n (0.6.11) ice_nine (0.10.0) jasmine (2.0.2) @@ -368,6 +372,12 @@ GEM rack (>= 1.1.3) rack-mount (0.8.3) rack (>= 1.0.0) + rack-oauth2 (1.0.8) + activesupport (>= 2.3) + attr_required (>= 0.0.5) + httpclient (>= 2.2.0.2) + multi_json (>= 1.3.6) + rack (>= 1.1) rack-protection (1.5.1) rack rack-test (0.6.2) @@ -616,6 +626,7 @@ DEPENDENCIES devise (= 3.2.4) devise-async (= 0.9.0) diffy (~> 3.0.3) + doorkeeper (= 2.0.1) dropzonejs-rails email_spec enumerize @@ -672,6 +683,7 @@ DEPENDENCIES rack-attack rack-cors rack-mini-profiler + rack-oauth2 (~> 1.0.5) rails (~> 4.1.0) rails_autolink (~> 1.1) rails_best_practices diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb new file mode 100644 index 00000000000..8eafe5e3b3d --- /dev/null +++ b/app/controllers/oauth/applications_controller.rb @@ -0,0 +1,25 @@ +class Oauth::ApplicationsController < Doorkeeper::ApplicationsController + before_filter :authenticate_user! + layout "profile" + + def index + @applications = current_user.oauth_applications + end + + def create + @application = Doorkeeper::Application.new(application_params) + @application.owner = current_user if Doorkeeper.configuration.confirm_application_owner? + if @application.save + flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create]) + redirect_to oauth_application_url(@application) + else + render :new + end + end + + def destroy + flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :destroy]) if @application.destroy + redirect_to profile_account_url + end + +end \ No newline at end of file diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb new file mode 100644 index 00000000000..c46707e2c77 --- /dev/null +++ b/app/controllers/oauth/authorizations_controller.rb @@ -0,0 +1,57 @@ +class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController + before_filter :authenticate_resource_owner! + layout "profile" + + def new + if pre_auth.authorizable? + if skip_authorization? || matching_token? + auth = authorization.authorize + redirect_to auth.redirect_uri + else + render "doorkeeper/authorizations/new" + end + else + render "doorkeeper/authorizations/error" + end + end + + # TODO: Handle raise invalid authorization + def create + redirect_or_render authorization.authorize + end + + def destroy + redirect_or_render authorization.deny + end + + private + + def matching_token? + Doorkeeper::AccessToken.matching_token_for pre_auth.client, + current_resource_owner.id, + pre_auth.scopes + end + + def redirect_or_render(auth) + if auth.redirectable? + redirect_to auth.redirect_uri + else + render json: auth.body, status: auth.status + end + end + + def pre_auth + @pre_auth ||= Doorkeeper::OAuth::PreAuthorization.new(Doorkeeper.configuration, + server.client_via_uid, + params) + end + + def authorization + @authorization ||= strategy.request + end + + def strategy + @strategy ||= server.authorization_request pre_auth.response_type + end +end + diff --git a/app/controllers/oauth/authorized_applications_controller.rb b/app/controllers/oauth/authorized_applications_controller.rb new file mode 100644 index 00000000000..b6d4a99c0a9 --- /dev/null +++ b/app/controllers/oauth/authorized_applications_controller.rb @@ -0,0 +1,8 @@ +class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicationsController + layout "profile" + + def destroy + Doorkeeper::AccessToken.revoke_all_for params[:id], current_resource_owner + redirect_to profile_account_url, notice: I18n.t(:notice, scope: [:doorkeeper, :flash, :authorized_applications, :destroy]) + end +end \ No newline at end of file diff --git a/app/controllers/profiles/accounts_controller.rb b/app/controllers/profiles/accounts_controller.rb index fe121691a10..5f15378c831 100644 --- a/app/controllers/profiles/accounts_controller.rb +++ b/app/controllers/profiles/accounts_controller.rb @@ -3,5 +3,7 @@ class Profiles::AccountsController < ApplicationController def show @user = current_user + @applications = current_user.oauth_applications + @authorized_applications = Doorkeeper::Application.authorized_for(current_user) end end diff --git a/app/models/user.rb b/app/models/user.rb index 7faeef1b5b0..6518fc50b70 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -106,6 +106,7 @@ class User < ActiveRecord::Base has_many :recent_events, -> { order "id DESC" }, foreign_key: :author_id, class_name: "Event" has_many :assigned_issues, dependent: :destroy, foreign_key: :assignee_id, class_name: "Issue" has_many :assigned_merge_requests, dependent: :destroy, foreign_key: :assignee_id, class_name: "MergeRequest" + has_many :oauth_applications, class_name: 'Doorkeeper::Application', as: :owner, dependent: :destroy # diff --git a/app/services/oauth2/access_token_validation_service.rb b/app/services/oauth2/access_token_validation_service.rb new file mode 100644 index 00000000000..95283489753 --- /dev/null +++ b/app/services/oauth2/access_token_validation_service.rb @@ -0,0 +1,41 @@ +module Oauth2::AccessTokenValidationService + # Results: + VALID = :valid + EXPIRED = :expired + REVOKED = :revoked + INSUFFICIENT_SCOPE = :insufficient_scope + + class << self + def validate(token, scopes: []) + if token.expired? + return EXPIRED + + elsif token.revoked? + return REVOKED + + elsif !self.sufficent_scope?(token, scopes) + return INSUFFICIENT_SCOPE + + else + return VALID + end + end + + protected + # True if the token's scope is a superset of required scopes, + # or the required scopes is empty. + def sufficent_scope?(token, scopes) + if scopes.blank? + # if no any scopes required, the scopes of token is sufficient. + return true + else + # If there are scopes required, then check whether + # the set of authorized scopes is a superset of the set of required scopes + required_scopes = Set.new(scopes) + authorized_scopes = Set.new(token.scopes) + + return authorized_scopes >= required_scopes + end + end + end +end \ No newline at end of file diff --git a/app/views/doorkeeper/applications/_delete_form.html.haml b/app/views/doorkeeper/applications/_delete_form.html.haml new file mode 100644 index 00000000000..bf8098f38d0 --- /dev/null +++ b/app/views/doorkeeper/applications/_delete_form.html.haml @@ -0,0 +1,4 @@ +- submit_btn_css ||= 'btn btn-link btn-remove btn-small' += form_tag oauth_application_path(application) do + %input{:name => "_method", :type => "hidden", :value => "delete"}/ + = submit_tag 'Destroy', onclick: "return confirm('Are you sure?')", class: submit_btn_css \ No newline at end of file diff --git a/app/views/doorkeeper/applications/_form.html.haml b/app/views/doorkeeper/applications/_form.html.haml new file mode 100644 index 00000000000..45ddf16ad0b --- /dev/null +++ b/app/views/doorkeeper/applications/_form.html.haml @@ -0,0 +1,25 @@ += form_for application, url: doorkeeper_submit_path(application), html: {class: 'form-horizontal', role: 'form'} do |f| + - if application.errors.any? + .alert.alert-danger{"data-alert" => ""} + %p Whoops! Check your form for possible errors + = content_tag :div, class: "form-group#{' has-error' if application.errors[:name].present?}" do + = f.label :name, class: 'col-sm-2 control-label' + .col-sm-10 + = f.text_field :name, class: 'form-control' + = doorkeeper_errors_for application, :name + = content_tag :div, class: "form-group#{' has-error' if application.errors[:redirect_uri].present?}" do + = f.label :redirect_uri, class: 'col-sm-2 control-label' + .col-sm-10 + = f.text_area :redirect_uri, class: 'form-control' + = doorkeeper_errors_for application, :redirect_uri + %span.help-block + Use one line per URI + - if Doorkeeper.configuration.native_redirect_uri + %span.help-block + Use + %code= Doorkeeper.configuration.native_redirect_uri + for local tests + .form-group + .col-sm-offset-2.col-sm-10 + = f.submit 'Submit', class: "btn btn-primary wide" + = link_to "Cancel", profile_account_path, :class => "btn btn-default" \ No newline at end of file diff --git a/app/views/doorkeeper/applications/edit.html.haml b/app/views/doorkeeper/applications/edit.html.haml new file mode 100644 index 00000000000..61584eb9c49 --- /dev/null +++ b/app/views/doorkeeper/applications/edit.html.haml @@ -0,0 +1,2 @@ +%h3.page-title Edit application += render 'form', application: @application \ No newline at end of file diff --git a/app/views/doorkeeper/applications/index.html.haml b/app/views/doorkeeper/applications/index.html.haml new file mode 100644 index 00000000000..e5be4b4bcac --- /dev/null +++ b/app/views/doorkeeper/applications/index.html.haml @@ -0,0 +1,16 @@ +%h3.page-title Your applications +%p= link_to 'New Application', new_oauth_application_path, class: 'btn btn-success' +%table.table.table-striped + %thead + %tr + %th Name + %th Callback URL + %th + %th + %tbody + - @applications.each do |application| + %tr{:id => "application_#{application.id}"} + %td= link_to application.name, oauth_application_path(application) + %td= application.redirect_uri + %td= link_to 'Edit', edit_oauth_application_path(application), class: 'btn btn-link' + %td= render 'delete_form', application: application \ No newline at end of file diff --git a/app/views/doorkeeper/applications/new.html.haml b/app/views/doorkeeper/applications/new.html.haml new file mode 100644 index 00000000000..655845e4af5 --- /dev/null +++ b/app/views/doorkeeper/applications/new.html.haml @@ -0,0 +1,2 @@ +%h3.page-title New application += render 'form', application: @application \ No newline at end of file diff --git a/app/views/doorkeeper/applications/show.html.haml b/app/views/doorkeeper/applications/show.html.haml new file mode 100644 index 00000000000..5236b865896 --- /dev/null +++ b/app/views/doorkeeper/applications/show.html.haml @@ -0,0 +1,21 @@ +%h3.page-title + Application: #{@application.name} +.row + .col-md-8 + %h4 Application Id: + %p + %code#application_id= @application.uid + %h4 Secret: + %p + %code#secret= @application.secret + %h4 Callback urls: + %table + - @application.redirect_uri.split.each do |uri| + %tr + %td + %code= uri + %td + = link_to 'Authorize', oauth_authorization_path(client_id: @application.uid, redirect_uri: uri, response_type: 'code'), class: 'btn btn-success', target: '_blank' +.prepend-top-20 + %p= link_to 'Edit', edit_oauth_application_path(@application), class: 'btn btn-primary wide pull-left' + %p= render 'delete_form', application: @application, submit_btn_css: 'btn btn-danger prepend-left-10' \ No newline at end of file diff --git a/app/views/doorkeeper/authorizations/error.html.haml b/app/views/doorkeeper/authorizations/error.html.haml new file mode 100644 index 00000000000..7561ec85ed9 --- /dev/null +++ b/app/views/doorkeeper/authorizations/error.html.haml @@ -0,0 +1,3 @@ +%h3.page-title An error has occurred +%main{:role => "main"} + %pre= @pre_auth.error_response.body[:error_description] \ No newline at end of file diff --git a/app/views/doorkeeper/authorizations/new.html.haml b/app/views/doorkeeper/authorizations/new.html.haml new file mode 100644 index 00000000000..15f9ee266c1 --- /dev/null +++ b/app/views/doorkeeper/authorizations/new.html.haml @@ -0,0 +1,28 @@ +%h3.page-title Authorize required +%main{:role => "main"} + %p.h4 + Authorize + %strong.text-info= @pre_auth.client.name + to use your account? + - if @pre_auth.scopes + #oauth-permissions + %p This application will be able to: + %ul.text-info + - @pre_auth.scopes.each do |scope| + %li= t scope, scope: [:doorkeeper, :scopes] + %hr/ + .actions + = form_tag oauth_authorization_path, method: :post do + = hidden_field_tag :client_id, @pre_auth.client.uid + = hidden_field_tag :redirect_uri, @pre_auth.redirect_uri + = hidden_field_tag :state, @pre_auth.state + = hidden_field_tag :response_type, @pre_auth.response_type + = hidden_field_tag :scope, @pre_auth.scope + = submit_tag "Authorize", class: "btn btn-success wide pull-left" + = form_tag oauth_authorization_path, method: :delete do + = hidden_field_tag :client_id, @pre_auth.client.uid + = hidden_field_tag :redirect_uri, @pre_auth.redirect_uri + = hidden_field_tag :state, @pre_auth.state + = hidden_field_tag :response_type, @pre_auth.response_type + = hidden_field_tag :scope, @pre_auth.scope + = submit_tag "Deny", class: "btn btn-danger prepend-left-10" \ No newline at end of file diff --git a/app/views/doorkeeper/authorizations/show.html.haml b/app/views/doorkeeper/authorizations/show.html.haml new file mode 100644 index 00000000000..9a402007194 --- /dev/null +++ b/app/views/doorkeeper/authorizations/show.html.haml @@ -0,0 +1,3 @@ +%h3.page-title Authorization code: +%main{:role => "main"} + %code#authorization_code= params[:code] \ No newline at end of file diff --git a/app/views/doorkeeper/authorized_applications/_delete_form.html.haml b/app/views/doorkeeper/authorized_applications/_delete_form.html.haml new file mode 100644 index 00000000000..5cbb4a70c19 --- /dev/null +++ b/app/views/doorkeeper/authorized_applications/_delete_form.html.haml @@ -0,0 +1,4 @@ +- submit_btn_css ||= 'btn btn-link btn-remove' += form_tag oauth_authorized_application_path(application) do + %input{:name => "_method", :type => "hidden", :value => "delete"}/ + = submit_tag 'Revoke', onclick: "return confirm('Are you sure?')", class: 'btn btn-link btn-remove btn-small' \ No newline at end of file diff --git a/app/views/doorkeeper/authorized_applications/index.html.haml b/app/views/doorkeeper/authorized_applications/index.html.haml new file mode 100644 index 00000000000..814cdc987ef --- /dev/null +++ b/app/views/doorkeeper/authorized_applications/index.html.haml @@ -0,0 +1,16 @@ +%header.page-header + %h1 Your authorized applications +%main{:role => "main"} + %table.table.table-striped + %thead + %tr + %th Application + %th Created At + %th + %th + %tbody + - @applications.each do |application| + %tr + %td= application.name + %td= application.created_at.strftime('%Y-%m-%d %H:%M:%S') + %td= render 'delete_form', application: application \ No newline at end of file diff --git a/app/views/layouts/doorkeeper/admin.html.erb b/app/views/layouts/doorkeeper/admin.html.erb new file mode 100644 index 00000000000..baeb5eb63fc --- /dev/null +++ b/app/views/layouts/doorkeeper/admin.html.erb @@ -0,0 +1,34 @@ + + + + + + + Doorkeeper + <%= stylesheet_link_tag "doorkeeper/admin/application" %> + <%= csrf_meta_tags %> + + + +
+ <%- if flash[:notice].present? %> +
+ <%= flash[:notice] %> +
+ <% end -%> + + <%= yield %> +
+ + diff --git a/app/views/layouts/doorkeeper/application.html.erb b/app/views/layouts/doorkeeper/application.html.erb new file mode 100644 index 00000000000..fd7a31584f3 --- /dev/null +++ b/app/views/layouts/doorkeeper/application.html.erb @@ -0,0 +1,23 @@ + + + + OAuth authorize required + + + + + <%= stylesheet_link_tag "doorkeeper/application" %> + <%= csrf_meta_tags %> + + +
+ <%- if flash[:notice].present? %> +
+ <%= flash[:notice] %> +
+ <% end -%> + + <%= yield %> +
+ + diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml index 05ba20e3611..f68fe87a75b 100644 --- a/app/views/layouts/nav/_profile.html.haml +++ b/app/views/layouts/nav/_profile.html.haml @@ -3,7 +3,7 @@ = link_to profile_path, title: "Profile" do %i.fa.fa-user Profile - = nav_link(controller: :accounts) do + = nav_link(controller: [:accounts, :applications]) do = link_to profile_account_path do %i.fa.fa-gear Account diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index a21dcff41c0..1d0b6d77189 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -75,3 +75,38 @@ The following groups will be abandoned. You should transfer or remove them: %strong #{current_user.solo_owned_groups.map(&:name).join(', ')} = link_to 'Delete account', user_registration_path, data: { confirm: "REMOVE #{current_user.name}? Are you sure?" }, method: :delete, class: "btn btn-remove" + + %h3.page-title + OAuth2 + %fieldset.oauth-applications + %legend Your applications + %p= link_to 'New Application', new_oauth_application_path, class: 'btn btn-success' + %table.table.table-striped + %thead + %tr + %th Name + %th Callback URL + %th + %th + %tbody + - @applications.each do |application| + %tr{:id => "application_#{application.id}"} + %td= link_to application.name, oauth_application_path(application) + %td= application.redirect_uri + %td= link_to 'Edit', edit_oauth_application_path(application), class: 'btn btn-link btn-small' + %td= render 'doorkeeper/applications/delete_form', application: application + + %fieldset.oauth-authorized-applications + %legend Your authorized applications + %table.table.table-striped + %thead + %tr + %th Name + %th Created At + %th + %tbody + - @authorized_applications.each do |application| + %tr{:id => "application_#{application.id}"} + %td= link_to application.name, oauth_application_path(application) + %td= application.created_at.strftime('%Y-%m-%d %H:%M:%S') + %td= render 'doorkeeper/authorized_applications/delete_form', application: application diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb new file mode 100644 index 00000000000..b2db3a7ea7e --- /dev/null +++ b/config/initializers/doorkeeper.rb @@ -0,0 +1,91 @@ +Doorkeeper.configure do + # Change the ORM that doorkeeper will use. + # Currently supported options are :active_record, :mongoid2, :mongoid3, :mongo_mapper + orm :active_record + + # This block will be called to check whether the resource owner is authenticated or not. + resource_owner_authenticator do + # Put your resource owner authentication logic here. + # Example implementation: + current_user || redirect_to(new_user_session_url) + end + + # If you want to restrict access to the web interface for adding oauth authorized applications, you need to declare the block below. + # admin_authenticator do + # # Put your admin authentication logic here. + # # Example implementation: + # Admin.find_by_id(session[:admin_id]) || redirect_to(new_admin_session_url) + # end + + # Authorization Code expiration time (default 10 minutes). + # authorization_code_expires_in 10.minutes + + # Access token expiration time (default 2 hours). + # If you want to disable expiration, set this to nil. + # access_token_expires_in 2.hours + + # Reuse access token for the same resource owner within an application (disabled by default) + # Rationale: https://github.com/doorkeeper-gem/doorkeeper/issues/383 + # reuse_access_token + + # Issue access tokens with refresh token (disabled by default) + use_refresh_token + + # Provide support for an owner to be assigned to each registered application (disabled by default) + # Optional parameter :confirmation => true (default false) if you want to enforce ownership of + # a registered application + # Note: you must also run the rails g doorkeeper:application_owner generator to provide the necessary support + enable_application_owner :confirmation => true + + # Define access token scopes for your provider + # For more information go to + # https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes + default_scopes :api + #optional_scopes :write, :update + + # Change the way client credentials are retrieved from the request object. + # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then + # falls back to the `:client_id` and `:client_secret` params from the `params` object. + # Check out the wiki for more information on customization + # client_credentials :from_basic, :from_params + + # Change the way access token is authenticated from the request object. + # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then + # falls back to the `:access_token` or `:bearer_token` params from the `params` object. + # Check out the wiki for more information on customization + access_token_methods :from_access_token_param, :from_bearer_authorization, :from_bearer_param + + # Change the native redirect uri for client apps + # When clients register with the following redirect uri, they won't be redirected to any server and the authorization code will be displayed within the provider + # The value can be any string. Use nil to disable this feature. When disabled, clients must provide a valid URL + # (Similar behaviour: https://developers.google.com/accounts/docs/OAuth2InstalledApp#choosingredirecturi) + # + native_redirect_uri nil#'urn:ietf:wg:oauth:2.0:oob' + + # Specify what grant flows are enabled in array of Strings. The valid + # strings and the flows they enable are: + # + # "authorization_code" => Authorization Code Grant Flow + # "implicit" => Implicit Grant Flow + # "password" => Resource Owner Password Credentials Grant Flow + # "client_credentials" => Client Credentials Grant Flow + # + # If not specified, Doorkeeper enables all the four grant flows. + # + # grant_flows %w(authorization_code implicit password client_credentials) + + # Under some circumstances you might want to have applications auto-approved, + # so that the user skips the authorization step. + # For example if dealing with trusted a application. + # skip_authorization do |resource_owner, client| + # client.superapp? or resource_owner.admin? + # end + + # WWW-Authenticate Realm (default "Doorkeeper"). + # realm "Doorkeeper" + + # Allow dynamic query parameters (disabled by default) + # Some applications require dynamic query parameters on their request_uri + # set to true if you want this to be allowed + # wildcard_redirect_uri false +end diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml new file mode 100644 index 00000000000..c5b6b75e7f6 --- /dev/null +++ b/config/locales/doorkeeper.en.yml @@ -0,0 +1,73 @@ +en: + activerecord: + errors: + models: + application: + attributes: + redirect_uri: + fragment_present: 'cannot contain a fragment.' + invalid_uri: 'must be a valid URI.' + relative_uri: 'must be an absolute URI.' + mongoid: + errors: + models: + application: + attributes: + redirect_uri: + fragment_present: 'cannot contain a fragment.' + invalid_uri: 'must be a valid URI.' + relative_uri: 'must be an absolute URI.' + mongo_mapper: + errors: + models: + application: + attributes: + redirect_uri: + fragment_present: 'cannot contain a fragment.' + invalid_uri: 'must be a valid URI.' + relative_uri: 'must be an absolute URI.' + doorkeeper: + errors: + messages: + # Common error messages + invalid_request: 'The request is missing a required parameter, includes an unsupported parameter value, or is otherwise malformed.' + invalid_redirect_uri: 'The redirect uri included is not valid.' + unauthorized_client: 'The client is not authorized to perform this request using this method.' + access_denied: 'The resource owner or authorization server denied the request.' + invalid_scope: 'The requested scope is invalid, unknown, or malformed.' + server_error: 'The authorization server encountered an unexpected condition which prevented it from fulfilling the request.' + temporarily_unavailable: 'The authorization server is currently unable to handle the request due to a temporary overloading or maintenance of the server.' + + #configuration error messages + credential_flow_not_configured: 'Resource Owner Password Credentials flow failed due to Doorkeeper.configure.resource_owner_from_credentials being unconfigured.' + resource_owner_authenticator_not_configured: 'Resource Owner find failed due to Doorkeeper.configure.resource_owner_authenticator being unconfiged.' + + # Access grant errors + unsupported_response_type: 'The authorization server does not support this response type.' + + # Access token errors + invalid_client: 'Client authentication failed due to unknown client, no client authentication included, or unsupported authentication method.' + invalid_grant: 'The provided authorization grant is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client.' + unsupported_grant_type: 'The authorization grant type is not supported by the authorization server.' + + # Password Access token errors + invalid_resource_owner: 'The provided resource owner credentials are not valid, or resource owner cannot be found' + + invalid_token: + revoked: "The access token was revoked" + expired: "The access token expired" + unknown: "The access token is invalid" + scopes: + api: Access your API + + flash: + applications: + create: + notice: 'Application created.' + destroy: + notice: 'Application deleted.' + update: + notice: 'Application updated.' + authorized_applications: + destroy: + notice: 'Application revoked.' diff --git a/config/routes.rb b/config/routes.rb index b6c5bb5b908..4d3039ce11a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -2,6 +2,11 @@ require 'sidekiq/web' require 'api/api' Gitlab::Application.routes.draw do + use_doorkeeper do + controllers :applications => 'oauth/applications', + :authorized_applications => 'oauth/authorized_applications', + :authorizations => 'oauth/authorizations' + end # # Search # diff --git a/db/migrate/20141216155758_create_doorkeeper_tables.rb b/db/migrate/20141216155758_create_doorkeeper_tables.rb new file mode 100644 index 00000000000..af5aa7d8b73 --- /dev/null +++ b/db/migrate/20141216155758_create_doorkeeper_tables.rb @@ -0,0 +1,42 @@ +class CreateDoorkeeperTables < ActiveRecord::Migration + def change + create_table :oauth_applications do |t| + t.string :name, null: false + t.string :uid, null: false + t.string :secret, null: false + t.text :redirect_uri, null: false + t.string :scopes, null: false, default: '' + t.timestamps + end + + add_index :oauth_applications, :uid, unique: true + + create_table :oauth_access_grants do |t| + t.integer :resource_owner_id, null: false + t.integer :application_id, null: false + t.string :token, null: false + t.integer :expires_in, null: false + t.text :redirect_uri, null: false + t.datetime :created_at, null: false + t.datetime :revoked_at + t.string :scopes + end + + add_index :oauth_access_grants, :token, unique: true + + create_table :oauth_access_tokens do |t| + t.integer :resource_owner_id + t.integer :application_id + t.string :token, null: false + t.string :refresh_token + t.integer :expires_in + t.datetime :revoked_at + t.datetime :created_at, null: false + t.string :scopes + end + + add_index :oauth_access_tokens, :token, unique: true + add_index :oauth_access_tokens, :resource_owner_id + add_index :oauth_access_tokens, :refresh_token, unique: true + end +end diff --git a/db/migrate/20141217125223_add_owner_to_application.rb b/db/migrate/20141217125223_add_owner_to_application.rb new file mode 100644 index 00000000000..7d5e6d07d0f --- /dev/null +++ b/db/migrate/20141217125223_add_owner_to_application.rb @@ -0,0 +1,7 @@ +class AddOwnerToApplication < ActiveRecord::Migration + def change + add_column :oauth_applications, :owner_id, :integer, null: true + add_column :oauth_applications, :owner_type, :string, null: true + add_index :oauth_applications, [:owner_id, :owner_type] + end +end \ No newline at end of file diff --git a/db/schema.rb b/db/schema.rb index b8335c5841b..73ddb14503f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20141205134006) do +ActiveRecord::Schema.define(version: 20141217125223) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -249,6 +249,49 @@ ActiveRecord::Schema.define(version: 20141205134006) do add_index "notes", ["project_id"], name: "index_notes_on_project_id", using: :btree add_index "notes", ["updated_at"], name: "index_notes_on_updated_at", using: :btree + create_table "oauth_access_grants", force: true do |t| + t.integer "resource_owner_id", null: false + t.integer "application_id", null: false + t.string "token", null: false + t.integer "expires_in", null: false + t.text "redirect_uri", null: false + t.datetime "created_at", null: false + t.datetime "revoked_at" + t.string "scopes" + end + + add_index "oauth_access_grants", ["token"], name: "index_oauth_access_grants_on_token", unique: true, using: :btree + + create_table "oauth_access_tokens", force: true do |t| + t.integer "resource_owner_id" + t.integer "application_id" + t.string "token", null: false + t.string "refresh_token" + t.integer "expires_in" + t.datetime "revoked_at" + t.datetime "created_at", null: false + t.string "scopes" + end + + add_index "oauth_access_tokens", ["refresh_token"], name: "index_oauth_access_tokens_on_refresh_token", unique: true, using: :btree + add_index "oauth_access_tokens", ["resource_owner_id"], name: "index_oauth_access_tokens_on_resource_owner_id", using: :btree + add_index "oauth_access_tokens", ["token"], name: "index_oauth_access_tokens_on_token", unique: true, using: :btree + + create_table "oauth_applications", force: true do |t| + t.string "name", null: false + t.string "uid", null: false + t.string "secret", null: false + t.text "redirect_uri", null: false + t.string "scopes", default: "", null: false + t.datetime "created_at" + t.datetime "updated_at" + t.integer "owner_id" + t.string "owner_type" + end + + add_index "oauth_applications", ["owner_id", "owner_type"], name: "index_oauth_applications_on_owner_id_and_owner_type", using: :btree + add_index "oauth_applications", ["uid"], name: "index_oauth_applications_on_uid", unique: true, using: :btree + create_table "projects", force: true do |t| t.string "name" t.string "path" diff --git a/features/profile/profile.feature b/features/profile/profile.feature index d7fa370fe2a..88a7a3e726b 100644 --- a/features/profile/profile.feature +++ b/features/profile/profile.feature @@ -71,6 +71,20 @@ Feature: Profile And I click on my profile picture Then I should see my user page + Scenario: I can manage application + Given I visit profile account page + Then I click on new application button + And I should see application form + Then I fill application form out and submit + And I see application + Then I click edit + And I see edit application form + Then I change name of application and submit + And I see that application was changed + Then I visit profile account page + And I click to remove application + Then I see that application is removed + @javascript Scenario: I change my application theme Given I visit profile design page diff --git a/features/steps/profile/profile.rb b/features/steps/profile/profile.rb index 38aaadcd28d..29fc7e68dac 100644 --- a/features/steps/profile/profile.rb +++ b/features/steps/profile/profile.rb @@ -221,4 +221,54 @@ class Spinach::Features::Profile < Spinach::FeatureSteps step 'I should see groups I belong to' do page.should have_css('.profile-groups-avatars', visible: true) end + + step 'I click on new application button' do + click_on 'New Application' + end + + step 'I should see application form' do + page.should have_content "New application" + end + + step 'I fill application form out and submit' do + fill_in :doorkeeper_application_name, with: 'test' + fill_in :doorkeeper_application_redirect_uri, with: 'https://test.com' + click_on "Submit" + end + + step 'I see application' do + page.should have_content "Application: test" + page.should have_content "Application Id" + page.should have_content "Secret" + end + + step 'I click edit' do + click_on "Edit" + end + + step 'I see edit application form' do + page.should have_content "Edit application" + end + + step 'I change name of application and submit' do + page.should have_content "Edit application" + fill_in :doorkeeper_application_name, with: 'test_changed' + click_on "Submit" + end + + step 'I see that application was changed' do + page.should have_content "test_changed" + page.should have_content "Application Id" + page.should have_content "Secret" + end + + step 'I click to remove application' do + within '.oauth-applications' do + click_on "Destroy" + end + end + + step "I see that application is removed" do + page.find(".oauth-applications").should_not have_content "test_changed" + end end diff --git a/lib/api/api.rb b/lib/api/api.rb index d26667ba3f7..cb46f477ff9 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -2,6 +2,7 @@ Dir["#{Rails.root}/lib/api/*.rb"].each {|file| require file} module API class API < Grape::API + include APIGuard version 'v3', using: :path rescue_from ActiveRecord::RecordNotFound do diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb new file mode 100644 index 00000000000..23975518181 --- /dev/null +++ b/lib/api/api_guard.rb @@ -0,0 +1,175 @@ +# Guard API with OAuth 2.0 Access Token + +require 'rack/oauth2' + +module APIGuard + extend ActiveSupport::Concern + + included do |base| + # OAuth2 Resource Server Authentication + use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request| + # The authenticator only fetches the raw token string + + # Must yield access token to store it in the env + request.access_token + end + + helpers HelperMethods + + install_error_responders(base) + end + + # Helper Methods for Grape Endpoint + module HelperMethods + # Invokes the doorkeeper guard. + # + # If token is presented and valid, then it sets @current_user. + # + # If the token does not have sufficient scopes to cover the requred scopes, + # then it raises InsufficientScopeError. + # + # If the token is expired, then it raises ExpiredError. + # + # If the token is revoked, then it raises RevokedError. + # + # If the token is not found (nil), then it raises TokenNotFoundError. + # + # Arguments: + # + # scopes: (optional) scopes required for this guard. + # Defaults to empty array. + # + def doorkeeper_guard!(scopes: []) + if (access_token = find_access_token).nil? + raise TokenNotFoundError + + else + case validate_access_token(access_token, scopes) + when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE + raise InsufficientScopeError.new(scopes) + + when Oauth2::AccessTokenValidationService::EXPIRED + raise ExpiredError + + when Oauth2::AccessTokenValidationService::REVOKED + raise RevokedError + + when Oauth2::AccessTokenValidationService::VALID + @current_user = User.find(access_token.resource_owner_id) + + end + end + end + + def doorkeeper_guard(scopes: []) + if access_token = find_access_token + case validate_access_token(access_token, scopes) + when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE + raise InsufficientScopeError.new(scopes) + + when Oauth2::AccessTokenValidationService::EXPIRED + raise ExpiredError + + when Oauth2::AccessTokenValidationService::REVOKED + raise RevokedError + + when Oauth2::AccessTokenValidationService::VALID + @current_user = User.find(access_token.resource_owner_id) + end + end + end + + def current_user + @current_user + end + + private + def find_access_token + @access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods) + end + + def doorkeeper_request + @doorkeeper_request ||= ActionDispatch::Request.new(env) + end + + def validate_access_token(access_token, scopes) + Oauth2::AccessTokenValidationService.validate(access_token, scopes: scopes) + end + end + + module ClassMethods + # Installs the doorkeeper guard on the whole Grape API endpoint. + # + # Arguments: + # + # scopes: (optional) scopes required for this guard. + # Defaults to empty array. + # + def guard_all!(scopes: []) + before do + guard! scopes: scopes + end + end + + private + def install_error_responders(base) + error_classes = [ MissingTokenError, TokenNotFoundError, + ExpiredError, RevokedError, InsufficientScopeError] + + base.send :rescue_from, *error_classes, oauth2_bearer_token_error_handler + end + + def oauth2_bearer_token_error_handler + Proc.new {|e| + response = case e + when MissingTokenError + Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new + + when TokenNotFoundError + Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( + :invalid_token, + "Bad Access Token.") + + when ExpiredError + Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( + :invalid_token, + "Token is expired. You can either do re-authorization or token refresh.") + + when RevokedError + Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( + :invalid_token, + "Token was revoked. You have to re-authorize from the user.") + + when InsufficientScopeError + # FIXME: ForbiddenError (inherited from Bearer::Forbidden of Rack::Oauth2) + # does not include WWW-Authenticate header, which breaks the standard. + Rack::OAuth2::Server::Resource::Bearer::Forbidden.new( + :insufficient_scope, + Rack::OAuth2::Server::Resource::ErrorMethods::DEFAULT_DESCRIPTION[:insufficient_scope], + { :scope => e.scopes}) + end + + response.finish + } + end + end + + # + # Exceptions + # + + class MissingTokenError < StandardError; end + + class TokenNotFoundError < StandardError; end + + class ExpiredError < StandardError; end + + class RevokedError < StandardError; end + + class InsufficientScopeError < StandardError + attr_reader :scopes + def initialize(scopes) + @scopes = scopes + end + end +end \ No newline at end of file diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 027fb20ec46..2f2342840fd 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -11,7 +11,7 @@ module API def current_user private_token = (params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]).to_s - @current_user ||= User.find_by(authentication_token: private_token) + @current_user ||= (User.find_by(authentication_token: private_token) || doorkeeper_guard) unless @current_user && Gitlab::UserAccess.allowed?(@current_user) return nil diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb index e2f222c0d34..cc071342d7c 100644 --- a/spec/requests/api/api_helpers_spec.rb +++ b/spec/requests/api/api_helpers_spec.rb @@ -41,6 +41,7 @@ describe API, api: true do describe ".current_user" do it "should return nil for an invalid token" do env[API::APIHelpers::PRIVATE_TOKEN_HEADER] = 'invalid token' + self.class.any_instance.stub(:doorkeeper_guard){ false } current_user.should be_nil end diff --git a/spec/requests/api/doorkeeper_access_spec.rb b/spec/requests/api/doorkeeper_access_spec.rb new file mode 100644 index 00000000000..ddef99d77af --- /dev/null +++ b/spec/requests/api/doorkeeper_access_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe API::API, api: true do + include ApiHelpers + + let!(:user) { create(:user) } + let!(:application) { Doorkeeper::Application.create!(:name => "MyApp", :redirect_uri => "https://app.com", :owner => user) } + let!(:token) { Doorkeeper::AccessToken.create! :application_id => application.id, :resource_owner_id => user.id } + + + describe "when unauthenticated" do + it "returns authentication success" do + get api("/user"), :access_token => token.token + response.status.should == 200 + end + end + + describe "when token invalid" do + it "returns authentication error" do + get api("/user"), :access_token => "123a" + response.status.should == 401 + end + end + + describe "authorization by private token" do + it "returns authentication success" do + get api("/user", user) + response.status.should == 200 + end + end +end From a61ccd4ad2d83b2422561a374c300260e5a6d240 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Wed, 24 Dec 2014 15:44:17 +0200 Subject: [PATCH 08/31] convert erb to haml --- app/views/layouts/doorkeeper/admin.html.erb | 34 ------------------- app/views/layouts/doorkeeper/admin.html.haml | 22 ++++++++++++ .../layouts/doorkeeper/application.html.erb | 23 ------------- .../layouts/doorkeeper/application.html.haml | 15 ++++++++ 4 files changed, 37 insertions(+), 57 deletions(-) delete mode 100644 app/views/layouts/doorkeeper/admin.html.erb create mode 100644 app/views/layouts/doorkeeper/admin.html.haml delete mode 100644 app/views/layouts/doorkeeper/application.html.erb create mode 100644 app/views/layouts/doorkeeper/application.html.haml diff --git a/app/views/layouts/doorkeeper/admin.html.erb b/app/views/layouts/doorkeeper/admin.html.erb deleted file mode 100644 index baeb5eb63fc..00000000000 --- a/app/views/layouts/doorkeeper/admin.html.erb +++ /dev/null @@ -1,34 +0,0 @@ - - - - - - - Doorkeeper - <%= stylesheet_link_tag "doorkeeper/admin/application" %> - <%= csrf_meta_tags %> - - - -
- <%- if flash[:notice].present? %> -
- <%= flash[:notice] %> -
- <% end -%> - - <%= yield %> -
- - diff --git a/app/views/layouts/doorkeeper/admin.html.haml b/app/views/layouts/doorkeeper/admin.html.haml new file mode 100644 index 00000000000..bd9adfab66d --- /dev/null +++ b/app/views/layouts/doorkeeper/admin.html.haml @@ -0,0 +1,22 @@ +!!! +%html + %head + %meta{:charset => "utf-8"} + %meta{:content => "IE=edge", "http-equiv" => "X-UA-Compatible"} + %meta{:content => "width=device-width, initial-scale=1.0", :name => "viewport"} + %title Doorkeeper + = stylesheet_link_tag "doorkeeper/admin/application" + = csrf_meta_tags + %body + .navbar.navbar-inverse.navbar-fixed-top{:role => "navigation"} + .container + .navbar-header + = link_to 'OAuth2 Provider', oauth_applications_path, class: 'navbar-brand' + %ul.nav.navbar-nav + = content_tag :li, class: "#{'active' if request.path == oauth_applications_path}" do + = link_to 'Applications', oauth_applications_path + .container + - if flash[:notice].present? + .alert.alert-info + = flash[:notice] + = yield \ No newline at end of file diff --git a/app/views/layouts/doorkeeper/application.html.erb b/app/views/layouts/doorkeeper/application.html.erb deleted file mode 100644 index fd7a31584f3..00000000000 --- a/app/views/layouts/doorkeeper/application.html.erb +++ /dev/null @@ -1,23 +0,0 @@ - - - - OAuth authorize required - - - - - <%= stylesheet_link_tag "doorkeeper/application" %> - <%= csrf_meta_tags %> - - -
- <%- if flash[:notice].present? %> -
- <%= flash[:notice] %> -
- <% end -%> - - <%= yield %> -
- - diff --git a/app/views/layouts/doorkeeper/application.html.haml b/app/views/layouts/doorkeeper/application.html.haml new file mode 100644 index 00000000000..e5f37fad1f4 --- /dev/null +++ b/app/views/layouts/doorkeeper/application.html.haml @@ -0,0 +1,15 @@ +!!! +%html + %head + %title OAuth authorize required + %meta{:charset => "utf-8"} + %meta{:content => "IE=edge", "http-equiv" => "X-UA-Compatible"} + %meta{:content => "width=device-width, initial-scale=1.0", :name => "viewport"} + = stylesheet_link_tag "doorkeeper/application" + = csrf_meta_tags + %body + #container + - if flash[:notice].present? + .alert.alert-info + = flash[:notice] + = yield \ No newline at end of file From 5140a4cd139e43a3c7a1d23fdd61bfc0d9aff6a6 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Dec 2014 14:32:49 +0200 Subject: [PATCH 09/31] Set of UI changes mostly for issue and merge request * return edit/close buttons to old position (right of title) * make 'Issue #1' header smaller * move mr commits to separate tab * change inline/side diff switcher to buttons from tabs * make issue sidebar start with dicsussion block Signed-off-by: Dmitriy Zaporozhets --- .../javascripts/merge_request.js.coffee | 3 + app/assets/stylesheets/generic/issue_box.scss | 4 +- .../stylesheets/sections/merge_requests.scss | 1 + app/helpers/diff_helper.rb | 18 +++ app/views/projects/diffs/_diffs.html.haml | 12 +- .../projects/issues/_discussion.html.haml | 37 ++++++ app/views/projects/issues/show.html.haml | 86 +++++--------- .../merge_requests/_discussion.html.haml | 31 ++++++ .../projects/merge_requests/_show.html.haml | 105 +++++++----------- .../merge_requests/show/_mr_title.html.haml | 17 ++- .../show/_participants.html.haml | 5 - 11 files changed, 179 insertions(+), 140 deletions(-) create mode 100644 app/views/projects/issues/_discussion.html.haml create mode 100644 app/views/projects/merge_requests/_discussion.html.haml diff --git a/app/assets/javascripts/merge_request.js.coffee b/app/assets/javascripts/merge_request.js.coffee index fba933ddab5..9e3ca45ce04 100644 --- a/app/assets/javascripts/merge_request.js.coffee +++ b/app/assets/javascripts/merge_request.js.coffee @@ -89,6 +89,9 @@ class @MergeRequest this.$('.merge-request-tabs .diffs-tab').addClass 'active' this.loadDiff() unless @diffs_loaded this.$('.diffs').show() + when 'commits' + this.$('.merge-request-tabs .commits-tab').addClass 'active' + this.$('.commits').show() else this.$('.merge-request-tabs .notes-tab').addClass 'active' this.$('.notes').show() diff --git a/app/assets/stylesheets/generic/issue_box.scss b/app/assets/stylesheets/generic/issue_box.scss index 176c45581a8..2563ab516e2 100644 --- a/app/assets/stylesheets/generic/issue_box.scss +++ b/app/assets/stylesheets/generic/issue_box.scss @@ -6,7 +6,9 @@ .issue-box { display: inline-block; - padding: 0 10px; + padding: 7px 13px; + font-weight: normal; + margin-right: 5px; &.issue-box-closed { background-color: $bg_danger; diff --git a/app/assets/stylesheets/sections/merge_requests.scss b/app/assets/stylesheets/sections/merge_requests.scss index a0f709070ac..f3525dc589e 100644 --- a/app/assets/stylesheets/sections/merge_requests.scss +++ b/app/assets/stylesheets/sections/merge_requests.scss @@ -102,6 +102,7 @@ .mr-state-widget { background: $box_bg; margin-bottom: 20px; + color: #666; @include box-shadow(0 1px 1px rgba(0, 0, 0, 0.09)); .ci_widget { diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index cb50d89cba8..a15af0be01a 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -117,4 +117,22 @@ module DiffHelper [comments_left, comments_right] end + + def inline_diff_btn + params_copy = params.dup + params_copy[:view] = 'inline' + + link_to url_for(params_copy), id: "commit-diff-viewtype", class: (params[:view] != 'parallel' ? 'btn active' : 'btn') do + 'Inline' + end + end + + def parallel_diff_btn + params_copy = params.dup + params_copy[:view] = 'parallel' + + link_to url_for(params_copy), id: "commit-diff-viewtype", class: (params[:view] == 'parallel' ? 'btn active' : 'btn') do + 'Side-by-side' + end + end end diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index 334ea1ba82f..48d4c33ce85 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -2,15 +2,9 @@ .col-md-8 = render 'projects/diffs/stats', diffs: diffs .col-md-4 - %ul.nav.nav-tabs - %li.pull-right{class: params[:view] == 'parallel' ? 'active' : ''} - - params_copy = params.dup - - params_copy[:view] = 'parallel' - = link_to "Side-by-side Diff", url_for(params_copy), {id: "commit-diff-viewtype"} - %li.pull-right{class: params[:view] != 'parallel' ? 'active' : ''} - - params_copy[:view] = 'inline' - = link_to "Inline Diff", url_for(params_copy), {id: "commit-diff-viewtype"} - + .btn-group.pull-right + = inline_diff_btn + = parallel_diff_btn - if show_diff_size_warning?(diffs) = render 'projects/diffs/warning', diffs: diffs diff --git a/app/views/projects/issues/_discussion.html.haml b/app/views/projects/issues/_discussion.html.haml new file mode 100644 index 00000000000..d62afe582b9 --- /dev/null +++ b/app/views/projects/issues/_discussion.html.haml @@ -0,0 +1,37 @@ +- content_for :note_actions do + - if can?(current_user, :modify_issue, @issue) + - if @issue.closed? + = link_to 'Reopen Issue', project_issue_path(@project, @issue, issue: {state_event: :reopen }, status_only: true), method: :put, class: "btn btn-grouped btn-reopen js-note-target-reopen", title: 'Reopen Issue' + - else + = link_to 'Close Issue', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn btn-grouped btn-close js-note-target-close", title: "Close Issue" +.row + .col-sm-9 + .participants + %cite.cgray + = pluralize(@issue.participants.count, 'participant') + - @issue.participants.each do |participant| + = link_to_member(@project, participant, name: false, size: 24) + + .voting_notes#notes= render "projects/notes/notes_with_form" + .col-sm-3 + %div + .clearfix + %span.slead.has_tooltip{:"data-original-title" => 'Cross-project reference'} + = cross_project_reference(@project, @issue) + %hr + .clearfix + .votes-holder + %h6 Votes + #votes= render 'votes/votes_block', votable: @issue + %hr + .context + %cite.cgray + = render partial: 'issue_context', locals: { issue: @issue } + + - if @issue.labels.any? + %hr + %h6 Labels + .issue-show-labels + - @issue.labels.each do |label| + = link_to project_issues_path(@project, label_name: label.name) do + %p= render_colored_label(label) diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 1c9af4c4501..b21a394ebeb 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -1,65 +1,37 @@ -%h3.page-title +%h4.page-title .issue-box{ class: issue_box_class(@issue) } - if @issue.closed? Closed - else Open Issue ##{@issue.iid} - .pull-right.creator - %small Created by #{link_to_member(@project, @issue.author)} #{issue_timestamp(@issue)} + %small.creator + · created by #{link_to_member(@project, @issue.author)} #{issue_timestamp(@issue)} + + .pull-right + - if can?(current_user, :write_issue, @project) + = link_to new_project_issue_path(@project), class: "btn btn-grouped", title: "New Issue", id: "new_issue_link" do + %i.fa.fa-plus + New Issue + - if can?(current_user, :modify_issue, @issue) + - if @issue.closed? + = link_to 'Reopen', project_issue_path(@project, @issue, issue: {state_event: :reopen }, status_only: true), method: :put, class: "btn btn-grouped btn-reopen" + - else + = link_to 'Close', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn btn-grouped btn-close", title: "Close Issue" + + = link_to edit_project_issue_path(@project, @issue), class: "btn btn-grouped issuable-edit" do + %i.fa.fa-pencil-square-o + Edit + %hr -.row - .col-sm-9 - %h3.issue-title - = gfm escape_once(@issue.title) - %div - - if @issue.description.present? - .description - .wiki - = preserve do - = markdown(@issue.description, parse_tasks: true) - %hr - - content_for :note_actions do - - if can?(current_user, :modify_issue, @issue) - - if @issue.closed? - = link_to 'Reopen Issue', project_issue_path(@project, @issue, issue: {state_event: :reopen }, status_only: true), method: :put, class: "btn btn-grouped btn-reopen js-note-target-reopen", title: 'Reopen Issue' - - else - = link_to 'Close Issue', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn btn-grouped btn-close js-note-target-close", title: "Close Issue" - .participants - %cite.cgray - = pluralize(@issue.participants.count, 'participant') - - @issue.participants.each do |participant| - = link_to_member(@project, participant, name: false, size: 24) - .issue-show-labels.pull-right - - @issue.labels.each do |label| - = link_to project_issues_path(@project, label_name: label.name) do - = render_colored_label(label) +%h3.issue-title + = gfm escape_once(@issue.title) +%div + - if @issue.description.present? + .description + .wiki + = preserve do + = markdown(@issue.description, parse_tasks: true) - .voting_notes#notes= render "projects/notes/notes_with_form" - .col-sm-3 - %div - - if can?(current_user, :write_issue, @project) - = link_to new_project_issue_path(@project), class: "btn btn-block", title: "New Issue", id: "new_issue_link" do - %i.fa.fa-plus - New Issue - - if can?(current_user, :modify_issue, @issue) - - if @issue.closed? - = link_to 'Reopen', project_issue_path(@project, @issue, issue: {state_event: :reopen }, status_only: true), method: :put, class: "btn btn-block btn-reopen" - - else - = link_to 'Close', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn btn-block btn-close", title: "Close Issue" - - = link_to edit_project_issue_path(@project, @issue), class: "btn btn-block issuable-edit" do - %i.fa.fa-pencil-square-o - Edit - .clearfix - %span.slead.has_tooltip{:"data-original-title" => 'Cross-project reference'} - = cross_project_reference(@project, @issue) - %hr - .clearfix - .votes-holder - %h6 Votes - #votes= render 'votes/votes_block', votable: @issue - %hr - .context - %cite.cgray - = render partial: 'issue_context', locals: { issue: @issue } +%hr += render "projects/issues/discussion" diff --git a/app/views/projects/merge_requests/_discussion.html.haml b/app/views/projects/merge_requests/_discussion.html.haml new file mode 100644 index 00000000000..b0b4f24dd3f --- /dev/null +++ b/app/views/projects/merge_requests/_discussion.html.haml @@ -0,0 +1,31 @@ +- content_for :note_actions do + - if can?(current_user, :modify_merge_request, @merge_request) + - if @merge_request.open? + = link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-grouped btn-close close-mr-link js-note-target-close", title: "Close merge request" + - if @merge_request.closed? + = link_to 'Reopen', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-grouped btn-reopen reopen-mr-link js-note-target-reopen", title: "Reopen merge request" + +.row + .col-sm-9 + = render "projects/merge_requests/show/participants" + = render "projects/notes/notes_with_form" + .col-sm-3 + .clearfix + %span.slead.has_tooltip{:"data-original-title" => 'Cross-project reference'} + = cross_project_reference(@project, @merge_request) + %hr + .votes-holder.hidden-sm.hidden-xs + %h6 Votes + #votes= render 'votes/votes_block', votable: @merge_request + %hr + .context + %cite.cgray + = render partial: 'projects/merge_requests/show/context', locals: { merge_request: @merge_request } + + - if @merge_request.labels.any? + %hr + %h6 Labels + .merge-request-show-labels + - @merge_request.labels.each do |label| + = link_to project_merge_requests_path(@project, label_name: label.name) do + %p= render_colored_label(label) diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 57ab6bdd545..cc42efb2f50 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -1,89 +1,64 @@ .merge-request = render "projects/merge_requests/show/mr_title" %hr - .row - .col-sm-9 - = render "projects/merge_requests/show/how_to_merge" - = render "projects/merge_requests/show/mr_box" - %hr - .append-bottom-20 - %p.slead - %span From - - if @merge_request.for_fork? - %strong.label-branch< - - if @merge_request.source_project - = link_to @merge_request.source_project_namespace, project_path(@merge_request.source_project) - - else - \ #{@merge_request.source_project_namespace} - \:#{@merge_request.source_branch} - %span into - %strong.label-branch #{@merge_request.target_project_namespace}:#{@merge_request.target_branch} + = render "projects/merge_requests/show/mr_box" + %hr + .append-bottom-20 + .slead + %span From + - if @merge_request.for_fork? + %strong.label-branch< + - if @merge_request.source_project + = link_to @merge_request.source_project_namespace, project_path(@merge_request.source_project) - else - %strong.label-branch #{@merge_request.source_branch} - %span into - %strong.label-branch #{@merge_request.target_branch} - = render "projects/merge_requests/show/state_widget" - = render "projects/merge_requests/show/commits" - = render "projects/merge_requests/show/participants" + \ #{@merge_request.source_project_namespace} + \:#{@merge_request.source_branch} + %span into + %strong.label-branch #{@merge_request.target_project_namespace}:#{@merge_request.target_branch} + - else + %strong.label-branch #{@merge_request.source_branch} + %span into + %strong.label-branch #{@merge_request.target_branch} + - if @merge_request.open? + %span.pull-right + .btn-group + %a.btn.dropdown-toggle{ data: {toggle: :dropdown} } + %i.fa.fa-download + Download as + %span.caret + %ul.dropdown-menu + %li= link_to "Email Patches", project_merge_request_path(@project, @merge_request, format: :patch) + %li= link_to "Plain Diff", project_merge_request_path(@project, @merge_request, format: :diff) - .col-sm-3 - .issue-btn-group - - if can?(current_user, :modify_merge_request, @merge_request) - - if @merge_request.open? - .btn-group-justified.append-bottom-20 - .btn-group - %a.btn.dropdown-toggle{ data: {toggle: :dropdown} } - %i.fa.fa-download - Download as - %span.caret - %ul.dropdown-menu - %li= link_to "Email Patches", project_merge_request_path(@project, @merge_request, format: :patch) - %li= link_to "Plain Diff", project_merge_request_path(@project, @merge_request, format: :diff) - = link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: { state_event: :close }), method: :put, class: "btn btn-block btn-close", title: "Close merge request" - = link_to edit_project_merge_request_path(@project, @merge_request), class: "btn btn-block issuable-edit", id: "edit_merge_request" do - %i.fa.fa-pencil-square-o - Edit - - if @merge_request.closed? - = link_to 'Reopen', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-block btn-reopen reopen-mr-link", title: "Close merge request" - .clearfix - %span.slead.has_tooltip{:"data-original-title" => 'Cross-project reference'} - = cross_project_reference(@project, @merge_request) - %hr - .votes-holder.hidden-sm.hidden-xs - %h6 Votes - #votes= render 'votes/votes_block', votable: @merge_request - %hr - .context - %cite.cgray - = render partial: 'projects/merge_requests/show/context', locals: { merge_request: @merge_request } + = render "projects/merge_requests/show/how_to_merge" + = render "projects/merge_requests/show/state_widget" - if @commits.present? %ul.nav.nav-tabs.merge-request-tabs %li.notes-tab{data: {action: 'notes'}} = link_to project_merge_request_path(@project, @merge_request) do - %i.fa.fa-comment + %i.fa.fa-comments Discussion %span.badge= @merge_request.mr_and_commit_notes.count + %li.commits-tab{data: {action: 'commits'}} + = link_to project_merge_request_path(@project, @merge_request) do + %i.fa.fa-database + Commits + %span.badge= @commits.size %li.diffs-tab{data: {action: 'diffs'}} = link_to diffs_project_merge_request_path(@project, @merge_request) do %i.fa.fa-list-alt Changes %span.badge= @merge_request.diffs.size - - content_for :note_actions do - - if can?(current_user, :modify_merge_request, @merge_request) - - if @merge_request.open? - = link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-grouped btn-close close-mr-link js-note-target-close", title: "Close merge request" - - if @merge_request.closed? - = link_to 'Reopen', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-grouped btn-reopen reopen-mr-link js-note-target-reopen", title: "Reopen merge request" - + .notes.tab-content.voting_notes#notes{ class: (controller.action_name == 'show') ? "" : "hide" } + = render "projects/merge_requests/discussion" + .commits.tab-content + = render "projects/merge_requests/show/commits" .diffs.tab-content - if current_page?(action: 'diffs') = render "projects/merge_requests/show/diffs" - .notes.tab-content.voting_notes#notes{ class: (controller.action_name == 'show') ? "" : "hide" } - .row - .col-sm-9 - = render "projects/notes/notes_with_form" + .mr-loading-status = spinner diff --git a/app/views/projects/merge_requests/show/_mr_title.html.haml b/app/views/projects/merge_requests/show/_mr_title.html.haml index fb34de43c1b..0f20eba382c 100644 --- a/app/views/projects/merge_requests/show/_mr_title.html.haml +++ b/app/views/projects/merge_requests/show/_mr_title.html.haml @@ -1,4 +1,4 @@ -%h3.page-title +%h4.page-title .issue-box{ class: issue_box_class(@merge_request) } - if @merge_request.merged? Merged @@ -7,5 +7,16 @@ - else Open = "Merge Request ##{@merge_request.iid}" - .pull-right.creator - %small Created by #{link_to_member(@project, @merge_request.author)} #{time_ago_with_tooltip(@merge_request.created_at)} + %small.creator + · + created by #{link_to_member(@project, @merge_request.author)} #{time_ago_with_tooltip(@merge_request.created_at)} + + .issue-btn-group.pull-right + - if can?(current_user, :modify_merge_request, @merge_request) + - if @merge_request.open? + = link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: { state_event: :close }), method: :put, class: "btn btn-grouped btn-close", title: "Close merge request" + = link_to edit_project_merge_request_path(@project, @merge_request), class: "btn btn-grouped issuable-edit", id: "edit_merge_request" do + %i.fa.fa-pencil-square-o + Edit + - if @merge_request.closed? + = link_to 'Reopen', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-grouped btn-reopen reopen-mr-link", title: "Close merge request" diff --git a/app/views/projects/merge_requests/show/_participants.html.haml b/app/views/projects/merge_requests/show/_participants.html.haml index b709c89cec2..15a97404cb0 100644 --- a/app/views/projects/merge_requests/show/_participants.html.haml +++ b/app/views/projects/merge_requests/show/_participants.html.haml @@ -2,8 +2,3 @@ %cite.cgray #{@merge_request.participants.count} participants - @merge_request.participants.each do |participant| = link_to_member(@project, participant, name: false, size: 24) - - .merge-request-show-labels.pull-right - - @merge_request.labels.each do |label| - = link_to project_merge_requests_path(@project, label_name: label.name) do - = render_colored_label(label) From 88b480174cbd0d95726df1390f667996efcf52f3 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Dec 2014 14:46:18 +0200 Subject: [PATCH 10/31] Improve issue/mr page for tablets Signed-off-by: Dmitriy Zaporozhets --- .../stylesheets/sections/merge_requests.scss | 14 ++++++++------ app/views/projects/issues/_discussion.html.haml | 4 ++-- .../projects/merge_requests/_discussion.html.haml | 4 ++-- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/app/assets/stylesheets/sections/merge_requests.scss b/app/assets/stylesheets/sections/merge_requests.scss index f3525dc589e..49bbae05340 100644 --- a/app/assets/stylesheets/sections/merge_requests.scss +++ b/app/assets/stylesheets/sections/merge_requests.scss @@ -19,13 +19,15 @@ } } -.merge-request .merge-request-tabs{ - margin: 20px 0; +@media(min-width: $screen-sm-max) { + .merge-request .merge-request-tabs{ + margin: 20px 0; - li { - a { - padding: 15px 40px; - font-size: 14px; + li { + a { + padding: 15px 40px; + font-size: 14px; + } } } } diff --git a/app/views/projects/issues/_discussion.html.haml b/app/views/projects/issues/_discussion.html.haml index d62afe582b9..ec03f375d6b 100644 --- a/app/views/projects/issues/_discussion.html.haml +++ b/app/views/projects/issues/_discussion.html.haml @@ -5,7 +5,7 @@ - else = link_to 'Close Issue', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn btn-grouped btn-close js-note-target-close", title: "Close Issue" .row - .col-sm-9 + .col-md-9 .participants %cite.cgray = pluralize(@issue.participants.count, 'participant') @@ -13,7 +13,7 @@ = link_to_member(@project, participant, name: false, size: 24) .voting_notes#notes= render "projects/notes/notes_with_form" - .col-sm-3 + .col-md-3.hidden-sm.hidden-xs %div .clearfix %span.slead.has_tooltip{:"data-original-title" => 'Cross-project reference'} diff --git a/app/views/projects/merge_requests/_discussion.html.haml b/app/views/projects/merge_requests/_discussion.html.haml index b0b4f24dd3f..6bb5c465596 100644 --- a/app/views/projects/merge_requests/_discussion.html.haml +++ b/app/views/projects/merge_requests/_discussion.html.haml @@ -6,10 +6,10 @@ = link_to 'Reopen', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-grouped btn-reopen reopen-mr-link js-note-target-reopen", title: "Reopen merge request" .row - .col-sm-9 + .col-md-9 = render "projects/merge_requests/show/participants" = render "projects/notes/notes_with_form" - .col-sm-3 + .col-md-3.hidden-sm.hidden-xs .clearfix %span.slead.has_tooltip{:"data-original-title" => 'Cross-project reference'} = cross_project_reference(@project, @merge_request) From 99e52c9ad082a4a8f953bab9f41e023de5182c37 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Dec 2014 14:58:43 +0200 Subject: [PATCH 11/31] Small UI imporovement for merge request accept widget and projects page Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/sections/merge_requests.scss | 1 - app/views/dashboard/projects.html.haml | 12 +++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/sections/merge_requests.scss b/app/assets/stylesheets/sections/merge_requests.scss index 49bbae05340..920702ff3c4 100644 --- a/app/assets/stylesheets/sections/merge_requests.scss +++ b/app/assets/stylesheets/sections/merge_requests.scss @@ -149,7 +149,6 @@ padding: 10px 15px; h4 { - font-size: 20px; font-weight: normal; } diff --git a/app/views/dashboard/projects.html.haml b/app/views/dashboard/projects.html.haml index 5b7835b097b..b880acf1245 100644 --- a/app/views/dashboard/projects.html.haml +++ b/app/views/dashboard/projects.html.haml @@ -38,17 +38,19 @@ = link_to project_path(project), class: dom_class(project) do = project.name_with_namespace + - if project.forked_from_project +   + %small + %i.fa.fa-code-fork + Forked from: + = link_to project.forked_from_project.name_with_namespace, project_path(project.forked_from_project) + - if current_user.can_leave_project?(project) .pull-right = link_to leave_project_team_members_path(project), data: { confirm: "Leave project?"}, method: :delete, remote: true, class: "btn-tiny btn remove-row", title: 'Leave project' do %i.fa.fa-sign-out Leave - - if project.forked_from_project - %small.pull-right - %i.fa.fa-code-fork - Forked from: - = link_to project.forked_from_project.name_with_namespace, project_path(project.forked_from_project) .project-info .pull-right - if project.archived? From fca161f5c50e282acf65e11decc86a35d5e43847 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Dec 2014 15:55:12 +0200 Subject: [PATCH 12/31] Fix tests Signed-off-by: Dmitriy Zaporozhets --- app/views/projects/merge_requests/_show.html.haml | 2 +- features/steps/project/commits/commits.rb | 6 +++--- features/steps/project/merge_requests.rb | 6 +++++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index cc42efb2f50..74ef819a7aa 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -41,7 +41,7 @@ Discussion %span.badge= @merge_request.mr_and_commit_notes.count %li.commits-tab{data: {action: 'commits'}} - = link_to project_merge_request_path(@project, @merge_request) do + = link_to project_merge_request_path(@project, @merge_request), title: 'Commits' do %i.fa.fa-database Commits %span.badge= @commits.size diff --git a/features/steps/project/commits/commits.rb b/features/steps/project/commits/commits.rb index 935f313e298..d515ee1ac11 100644 --- a/features/steps/project/commits/commits.rb +++ b/features/steps/project/commits/commits.rb @@ -78,14 +78,14 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps end step 'I click side-by-side diff button' do - click_link "Side-by-side Diff" + click_link "Side-by-side" end step 'I see side-by-side diff button' do - page.should have_content "Side-by-side Diff" + page.should have_content "Side-by-side" end step 'I see inline diff button' do - page.should have_content "Inline Diff" + page.should have_content "Inline" end end diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index b00f610cfae..28928d602d6 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -109,6 +109,10 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps end step 'I click on the commit in the merge request' do + within '.merge-request-tabs' do + click_link 'Commits' + end + within '.mr-commits' do click_link Commit.truncate_sha(sample_commit.id) end @@ -261,7 +265,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps end step 'I click Side-by-side Diff tab' do - click_link 'Side-by-side Diff' + click_link 'Side-by-side' end step 'I should see comments on the side-by-side diff page' do From 40ff1bc8ba4969a47e805694ec11a367a15f23eb Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Dec 2014 15:55:23 +0200 Subject: [PATCH 13/31] Align sidebar navigation differently Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/sections/sidebar.scss | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/sections/sidebar.scss b/app/assets/stylesheets/sections/sidebar.scss index 65229336e92..51d6b2c920c 100644 --- a/app/assets/stylesheets/sections/sidebar.scss +++ b/app/assets/stylesheets/sections/sidebar.scss @@ -60,7 +60,7 @@ font-size: 13px; line-height: 20px; text-shadow: 0 1px 2px #FFF; - padding-left: 67px; + padding-left: 20px; &:hover { text-decoration: none; @@ -75,6 +75,7 @@ i { width: 20px; color: #888; + margin-right: 23px; } } } @@ -91,7 +92,7 @@ a { padding: 5px 15px; font-size: 12px; - padding-left: 67px; + padding-left: 20px; } } } From 7fe8d41d88f744b16e6e12c1c07ef3f956994110 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Dec 2014 16:46:28 +0200 Subject: [PATCH 14/31] Improve code style Signed-off-by: Dmitriy Zaporozhets --- app/controllers/oauth/applications_controller.rb | 14 ++++++++++---- app/controllers/oauth/authorizations_controller.rb | 11 ++++++----- .../oauth/authorized_applications_controller.rb | 4 ++-- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb index 8eafe5e3b3d..b53e9662af0 100644 --- a/app/controllers/oauth/applications_controller.rb +++ b/app/controllers/oauth/applications_controller.rb @@ -8,7 +8,11 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController def create @application = Doorkeeper::Application.new(application_params) - @application.owner = current_user if Doorkeeper.configuration.confirm_application_owner? + + if Doorkeeper.configuration.confirm_application_owner? + @application.owner = current_user + end + if @application.save flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create]) redirect_to oauth_application_url(@application) @@ -18,8 +22,10 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController end def destroy - flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :destroy]) if @application.destroy + if @application.destroy + flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :destroy]) + end + redirect_to profile_account_url end - -end \ No newline at end of file +end diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index c46707e2c77..72cbbf2e616 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/app/controllers/oauth/authorizations_controller.rb @@ -27,9 +27,9 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController private def matching_token? - Doorkeeper::AccessToken.matching_token_for pre_auth.client, - current_resource_owner.id, - pre_auth.scopes + Doorkeeper::AccessToken.matching_token_for(pre_auth.client, + current_resource_owner.id, + pre_auth.scopes) end def redirect_or_render(auth) @@ -41,7 +41,8 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController end def pre_auth - @pre_auth ||= Doorkeeper::OAuth::PreAuthorization.new(Doorkeeper.configuration, + @pre_auth ||= + Doorkeeper::OAuth::PreAuthorization.new(Doorkeeper.configuration, server.client_via_uid, params) end @@ -51,7 +52,7 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController end def strategy - @strategy ||= server.authorization_request pre_auth.response_type + @strategy ||= server.authorization_request(pre_auth.response_type) end end diff --git a/app/controllers/oauth/authorized_applications_controller.rb b/app/controllers/oauth/authorized_applications_controller.rb index b6d4a99c0a9..202421b4abd 100644 --- a/app/controllers/oauth/authorized_applications_controller.rb +++ b/app/controllers/oauth/authorized_applications_controller.rb @@ -2,7 +2,7 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio layout "profile" def destroy - Doorkeeper::AccessToken.revoke_all_for params[:id], current_resource_owner + Doorkeeper::AccessToken.revoke_all_for(params[:id], current_resource_owner) redirect_to profile_account_url, notice: I18n.t(:notice, scope: [:doorkeeper, :flash, :authorized_applications, :destroy]) end -end \ No newline at end of file +end From 592e396869ba5dc116cec333733cea8dfbf4a9b5 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Dec 2014 18:35:04 +0200 Subject: [PATCH 15/31] Rework oauth2 feature * improve UI * add authorization * add separate page for oauth applications Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/generic/tables.scss | 20 +++++++++ app/assets/stylesheets/sections/tree.scss | 13 ------ .../oauth/applications_controller.rb | 12 +++++- .../oauth/authorizations_controller.rb | 1 - .../authorized_applications_controller.rb | 2 +- .../profiles/accounts_controller.rb | 2 - app/controllers/profiles_controller.rb | 5 +++ app/models/user.rb | 4 ++ .../doorkeeper/applications/_form.html.haml | 7 ++- .../doorkeeper/applications/show.html.haml | 37 +++++++++------- app/views/layouts/nav/_profile.html.haml | 6 ++- app/views/profiles/accounts/show.html.haml | 34 --------------- app/views/profiles/applications.html.haml | 43 +++++++++++++++++++ config/routes.rb | 1 + 14 files changed, 114 insertions(+), 73 deletions(-) create mode 100644 app/assets/stylesheets/generic/tables.scss create mode 100644 app/views/profiles/applications.html.haml diff --git a/app/assets/stylesheets/generic/tables.scss b/app/assets/stylesheets/generic/tables.scss new file mode 100644 index 00000000000..71a7d4abaee --- /dev/null +++ b/app/assets/stylesheets/generic/tables.scss @@ -0,0 +1,20 @@ +table { + &.table { + tr { + td, th { + padding: 8px 10px; + line-height: 20px; + vertical-align: middle; + } + th { + font-weight: normal; + font-size: 15px; + border-bottom: 1px solid #CCC !important; + } + td { + border-color: #F1F1F1 !important; + border-bottom: 1px solid; + } + } + } +} diff --git a/app/assets/stylesheets/sections/tree.scss b/app/assets/stylesheets/sections/tree.scss index 678a6cd716d..bc7451e2d53 100644 --- a/app/assets/stylesheets/sections/tree.scss +++ b/app/assets/stylesheets/sections/tree.scss @@ -17,19 +17,6 @@ @include border-radius(0); tr { - td, th { - padding: 8px 10px; - line-height: 20px; - } - th { - font-weight: normal; - font-size: 15px; - border-bottom: 1px solid #CCC !important; - } - td { - border-color: #F1F1F1 !important; - border-bottom: 1px solid; - } &:hover { td { background: $hover; diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb index b53e9662af0..93201eff303 100644 --- a/app/controllers/oauth/applications_controller.rb +++ b/app/controllers/oauth/applications_controller.rb @@ -3,7 +3,7 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController layout "profile" def index - @applications = current_user.oauth_applications + head :forbidden and return end def create @@ -28,4 +28,14 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController redirect_to profile_account_url end + + private + + def set_application + @application = current_user.oauth_applications.find(params[:id]) + end + + rescue_from ActiveRecord::RecordNotFound do |exception| + render "errors/not_found", layout: "errors", status: 404 + end end diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index 72cbbf2e616..a57b4a60c24 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/app/controllers/oauth/authorizations_controller.rb @@ -55,4 +55,3 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController @strategy ||= server.authorization_request(pre_auth.response_type) end end - diff --git a/app/controllers/oauth/authorized_applications_controller.rb b/app/controllers/oauth/authorized_applications_controller.rb index 202421b4abd..0b27ce7da72 100644 --- a/app/controllers/oauth/authorized_applications_controller.rb +++ b/app/controllers/oauth/authorized_applications_controller.rb @@ -3,6 +3,6 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio def destroy Doorkeeper::AccessToken.revoke_all_for(params[:id], current_resource_owner) - redirect_to profile_account_url, notice: I18n.t(:notice, scope: [:doorkeeper, :flash, :authorized_applications, :destroy]) + redirect_to applications_profile_url, notice: I18n.t(:notice, scope: [:doorkeeper, :flash, :authorized_applications, :destroy]) end end diff --git a/app/controllers/profiles/accounts_controller.rb b/app/controllers/profiles/accounts_controller.rb index 5f15378c831..fe121691a10 100644 --- a/app/controllers/profiles/accounts_controller.rb +++ b/app/controllers/profiles/accounts_controller.rb @@ -3,7 +3,5 @@ class Profiles::AccountsController < ApplicationController def show @user = current_user - @applications = current_user.oauth_applications - @authorized_applications = Doorkeeper::Application.authorized_for(current_user) end end diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index e877f9b9049..c0b7e2223a2 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -13,6 +13,11 @@ class ProfilesController < ApplicationController def design end + def applications + @applications = current_user.oauth_applications + @authorized_tokens = current_user.oauth_authorized_tokens + end + def update user_params.except!(:email) if @user.ldap_user? diff --git a/app/models/user.rb b/app/models/user.rb index 6518fc50b70..7dae318e780 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -565,4 +565,8 @@ class User < ActiveRecord::Base namespaces += masters_groups end end + + def oauth_authorized_tokens + Doorkeeper::AccessToken.where(resource_owner_id: self.id, revoked_at: nil) + end end diff --git a/app/views/doorkeeper/applications/_form.html.haml b/app/views/doorkeeper/applications/_form.html.haml index 45ddf16ad0b..a5fec2fabdb 100644 --- a/app/views/doorkeeper/applications/_form.html.haml +++ b/app/views/doorkeeper/applications/_form.html.haml @@ -19,7 +19,6 @@ Use %code= Doorkeeper.configuration.native_redirect_uri for local tests - .form-group - .col-sm-offset-2.col-sm-10 - = f.submit 'Submit', class: "btn btn-primary wide" - = link_to "Cancel", profile_account_path, :class => "btn btn-default" \ No newline at end of file + .form-actions + = f.submit 'Submit', class: "btn btn-primary wide" + = link_to "Cancel", applications_profile_path, class: "btn btn-default" diff --git a/app/views/doorkeeper/applications/show.html.haml b/app/views/doorkeeper/applications/show.html.haml index 5236b865896..82e78b4af13 100644 --- a/app/views/doorkeeper/applications/show.html.haml +++ b/app/views/doorkeeper/applications/show.html.haml @@ -1,21 +1,26 @@ %h3.page-title Application: #{@application.name} -.row - .col-md-8 - %h4 Application Id: - %p + + +%table.table + %tr + %td + Application Id + %td %code#application_id= @application.uid - %h4 Secret: - %p + %tr + %td + Secret: + %td %code#secret= @application.secret - %h4 Callback urls: - %table + + %tr + %td + Callback url + %td - @application.redirect_uri.split.each do |uri| - %tr - %td - %code= uri - %td - = link_to 'Authorize', oauth_authorization_path(client_id: @application.uid, redirect_uri: uri, response_type: 'code'), class: 'btn btn-success', target: '_blank' -.prepend-top-20 - %p= link_to 'Edit', edit_oauth_application_path(@application), class: 'btn btn-primary wide pull-left' - %p= render 'delete_form', application: @application, submit_btn_css: 'btn btn-danger prepend-left-10' \ No newline at end of file + %div + %span.monospace= uri +.form-actions + = link_to 'Edit', edit_oauth_application_path(@application), class: 'btn btn-primary wide pull-left' + = render 'delete_form', application: @application, submit_btn_css: 'btn btn-danger prepend-left-10' diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml index f68fe87a75b..8bb45e4a6d0 100644 --- a/app/views/layouts/nav/_profile.html.haml +++ b/app/views/layouts/nav/_profile.html.haml @@ -3,10 +3,14 @@ = link_to profile_path, title: "Profile" do %i.fa.fa-user Profile - = nav_link(controller: [:accounts, :applications]) do + = nav_link(controller: [:accounts]) do = link_to profile_account_path do %i.fa.fa-gear Account + = nav_link(path: ['profiles#applications', 'applications#edit', 'applications#show', 'applications#new']) do + = link_to applications_profile_path do + %i.fa.fa-cloud + Applications = nav_link(controller: :emails) do = link_to profile_emails_path do %i.fa.fa-envelope-o diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 1d0b6d77189..53a50f6796b 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -75,38 +75,4 @@ The following groups will be abandoned. You should transfer or remove them: %strong #{current_user.solo_owned_groups.map(&:name).join(', ')} = link_to 'Delete account', user_registration_path, data: { confirm: "REMOVE #{current_user.name}? Are you sure?" }, method: :delete, class: "btn btn-remove" - - %h3.page-title - OAuth2 - %fieldset.oauth-applications - %legend Your applications - %p= link_to 'New Application', new_oauth_application_path, class: 'btn btn-success' - %table.table.table-striped - %thead - %tr - %th Name - %th Callback URL - %th - %th - %tbody - - @applications.each do |application| - %tr{:id => "application_#{application.id}"} - %td= link_to application.name, oauth_application_path(application) - %td= application.redirect_uri - %td= link_to 'Edit', edit_oauth_application_path(application), class: 'btn btn-link btn-small' - %td= render 'doorkeeper/applications/delete_form', application: application - %fieldset.oauth-authorized-applications - %legend Your authorized applications - %table.table.table-striped - %thead - %tr - %th Name - %th Created At - %th - %tbody - - @authorized_applications.each do |application| - %tr{:id => "application_#{application.id}"} - %td= link_to application.name, oauth_application_path(application) - %td= application.created_at.strftime('%Y-%m-%d %H:%M:%S') - %td= render 'doorkeeper/authorized_applications/delete_form', application: application diff --git a/app/views/profiles/applications.html.haml b/app/views/profiles/applications.html.haml new file mode 100644 index 00000000000..cdb188dc1af --- /dev/null +++ b/app/views/profiles/applications.html.haml @@ -0,0 +1,43 @@ +%h3.page-title + OAuth2 + +%fieldset.oauth-applications + %legend Your applications + %p= link_to 'New Application', new_oauth_application_path, class: 'btn btn-success' + - if @applications.any? + %table.table.table-striped + %thead + %tr + %th Name + %th Callback URL + %th Clients + %th + %th + %tbody + - @applications.each do |application| + %tr{:id => "application_#{application.id}"} + %td= link_to application.name, oauth_application_path(application) + %td + - application.redirect_uri.split.each do |uri| + %div= uri + %td= application.access_tokens.count + %td= link_to 'Edit', edit_oauth_application_path(application), class: 'btn btn-link btn-small' + %td= render 'doorkeeper/applications/delete_form', application: application + +%fieldset.oauth-authorized-applications.prepend-top-20 + %legend Authorized applications + %table.table.table-striped + %thead + %tr + %th Name + %th Authorized At + %th Scope + %th + %tbody + - @authorized_tokens.each do |token| + - application = token.application + %tr{:id => "application_#{application.id}"} + %td= application.name + %td= token.created_at + %td= token.scopes + %td= render 'doorkeeper/authorized_applications/delete_form', application: application diff --git a/config/routes.rb b/config/routes.rb index 4d3039ce11a..1d571e21b88 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -118,6 +118,7 @@ Gitlab::Application.routes.draw do member do get :history get :design + get :applications put :reset_private_token put :update_username From aadfb3665f39e5886254bac856ebd1cc47f8c652 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Dec 2014 18:46:19 +0200 Subject: [PATCH 16/31] Fix tests and add message if no oauth apps Signed-off-by: Dmitriy Zaporozhets --- .../oauth/applications_controller.rb | 2 +- app/views/profiles/applications.html.haml | 34 +++++++++++-------- features/profile/profile.feature | 6 ++-- features/steps/shared/paths.rb | 4 +++ 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb index 93201eff303..3407490e498 100644 --- a/app/controllers/oauth/applications_controller.rb +++ b/app/controllers/oauth/applications_controller.rb @@ -26,7 +26,7 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :destroy]) end - redirect_to profile_account_url + redirect_to applications_profile_url end private diff --git a/app/views/profiles/applications.html.haml b/app/views/profiles/applications.html.haml index cdb188dc1af..cb24e4a3dde 100644 --- a/app/views/profiles/applications.html.haml +++ b/app/views/profiles/applications.html.haml @@ -26,18 +26,22 @@ %fieldset.oauth-authorized-applications.prepend-top-20 %legend Authorized applications - %table.table.table-striped - %thead - %tr - %th Name - %th Authorized At - %th Scope - %th - %tbody - - @authorized_tokens.each do |token| - - application = token.application - %tr{:id => "application_#{application.id}"} - %td= application.name - %td= token.created_at - %td= token.scopes - %td= render 'doorkeeper/authorized_applications/delete_form', application: application + + - if @authorized_tokens.any? + %table.table.table-striped + %thead + %tr + %th Name + %th Authorized At + %th Scope + %th + %tbody + - @authorized_tokens.each do |token| + - application = token.application + %tr{:id => "application_#{application.id}"} + %td= application.name + %td= token.created_at + %td= token.scopes + %td= render 'doorkeeper/authorized_applications/delete_form', application: application + - else + %p.light You dont have any authorized applications diff --git a/features/profile/profile.feature b/features/profile/profile.feature index 88a7a3e726b..fd132e1cd80 100644 --- a/features/profile/profile.feature +++ b/features/profile/profile.feature @@ -72,7 +72,7 @@ Feature: Profile Then I should see my user page Scenario: I can manage application - Given I visit profile account page + Given I visit profile applications page Then I click on new application button And I should see application form Then I fill application form out and submit @@ -81,7 +81,7 @@ Feature: Profile And I see edit application form Then I change name of application and submit And I see that application was changed - Then I visit profile account page + Then I visit profile applications page And I click to remove application Then I see that application is removed @@ -115,4 +115,4 @@ Feature: Profile Scenario: I see the password strength indicator with success Given I visit profile password page When I try to set a strong password - Then I should see the input field green \ No newline at end of file + Then I should see the input field green diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index 5f292255ce1..ca038732231 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -94,6 +94,10 @@ module SharedPaths visit profile_path end + step 'I visit profile applications page' do + visit applications_profile_path + end + step 'I visit profile password page' do visit edit_profile_password_path end From d2bd5e833fdcf5bbb039936a3f71eaf7ff829063 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Dec 2014 20:51:07 +0200 Subject: [PATCH 17/31] Fix nav_link support for several path options Signed-off-by: Dmitriy Zaporozhets --- app/helpers/tab_helper.rb | 52 +++++++++++++++--------- app/views/layouts/nav/_profile.html.haml | 3 +- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/app/helpers/tab_helper.rb b/app/helpers/tab_helper.rb index bc43e078568..639fc98c222 100644 --- a/app/helpers/tab_helper.rb +++ b/app/helpers/tab_helper.rb @@ -28,6 +28,10 @@ module TabHelper # nav_link(controller: [:tree, :refs]) { "Hello" } # # => '
  • Hello
  • ' # + # # Several paths + # nav_link(path: ['tree#show', 'profile#show']) { "Hello" } + # # => '
  • Hello
  • ' + # # # Shorthand path # nav_link(path: 'tree#show') { "Hello" } # # => '
  • Hello
  • ' @@ -38,25 +42,7 @@ module TabHelper # # Returns a list item element String def nav_link(options = {}, &block) - if path = options.delete(:path) - if path.respond_to?(:each) - c = path.map { |p| p.split('#').first } - a = path.map { |p| p.split('#').last } - else - c, a, _ = path.split('#') - end - else - c = options.delete(:controller) - a = options.delete(:action) - end - - if c && a - # When given both options, make sure BOTH are active - klass = current_controller?(*c) && current_action?(*a) ? 'active' : '' - else - # Otherwise check EITHER option - klass = current_controller?(*c) || current_action?(*a) ? 'active' : '' - end + klass = active_nav_link?(options) ? 'active' : '' # Add our custom class into the html_options, which may or may not exist # and which may or may not already have a :class key @@ -72,6 +58,34 @@ module TabHelper end end + def active_nav_link?(options) + if path = options.delete(:path) + unless path.respond_to?(:each) + path = [path] + end + + path.any? do |single_path| + current_path?(single_path) + end + else + c = options.delete(:controller) + a = options.delete(:action) + + if c && a + # When given both options, make sure BOTH are true + current_controller?(*c) && current_action?(*a) + else + # Otherwise check EITHER option + current_controller?(*c) || current_action?(*a) + end + end + end + + def current_path?(path) + c, a, _ = path.split('#') + current_controller?(c) && current_action?(a) + end + def project_tab_class return "active" if current_page?(controller: "/projects", action: :edit, id: @project) diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml index 2821e5c0668..36b48a5d02d 100644 --- a/app/views/layouts/nav/_profile.html.haml +++ b/app/views/layouts/nav/_profile.html.haml @@ -11,7 +11,8 @@ = nav_link(path: ['profiles#applications', 'applications#edit', 'applications#show', 'applications#new']) do = link_to applications_profile_path do %i.fa.fa-cloud - Applications + %span + Applications = nav_link(controller: :emails) do = link_to profile_emails_path do %i.fa.fa-envelope-o From b01c5d993c10704c5097d9eaba24ef849fe3a46d Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Dec 2014 21:31:04 +0200 Subject: [PATCH 18/31] New CHANGELOG items Signed-off-by: Dmitriy Zaporozhets --- CHANGELOG | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 4b78d1218ca..e5e1c7d349b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,13 +7,13 @@ v 7.7.0 - - - + - OAuth applications feature - - + - Set project path instead of project name in create form - - - - - - - - + - New side navigation From e3951019f5de7359659a4c13db0eeb16cf1195f1 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 26 Dec 2014 14:19:30 +0200 Subject: [PATCH 19/31] Put nprogress spinner to bottom left position Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/generic/common.scss | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/assets/stylesheets/generic/common.scss b/app/assets/stylesheets/generic/common.scss index 2fc738c18d8..dfc7b0de9b7 100644 --- a/app/assets/stylesheets/generic/common.scss +++ b/app/assets/stylesheets/generic/common.scss @@ -355,3 +355,9 @@ table { .task-status { margin-left: 10px; } + +#nprogress .spinner { + top: auto !important; + bottom: 20px !important; + left: 20px !important; +} From 038161f4e0b41a0fd6b877171a1f47d062b5c857 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 26 Dec 2014 14:19:54 +0200 Subject: [PATCH 20/31] Make nprogress color to red Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/main/variables.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/main/variables.scss b/app/assets/stylesheets/main/variables.scss index ca296c85a91..92b220f8019 100644 --- a/app/assets/stylesheets/main/variables.scss +++ b/app/assets/stylesheets/main/variables.scss @@ -46,4 +46,4 @@ $deleted: #f77; /** * NProgress customize */ -$nprogress-color: #3498db; +$nprogress-color: #c0392b; From f1c39763c9daf5f053f9b9ae0bcd1c50ea59133f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 26 Dec 2014 14:25:37 +0200 Subject: [PATCH 21/31] Fix UI for no-ssh-key message Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/generic/common.scss | 20 -------------------- app/views/layouts/project_settings.html.haml | 3 --- app/views/layouts/projects.html.haml | 2 -- app/views/projects/show.html.haml | 3 +++ app/views/shared/_no_ssh.html.haml | 20 +++++++------------- 5 files changed, 10 insertions(+), 38 deletions(-) diff --git a/app/assets/stylesheets/generic/common.scss b/app/assets/stylesheets/generic/common.scss index dfc7b0de9b7..6c37cbf072e 100644 --- a/app/assets/stylesheets/generic/common.scss +++ b/app/assets/stylesheets/generic/common.scss @@ -207,26 +207,6 @@ li.note { } } -.no-ssh-key-message { - padding: 10px 0; - background: #C67; - margin: 0; - color: #FFF; - margin-top: -1px; - text-align: center; - - a { - color: #fff; - text-decoration: underline; - } - - .links-xs { - text-align: center; - font-size: 16px; - padding: 5px; - } -} - .warning_message { border-left: 4px solid #ed9; color: #b90; diff --git a/app/views/layouts/project_settings.html.haml b/app/views/layouts/project_settings.html.haml index 810fb4e2005..0f20bf38bfd 100644 --- a/app/views/layouts/project_settings.html.haml +++ b/app/views/layouts/project_settings.html.haml @@ -5,8 +5,5 @@ = render "layouts/broadcast" = render "layouts/head_panel", title: project_title(@project) = render "layouts/init_auto_complete" - - if can?(current_user, :download_code, @project) - = render 'shared/no_ssh' - - @project_settings_nav = true = render 'layouts/page', sidebar: 'layouts/nav/project' diff --git a/app/views/layouts/projects.html.haml b/app/views/layouts/projects.html.haml index b4b1bcf241c..d4ee53db55c 100644 --- a/app/views/layouts/projects.html.haml +++ b/app/views/layouts/projects.html.haml @@ -5,6 +5,4 @@ = render "layouts/broadcast" = render "layouts/head_panel", title: project_title(@project) = render "layouts/init_auto_complete" - - if can?(current_user, :download_code, @project) - = render 'shared/no_ssh' = render 'layouts/page', sidebar: 'layouts/nav/project' diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index 9b06ebe95a4..14d1ad956e3 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -1,3 +1,6 @@ +- if can?(current_user, :download_code, @project) + = render 'shared/no_ssh' + = render "home_panel" - readme = @repository.readme diff --git a/app/views/shared/_no_ssh.html.haml b/app/views/shared/_no_ssh.html.haml index e70eb4d01b9..e1c2a962982 100644 --- a/app/views/shared/_no_ssh.html.haml +++ b/app/views/shared/_no_ssh.html.haml @@ -1,14 +1,8 @@ - if cookies[:hide_no_ssh_message].blank? && current_user.require_ssh_key? && !current_user.hide_no_ssh_key - .no-ssh-key-message - .container - You won't be able to pull or push project code via SSH until you #{link_to 'add an SSH key', new_profile_key_path} to your profile - .pull-right.hidden-xs - = link_to "Don't show again", profile_path(user: {hide_no_ssh_key: true}), method: :put, class: 'hide-no-ssh-message', remote: true - | - = link_to 'Remind later', '#', class: 'hide-no-ssh-message' - .links-xs.visible-xs - = link_to "Add key", new_profile_key_path - | - = link_to "Don't show again", profile_path(user: {hide_no_ssh_key: true}), method: :put, class: 'hide-no-ssh-message', remote: true - | - = link_to 'Later', '#', class: 'hide-no-ssh-message' + .no-ssh-key-message.alert.alert-warning.hidden-xs + You won't be able to pull or push project code via SSH until you #{link_to 'add an SSH key', new_profile_key_path} to your profile + + .pull-right + = link_to "Don't show again", profile_path(user: {hide_no_ssh_key: true}), method: :put, class: 'hide-no-ssh-message', remote: true + | + = link_to 'Remind later', '#', class: 'hide-no-ssh-message' From a248efadf7a4a8e2ebf0a98413c195b3c8766f20 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 26 Dec 2014 14:28:49 +0200 Subject: [PATCH 22/31] Fix links for no-ssh message Signed-off-by: Dmitriy Zaporozhets --- app/views/shared/_no_ssh.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/shared/_no_ssh.html.haml b/app/views/shared/_no_ssh.html.haml index e1c2a962982..8e6f802fd3b 100644 --- a/app/views/shared/_no_ssh.html.haml +++ b/app/views/shared/_no_ssh.html.haml @@ -3,6 +3,6 @@ You won't be able to pull or push project code via SSH until you #{link_to 'add an SSH key', new_profile_key_path} to your profile .pull-right - = link_to "Don't show again", profile_path(user: {hide_no_ssh_key: true}), method: :put, class: 'hide-no-ssh-message', remote: true + = link_to "Don't show again", profile_path(user: {hide_no_ssh_key: true}), method: :put | = link_to 'Remind later', '#', class: 'hide-no-ssh-message' From 573d554c6927f0e6804c986af7d8837e0abd6cd9 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 26 Dec 2014 14:34:50 +0200 Subject: [PATCH 23/31] set z-index for navbar and sidebar explicitly Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/sections/header.scss | 3 +-- app/assets/stylesheets/sections/sidebar.scss | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/sections/header.scss b/app/assets/stylesheets/sections/header.scss index db419f76532..f71b62ace9c 100644 --- a/app/assets/stylesheets/sections/header.scss +++ b/app/assets/stylesheets/sections/header.scss @@ -4,6 +4,7 @@ */ header { &.navbar-gitlab { + z-index: 100; margin-bottom: 0; min-height: 40px; border: none; @@ -82,8 +83,6 @@ header { } } - z-index: 10; - .container { width: 100% !important; padding-left: 0px; diff --git a/app/assets/stylesheets/sections/sidebar.scss b/app/assets/stylesheets/sections/sidebar.scss index 51d6b2c920c..fdf9eb86d46 100644 --- a/app/assets/stylesheets/sections/sidebar.scss +++ b/app/assets/stylesheets/sections/sidebar.scss @@ -3,6 +3,7 @@ } .sidebar-wrapper { + z-index: 99; overflow-y: auto; background: #F5F5F5; } From aa54482a37df5b122e35029a316094c2f55d5044 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 26 Dec 2014 14:53:50 +0200 Subject: [PATCH 24/31] Fix no-ssh message for non logged in user Signed-off-by: Dmitriy Zaporozhets --- app/views/projects/show.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index 14d1ad956e3..af6e4567c1b 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -1,4 +1,4 @@ -- if can?(current_user, :download_code, @project) +- if current_user && can?(current_user, :download_code, @project) = render 'shared/no_ssh' = render "home_panel" From 1b6ebd17179b610f832d5a1cfda866167124bb1c Mon Sep 17 00:00:00 2001 From: Stephan van Leeuwen Date: Fri, 26 Dec 2014 15:29:02 +0100 Subject: [PATCH 25/31] Updated sidebar style to span the whole height. --- app/assets/stylesheets/sections/sidebar.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/sections/sidebar.scss b/app/assets/stylesheets/sections/sidebar.scss index 51d6b2c920c..b836e566a10 100644 --- a/app/assets/stylesheets/sections/sidebar.scss +++ b/app/assets/stylesheets/sections/sidebar.scss @@ -104,10 +104,11 @@ .sidebar-wrapper { width: 250px; - position: absolute; + position: fixed; left: 250px; height: 100%; margin-left: -250px; + border-right: 1px solid #EAEAEA; .nav-sidebar { margin-top: 20px; @@ -119,7 +120,6 @@ .content-wrapper { padding: 20px; - border-left: 1px solid #EAEAEA; } } From 071ad02c027ac14e0944e957664fc24c82b720c3 Mon Sep 17 00:00:00 2001 From: Stephan van Leeuwen Date: Fri, 26 Dec 2014 15:29:19 +0100 Subject: [PATCH 26/31] Updated mobile sidebar to allow scrolling --- app/assets/stylesheets/sections/sidebar.scss | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/sections/sidebar.scss b/app/assets/stylesheets/sections/sidebar.scss index b836e566a10..697e9d20231 100644 --- a/app/assets/stylesheets/sections/sidebar.scss +++ b/app/assets/stylesheets/sections/sidebar.scss @@ -130,14 +130,16 @@ .sidebar-wrapper { width: 52px; - position: absolute; + position: fixed; left: 50px; height: 100%; margin-left: -50px; + border-right: 1px solid #EAEAEA; + overflow-x: hidden; .nav-sidebar { margin-top: 20px; - position: fixed; + position: absolute; top: 45px; width: 52px; From 465f186954d00fa47c8b05cc91f33e7943aa209a Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 26 Dec 2014 18:33:53 +0200 Subject: [PATCH 27/31] Show assigned issues/mr be default on dashboard This was default before but now it fixed with providing assignee_id parameter making url shareble and dont reset when other filters users. Also this commit removes old methods that are not used any more. Signed-off-by: Dmitriy Zaporozhets --- app/controllers/application_controller.rb | 4 --- app/helpers/dashboard_helper.rb | 38 +++------------------- app/views/layouts/nav/_dashboard.html.haml | 4 +-- features/steps/shared/paths.rb | 5 +-- 4 files changed, 9 insertions(+), 42 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 41ad5f98ace..4b8cae469e3 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -257,10 +257,6 @@ class ApplicationController < ActionController::Base # or improve current implementation to filter only issues you # created or assigned or mentioned #@filter_params[:authorized_only] = true - - unless @filter_params[:assignee_id] - @filter_params[:assignee_id] = current_user.id - end end @filter_params diff --git a/app/helpers/dashboard_helper.rb b/app/helpers/dashboard_helper.rb index 976a396e7b6..3e6f3b41ff5 100644 --- a/app/helpers/dashboard_helper.rb +++ b/app/helpers/dashboard_helper.rb @@ -1,13 +1,4 @@ module DashboardHelper - def entities_per_project(project, entity) - case entity.to_sym - when :issue then @issues.where(project_id: project.id) - when :merge_request then @merge_requests.where(target_project_id: project.id) - else - [] - end.count - end - def projects_dashboard_filter_path(options={}) exist_opts = { sort: params[:sort], @@ -22,32 +13,11 @@ module DashboardHelper path end - def assigned_entities_count(current_user, entity, scope = nil) - items = current_user.send('assigned_' + entity.pluralize) - get_count(items, scope) + def assigned_issues_dashboard_path + issues_dashboard_path(assignee_id: current_user.id) end - def authored_entities_count(current_user, entity, scope = nil) - items = current_user.send(entity.pluralize) - get_count(items, scope) - end - - def authorized_entities_count(current_user, entity, scope = nil) - items = entity.classify.constantize - get_count(items, scope, true, current_user) - end - - protected - - def get_count(items, scope, get_authorized = false, current_user = nil) - items = items.opened - if scope.kind_of?(Group) - items = items.of_group(scope) - elsif scope.kind_of?(Project) - items = items.of_projects(scope) - elsif get_authorized - items = items.of_projects(current_user.authorized_projects) - end - items.count + def assigned_mrs_dashboard_path + merge_requests_dashboard_path(assignee_id: current_user.id) end end diff --git a/app/views/layouts/nav/_dashboard.html.haml b/app/views/layouts/nav/_dashboard.html.haml index da1976346d5..a2eaa2d83c5 100644 --- a/app/views/layouts/nav/_dashboard.html.haml +++ b/app/views/layouts/nav/_dashboard.html.haml @@ -10,13 +10,13 @@ %span Projects = nav_link(path: 'dashboard#issues') do - = link_to issues_dashboard_path, class: 'shortcuts-issues' do + = link_to assigned_issues_dashboard_path, class: 'shortcuts-issues' do %i.fa.fa-exclamation-circle %span Issues %span.count= current_user.assigned_issues.opened.count = nav_link(path: 'dashboard#merge_requests') do - = link_to merge_requests_dashboard_path, class: 'shortcuts-merge_requests' do + = link_to assigned_mrs_dashboard_path, class: 'shortcuts-merge_requests' do %i.fa.fa-tasks %span Merge Requests diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index ca038732231..b60d290ae9c 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -1,6 +1,7 @@ module SharedPaths include Spinach::DSL include RepoHelpers + include DashboardHelper step 'I visit new project page' do visit new_project_path @@ -71,11 +72,11 @@ module SharedPaths end step 'I visit dashboard issues page' do - visit issues_dashboard_path + visit assigned_issues_dashboard_path end step 'I visit dashboard merge requests page' do - visit merge_requests_dashboard_path + visit assigned_mrs_dashboard_path end step 'I visit dashboard search page' do From 4adc033761db149e5bb46f4be02788f1fd384b20 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sat, 27 Dec 2014 16:03:02 +0200 Subject: [PATCH 28/31] Improve UI for group milestone and project milestone pages Signed-off-by: Dmitriy Zaporozhets --- app/views/groups/milestones/show.html.haml | 70 +++++++++--------- app/views/projects/milestones/show.html.haml | 76 ++++++++++---------- 2 files changed, 71 insertions(+), 75 deletions(-) diff --git a/app/views/groups/milestones/show.html.haml b/app/views/groups/milestones/show.html.haml index 411d1822be0..7bcac56c37b 100644 --- a/app/views/groups/milestones/show.html.haml +++ b/app/views/groups/milestones/show.html.haml @@ -1,4 +1,9 @@ -%h3.page-title +%h4.page-title + .issue-box{ class: "issue-box-#{@group_milestone.closed? ? 'closed' : 'open'}" } + - if @group_milestone.closed? + Closed + - else + Open Milestone #{@group_milestone.title} .pull-right - if can?(current_user, :manage_group, @group) @@ -7,46 +12,41 @@ - else = link_to 'Reopen Milestone', group_milestone_path(@group, @group_milestone.safe_title, title: @group_milestone.title, milestone: {state_event: :activate }), method: :put, class: "btn btn-small btn-grouped btn-reopen" +%hr - if (@group_milestone.total_items_count == @group_milestone.closed_items_count) && @group_milestone.active? .alert.alert-success %span All issues for this milestone are closed. You may close the milestone now. -.back-link - = link_to group_milestones_path(@group) do - ← To milestones list - -.issue-box{ class: "issue-box-#{@group_milestone.closed? ? 'closed' : 'open'}" } - .state.clearfix - .state-label - - if @group_milestone.closed? - Closed - - else - Open - - %h4.title - = gfm escape_once(@group_milestone.title) - - .description - - @group_milestone.milestones.each do |milestone| - %hr - %h4 - = link_to "#{milestone.project.name} - #{milestone.title}", project_milestone_path(milestone.project, milestone) - %span.pull-right= milestone.expires_at +.description +%table.table + %thead + %tr + %th Project + %th Open issues + %th State + %th Due date + - @group_milestone.milestones.each do |milestone| + %tr + %td + = link_to "#{milestone.project.name}", project_milestone_path(milestone.project, milestone) + %td + = milestone.issues.opened.count + %td - if milestone.closed? - %span.label.label-danger #{milestone.state} - = preserve do - - if milestone.description.present? - = milestone.description + Closed + - else + Open + %td + = milestone.expires_at - .context - %p - Progress: - #{@group_milestone.closed_items_count} closed - – - #{@group_milestone.open_items_count} open - - .progress.progress-info - .progress-bar{style: "width: #{@group_milestone.percent_complete}%;"} +.context + %p.lead + Progress: + #{@group_milestone.closed_items_count} closed + – + #{@group_milestone.open_items_count} open + .progress.progress-info + .progress-bar{style: "width: #{@group_milestone.percent_complete}%;"} %ul.nav.nav-tabs %li.active diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml index cd62e4811ac..031b5a31895 100644 --- a/app/views/projects/milestones/show.html.haml +++ b/app/views/projects/milestones/show.html.haml @@ -1,5 +1,5 @@ = render "projects/issues_nav" -%h3.page-title +%h4.page-title .issue-box{ class: issue_box_class(@milestone) } - if @milestone.closed? Closed @@ -8,52 +8,44 @@ - else Open Milestone ##{@milestone.iid} - .pull-right.creator - %small= @milestone.expires_at + %small.creator + = @milestone.expires_at + .pull-right + - if can?(current_user, :admin_milestone, @project) + = link_to edit_project_milestone_path(@project, @milestone), class: "btn btn-grouped" do + %i.fa.fa-pencil-square-o + Edit + - if @milestone.active? + = link_to 'Close Milestone', project_milestone_path(@project, @milestone, milestone: {state_event: :close }), method: :put, class: "btn btn-close btn-grouped" + - else + = link_to 'Reopen Milestone', project_milestone_path(@project, @milestone, milestone: {state_event: :activate }), method: :put, class: "btn btn-reopen btn-grouped" %hr - if @milestone.issues.any? && @milestone.can_be_closed? .alert.alert-success %span All issues for this milestone are closed. You may close milestone now. -.row - .col-sm-9 - %h3.issue-title - = gfm escape_once(@milestone.title) - %div - - if @milestone.description.present? - .description - .wiki - = preserve do - = markdown @milestone.description - %hr - .context - %p.lead - Progress: - #{@milestone.closed_items_count} closed - – - #{@milestone.open_items_count} open -   - %span.light #{@milestone.percent_complete}% complete - %span.pull-right= @milestone.expires_at - .progress.progress-info - .progress-bar{style: "width: #{@milestone.percent_complete}%;"} - - .col-sm-3 - %div - - if can?(current_user, :admin_milestone, @project) - = link_to edit_project_milestone_path(@project, @milestone), class: "btn btn-block" do - %i.fa.fa-pencil-square-o - Edit - - if @milestone.active? - = link_to 'Close Milestone', project_milestone_path(@project, @milestone, milestone: {state_event: :close }), method: :put, class: "btn btn-close btn-block" - - else - = link_to 'Reopen Milestone', project_milestone_path(@project, @milestone, milestone: {state_event: :activate }), method: :put, class: "btn btn-reopen btn-block" - = link_to new_project_issue_path(@project, issue: { milestone_id: @milestone.id }), class: "btn btn-block", title: "New Issue" do - %i.fa.fa-plus - New Issue - = link_to 'Browse Issues', project_issues_path(@milestone.project, milestone_id: @milestone.id), class: "btn edit-milestone-link btn-block" +%h3.issue-title + = gfm escape_once(@milestone.title) +%div + - if @milestone.description.present? + .description + .wiki + = preserve do + = markdown @milestone.description +%hr +.context + %p.lead + Progress: + #{@milestone.closed_items_count} closed + – + #{@milestone.open_items_count} open +   + %span.light #{@milestone.percent_complete}% complete + %span.pull-right= @milestone.expires_at + .progress.progress-info + .progress-bar{style: "width: #{@milestone.percent_complete}%;"} %ul.nav.nav-tabs @@ -71,6 +63,10 @@ %span.badge= @users.count .pull-right + = link_to new_project_issue_path(@project, issue: { milestone_id: @milestone.id }), class: "btn btn-grouped", title: "New Issue" do + %i.fa.fa-plus + New Issue + = link_to 'Browse Issues', project_issues_path(@milestone.project, milestone_id: @milestone.id), class: "btn edit-milestone-link btn-grouped" .tab-content .tab-pane.active#tab-issues From b8bffc0da26ff53a220ec83e71a195666867d28f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sat, 27 Dec 2014 17:48:10 +0200 Subject: [PATCH 29/31] Dont check for milestone description on group milestone page Signed-off-by: Dmitriy Zaporozhets --- features/steps/groups.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/features/steps/groups.rb b/features/steps/groups.rb index 0bd7a32f5cb..f09d751dba3 100644 --- a/features/steps/groups.rb +++ b/features/steps/groups.rb @@ -188,7 +188,6 @@ class Spinach::Features::Groups < Spinach::FeatureSteps end step 'I should see group milestone with descriptions and expiry date' do - page.should have_content('Lorem Ipsum is simply dummy text of the printing and typesetting industry') page.should have_content('expires at Aug 20, 2114') end From 6342bc299457a2b298e8cb556bcd55efe1bbe030 Mon Sep 17 00:00:00 2001 From: Stephan van Leeuwen Date: Sat, 27 Dec 2014 20:08:29 +0100 Subject: [PATCH 30/31] Changed header to stay at the top of the page. --- app/assets/stylesheets/generic/common.scss | 2 +- app/assets/stylesheets/sections/header.scss | 3 +++ app/assets/stylesheets/sections/sidebar.scss | 5 +++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/generic/common.scss b/app/assets/stylesheets/generic/common.scss index 2fc738c18d8..fa25757cfc3 100644 --- a/app/assets/stylesheets/generic/common.scss +++ b/app/assets/stylesheets/generic/common.scss @@ -282,7 +282,7 @@ img.emoji { } .navless-container { - margin-top: 20px; + margin-top: 68px; } .description-block { diff --git a/app/assets/stylesheets/sections/header.scss b/app/assets/stylesheets/sections/header.scss index db419f76532..33a37ee6d7a 100644 --- a/app/assets/stylesheets/sections/header.scss +++ b/app/assets/stylesheets/sections/header.scss @@ -7,6 +7,9 @@ header { margin-bottom: 0; min-height: 40px; border: none; + position: fixed; + top: 0; + width: 100%; .navbar-inner { filter: none; diff --git a/app/assets/stylesheets/sections/sidebar.scss b/app/assets/stylesheets/sections/sidebar.scss index 697e9d20231..d03f73f2872 100644 --- a/app/assets/stylesheets/sections/sidebar.scss +++ b/app/assets/stylesheets/sections/sidebar.scss @@ -11,6 +11,7 @@ width: 100%; padding: 15px; background: #FFF; + margin-top: 48px; } .nav-sidebar { @@ -131,9 +132,9 @@ .sidebar-wrapper { width: 52px; position: fixed; - left: 50px; + top: 0; + left: 0; height: 100%; - margin-left: -50px; border-right: 1px solid #EAEAEA; overflow-x: hidden; From 05a6115bc8f9ae3e3b41a155334adb6f73d5a0cc Mon Sep 17 00:00:00 2001 From: Ciro Santilli Date: Sat, 27 Dec 2014 21:43:43 +0100 Subject: [PATCH 31/31] doc workflow markdown style - add h1 to README - move h1 in workflow.md to h2 since the top image acts as h1 - typos --- doc/workflow/README.md | 2 ++ doc/workflow/gitlab_flow.md | 42 ++++++++++++++++++------------------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/doc/workflow/README.md b/doc/workflow/README.md index c26d85e9955..f0e0f51b1ac 100644 --- a/doc/workflow/README.md +++ b/doc/workflow/README.md @@ -1,3 +1,5 @@ +# Workflow + - [Workflow](workflow.md) - [Project Features](project_features.md) - [Authorization for merge requests](authorization_for_merge_requests.md) diff --git a/doc/workflow/gitlab_flow.md b/doc/workflow/gitlab_flow.md index f8fd7c97e2a..1dbff60cbfd 100644 --- a/doc/workflow/gitlab_flow.md +++ b/doc/workflow/gitlab_flow.md @@ -1,6 +1,6 @@ ![GitLab Flow](gitlab_flow.png) -# Introduction +## Introduction Version management with git makes branching and merging much easier than older versioning systems such as SVN. This allows a wide variety of branching strategies and workflows. @@ -29,9 +29,9 @@ People have a hard time figuring out which branch they should develop on or depl Frequently the reaction to this problem is to adopt a standardized pattern such as [git flow](http://nvie.com/posts/a-successful-git-branching-model/) and [GitHub flow](http://scottchacon.com/2011/08/31/github-flow.html) We think there is still room for improvement and will detail a set of practices we call GitLab flow. -# Git flow and its problems +## Git flow and its problems -[![Git Flow timeline by Vincent Driessen, used with persmission](gitdashflow.png) +[![Git Flow timeline by Vincent Driessen, used with permission](gitdashflow.png) Git flow was one of the first proposals to use git branches and it has gotten a lot of attention. It advocates a master branch and a separate develop branch as well as supporting branches for features, releases and hotfixes. @@ -50,7 +50,7 @@ Frequently developers make a mistake and for example changes are only merged int The root cause of these errors is that git flow is too complex for most of the use cases. And doing releases doesn't automatically mean also doing hotfixes. -# GitHub flow as a simpler alternative +## GitHub flow as a simpler alternative ![Master branch with feature branches merged in](github_flow.png) @@ -62,13 +62,13 @@ Merging everything into the master branch and deploying often means you minimize But this flow still leaves a lot of questions unanswered regarding deployments, environments, releases and integrations with issues. With GitLab flow we offer additional guidance for these questions. -# Production branch with GitLab flow +## Production branch with GitLab flow ![Master branch and production branch with arrow that indicate deployments](production_branch.png) GitHub flow does assume you are able to deploy to production every time you merge a feature branch. This is possible for SaaS applications but are many cases where this is not possible. -One would be a situation where you are not in control of the exact release moment, for example an iOS application that needs to pass AppStore validation. +One would be a situation where you are not in control of the exact release moment, for example an iOS application that needs to pass App Store validation. Another example is when you have deployment windows (workdays from 10am to 4pm when the operations team is at full capacity) but you also merge code at other times. In these cases you can make a production branch that reflects the deployed code. You can deploy a new version by merging in master to the production branch. @@ -78,7 +78,7 @@ This time is pretty accurate if you automatically deploy your production branch. If you need a more exact time you can have your deployment script create a tag on each deployment. This flow prevents the overhead of releasing, tagging and merging that is common to git flow. -# Environment branches with GitLab flow +## Environment branches with GitLab flow ![Multiple branches with the code cascading from one to another](environment_branches.png) @@ -93,7 +93,7 @@ If master is good to go (it should be if you a practicing [continuous delivery]( If this is not possible because more manual testing is required you can send merge requests from the feature branch to the downstream branches. An 'extreme' version of environment branches are setting up an environment for each feature branch as done by [Teatro](http://teatro.io/). -# Release branches with GitLab flow +## Release branches with GitLab flow ![Master and multiple release branches that vary in length with cherrypicks from master](release_branches.png) @@ -109,7 +109,7 @@ Every time a bug-fix is included in a release branch the patch version is raised Some projects also have a stable branch that points to the same commit as the latest released branch. In this flow it is not common to have a production branch (or git flow master branch). -# Merge/pull requests with GitLab flow +## Merge/pull requests with GitLab flow ![Merge request with line comments](mr_inline_comments.png) @@ -134,7 +134,7 @@ If the assigned person does not feel comfortable they can close the merge reques In GitLab it is common to protect the long-lived branches (e.g. the master branch) so that normal developers [can't modify these protected branches](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/permissions/permissions.md). So if you want to merge it into a protected branch you assign it to someone with master authorizations. -# Issues with GitLab flow +## Issues with GitLab flow ![Merge request with the branch name 15-require-a-password-to-change-it and assignee field shown](merge_request.png) @@ -168,7 +168,7 @@ In this case it is no problem to reuse the same branch name since it was deleted At any time there is at most one branch for every issue. It is possible that one feature branch solves more than one issue. -# Linking and closing issues from merge requests +## Linking and closing issues from merge requests ![Merge request showing the linked issues that will be closed](close_issue_mr.png) @@ -181,7 +181,7 @@ If you only want to make the reference without closing the issue you can also ju If you have an issue that spans across multiple repositories, the best thing is to create an issue for each repository and link all issues to a parent issue. -# Squashing commits with rebase +## Squashing commits with rebase ![Vim screen showing the rebase view](rebase.png) @@ -189,7 +189,7 @@ With git you can use an interactive rebase (rebase -i) to squash multiple commit This functionality is useful if you made a couple of commits for small changes during development and want to replace them with a single commit or if you want to make the order more logical. However you should never rebase commits you have pushed to a remote server. Somebody can have referred to the commits or cherry-picked them. -When you rebase you change the identifier (SHA1) of the commit and this is confusing. +When you rebase you change the identifier (SHA-1) of the commit and this is confusing. If you do that the same change will be known under multiple identifiers and this can cause much confusion. If people already reviewed your code it will be hard for them to review only the improvements you made since then if you have rebased everything into one commit. @@ -207,7 +207,7 @@ If you revert a merge and you change your mind, revert the revert instead of mer Being able to revert a merge is a good reason always to create a merge commit when you merge manually with the `--no-ff` option. Git management software will always create a merge commit when you accept a merge request. -# Do not order commits with rebase +## Do not order commits with rebase ![List of sequential merge commits](merge_commits.png) @@ -231,8 +231,8 @@ The last reason for creating merge commits is having long lived branches that yo Martin Fowler, in [his article about feature branches](http://martinfowler.com/bliki/FeatureBranch.html) talks about this Continuous Integration (CI). At GitLab we are guilty of confusing CI with branch testing. Quoting Martin Fowler: "I've heard people say they are doing CI because they are running builds, perhaps using a CI server, on every branch with every commit. That's continuous building, and a Good Thing, but there's no integration, so it's not CI.". -The solution to prevent many merge commits is to keep your feature branches shortlived, the vast majority should take less than one day of work. -If your feature branches commenly take more than a day of work, look into ways to create smaller units of work and/or use [feature toggles](http://martinfowler.com/bliki/FeatureToggle.html). +The solution to prevent many merge commits is to keep your feature branches short-lived, the vast majority should take less than one day of work. +If your feature branches commonly take more than a day of work, look into ways to create smaller units of work and/or use [feature toggles](http://martinfowler.com/bliki/FeatureToggle.html). As for the long running branches that take more than one day there are two strategies. In a CI strategy you can merge in master at the start of the day to prevent painful merges at a later time. In a synchronization point strategy you only merge in from well defined points in time, for example a tagged release. @@ -244,7 +244,7 @@ Developing software happen in small messy steps and it is OK to have your histor You can use tools to view the network graphs of commits and understand the messy history that created your code. If you rebase code the history is incorrect, and there is no way for tools to remedy this because they can't deal with changing commit identifiers. -# Voting on merge requests +## Voting on merge requests ![Voting slider in GitLab](voting_slider.png) @@ -252,7 +252,7 @@ It is common to voice approval or disapproval by using +1 or -1 emoticons. In GitLab the +1 and -1 are aggregated and shown at the top of the merge request. As a rule of thumb anything that doesn't have two times more +1's than -1's is suspect and should not be merged yet. -# Pushing and removing branches +## Pushing and removing branches ![Remove checkbox for branch in merge requests](remove_checkbox.png) @@ -266,7 +266,7 @@ This ensures that the branch overview in the repository management software show This also ensures that when someone reopens the issue a new branch with the same name can be used without problem. When you reopen an issue you need to create a new merge request. -# Committing often and with the right message +## Committing often and with the right message ![Good and bad commit message](good_commit.png) @@ -282,7 +282,7 @@ Some words that are bad commit messages because they don't contain munch informa The word fix or fixes is also a red flag, unless it comes after the commit sentence and references an issue number. To see more information about the formatting of commit messages please see this great [blog post by Tim Pope](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html). -# Testing before merging +## Testing before merging ![Merge requests showing the test states, red, yellow and green](ci_mr.png) @@ -299,7 +299,7 @@ If there are no merge conflicts and the feature branches are short lived the ris If there are merge conflicts you merge the master branch into the feature branch and the CI server will rerun the tests. If you have long lived feature branches that last for more than a few days you should make your issues smaller. -# Merging in other code +## Merging in other code ![Shell output showing git pull output](git_pull.png)