diff options
23 files changed, 182 insertions, 40 deletions
diff --git a/CHANGELOG b/CHANGELOG index a1a7f443845..66d23dcfd4f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,8 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.12.0 (unreleased) - Shorten merge request WIP text. + - Add option to disallow users from registering any application to use GitLab as an OAuth provider + - Support editing target branch of merge request (Stan Hu) - Refactor permission checks with issues and merge requests project settings (Stan Hu) - Fix Markdown preview not working in Edit Milestone page (Stan Hu) - Fix Zen Mode not closing with ESC key (Stan Hu) @@ -30,6 +32,7 @@ v 7.12.0 (unreleased) - Clarify navigation labels for Project Settings and Group Settings. - Move user avatar and logout button to sidebar - You can not remove user if he/she is an only owner of group + - User should be able to leave group. If not - show him proper message - User has ability to leave project v 7.11.4 diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 4c35622fff1..5aaae94e6bf 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -43,6 +43,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :default_snippet_visibility, :restricted_signup_domains_raw, :version_check_enabled, + :user_oauth_applications, restricted_visibility_levels: [], ) end diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index a11c554a2af..040255f08e6 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -66,7 +66,11 @@ class Groups::GroupMembersController < Groups::ApplicationController @group_member.destroy redirect_to(dashboard_groups_path, notice: "You left #{group.name} group.") else - return render_403 + if @group.last_owner?(current_user) + redirect_to(dashboard_groups_path, alert: "You can not leave #{group.name} group because you're the last owner. Transfer or delete the group.") + else + return render_403 + end end end diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb index 507b8290a2b..fc31118124b 100644 --- a/app/controllers/oauth/applications_controller.rb +++ b/app/controllers/oauth/applications_controller.rb @@ -1,6 +1,8 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController + include Gitlab::CurrentSettings include PageLayoutHelper + before_action :verify_user_oauth_applications_enabled before_action :authenticate_user! layout 'profile' @@ -32,6 +34,12 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController private + def verify_user_oauth_applications_enabled + return if current_application_settings.user_oauth_applications? + + redirect_to applications_profile_url + end + def set_application @application = current_user.oauth_applications.find(params[:id]) end diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 241d6075c9f..63c3ff5674d 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -19,6 +19,10 @@ module ApplicationSettingsHelper current_application_settings.sign_in_text end + def user_oauth_applications? + current_application_settings.user_oauth_applications + end + # Return a group of checkboxes that use Bootstrap's button plugin for a # toggle button effect. def restricted_level_checkboxes(help_block_id) diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index d5123249c53..c465158f764 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -18,6 +18,7 @@ # default_project_visibility :integer # default_snippet_visibility :integer # restricted_signup_domains :text +# user_oauth_applications :bool default(TRUE) # class ApplicationSetting < ActiveRecord::Base diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5690c375b96..f1f9f23b12c 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -197,7 +197,6 @@ class MergeRequest < ActiveRecord::Base def update_merge_request_diff if source_branch_changed? || target_branch_changed? reload_code - mark_as_unchecked end end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index c5769a5ad27..1d99223cfe6 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -20,4 +20,10 @@ class IssuableBaseService < BaseService SystemNoteService.change_title( issuable, issuable.project, current_user, old_title) end + + def create_branch_change_note(issuable, branch_type, old_branch, new_branch) + SystemNoteService.change_branch( + issuable, issuable.project, current_user, branch_type, + old_branch, new_branch) + end end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 34fd59d6927..34c190bf621 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -5,7 +5,7 @@ require_relative 'close_service' module MergeRequests class UpdateService < MergeRequests::BaseService def execute(merge_request) - # We dont allow change of source/target projects + # We don't allow change of source/target projects # after merge request was created params.except!(:source_project_id) params.except!(:target_project_id) @@ -41,6 +41,12 @@ module MergeRequests ) end + if merge_request.previous_changes.include?('target_branch') + create_branch_change_note(merge_request, 'target', + merge_request.previous_changes['target_branch'].first, + merge_request.target_branch) + end + if merge_request.previous_changes.include?('milestone_id') create_milestone_note(merge_request) end @@ -54,6 +60,11 @@ module MergeRequests create_title_change_note(merge_request, merge_request.previous_changes['title'].first) end + if merge_request.previous_changes.include?('target_branch') || + merge_request.previous_changes.include?('source_branch') + merge_request.mark_as_unchecked + end + merge_request.notice_added_references(merge_request.project, current_user) execute_hooks(merge_request, 'update') end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 1527ae0486d..b6801a92330 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -149,6 +149,25 @@ class SystemNoteService create_note(noteable: noteable, project: project, author: author, note: body) end + # Called when a branch in Noteable is changed + # + # noteable - Noteable object + # project - Project owning noteable + # author - User performing the change + # branch_type - 'source' or 'target' + # old_branch - old branch name + # new_branch - new branch nmae + # + # Example Note text: + # + # "Target branch changed from `Old` to `New`" + # + # Returns the created Note object + def self.change_branch(noteable, project, author, branch_type, old_branch, new_branch) + body = "#{branch_type} branch changed from `#{old_branch}` to `#{new_branch}`".capitalize + create_note(noteable: noteable, project: project, author: author, note: body) + end + # Called when a Mentionable references a Noteable # # noteable - Noteable object being referenced diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 4ceae814805..dd8978647c4 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -30,7 +30,7 @@ .checkbox = f.label :twitter_sharing_enabled do = f.check_box :twitter_sharing_enabled, :'aria-describedby' => 'twitter_help_block' - %strong Twitter enabled + Twitter enabled %span.help-block#twitter_help_block Show users a button to share their newly created public or internal projects on twitter .form-group .col-sm-offset-2.col-sm-10 @@ -83,6 +83,13 @@ .col-sm-10 = f.text_area :restricted_signup_domains_raw, placeholder: 'domain.com', class: 'form-control' .help-block Only users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com + .form_group + = f.label :user_oauth_applications, 'User OAuth applications', class: 'control-label col-sm-2' + .col-sm-10 + .checkbox + = f.label :user_oauth_applications do + = f.check_box :user_oauth_applications + Allow users to register any application to use GitLab as an OAuth provider .form-actions = f.submit 'Save', class: 'btn btn-primary' diff --git a/app/views/dashboard/groups/index.html.haml b/app/views/dashboard/groups/index.html.haml index 5ecd53cff84..cfb386e131f 100644 --- a/app/views/dashboard/groups/index.html.haml +++ b/app/views/dashboard/groups/index.html.haml @@ -23,10 +23,9 @@ %i.fa.fa-cogs Settings - - if can?(current_user, :destroy_group_member, group_member) - = link_to leave_group_group_members_path(group), data: { confirm: leave_group_message(group.name) }, method: :delete, class: "btn-sm btn btn-grouped", title: 'Remove user from group' do - %i.fa.fa-sign-out - Leave + = link_to leave_group_group_members_path(group), data: { confirm: leave_group_message(group.name) }, method: :delete, class: "btn-sm btn btn-grouped", title: 'Leave this group' do + %i.fa.fa-sign-out + Leave = image_tag group_icon(group), class: "avatar s40 avatar-tile" = link_to group, class: 'group-name' do diff --git a/app/views/profiles/applications.html.haml b/app/views/profiles/applications.html.haml index c145a9b7f6d..2c4f0804f0b 100644 --- a/app/views/profiles/applications.html.haml +++ b/app/views/profiles/applications.html.haml @@ -2,37 +2,43 @@ %h3.page-title = page_title %p.light - OAuth2 protocol settings below. + - if user_oauth_applications? + Manage applications that can use GitLab as an OAuth provider, + and applications that you've authorized to use your account. + - else + Manage applications that you've authorized to use your account. %hr -.oauth-applications - %h3 - Your applications - .pull-right - = 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-sm' - %td= render 'doorkeeper/applications/delete_form', application: application +- if user_oauth_applications? + .oauth-applications + %h3 + Your applications + .pull-right + = 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-sm' + %td= render 'doorkeeper/applications/delete_form', application: application .oauth-authorized-applications.prepend-top-20 - %h3 - Authorized applications + - if user_oauth_applications? + %h3 + Authorized applications - if @authorized_tokens.any? %table.table.table-striped diff --git a/app/views/projects/_issuable_form.html.haml b/app/views/projects/_issuable_form.html.haml index c85da8eff98..2c5b24b8130 100644 --- a/app/views/projects/_issuable_form.html.haml +++ b/app/views/projects/_issuable_form.html.haml @@ -79,6 +79,24 @@ - if can? current_user, :admin_label, issuable.project = link_to 'Create new label', new_namespace_project_label_path(issuable.project.namespace, issuable.project), target: :blank +- if issuable.is_a?(MergeRequest) + %hr + - unless @merge_request.persisted? + .form-group + = f.label :source_branch, class: 'control-label' do + %i.fa.fa-code-fork + Source Branch + .col-sm-10 + = f.select(:source_branch, [@merge_request.source_branch], { }, { class: 'source_branch select2 span2', disabled: true }) + %p.help-block + = link_to 'Change source branch', mr_change_branches_path(@merge_request) + .form-group + = f.label :target_branch, class: 'control-label' do + %i.fa.fa-code-fork + Target Branch + .col-sm-10 + = f.select(:target_branch, @merge_request.target_branches, { include_blank: "Select branch" }, { class: 'target_branch select2 span2' }) + .form-actions - if !issuable.project.empty_repo? && (guide_url = contribution_guide_url(issuable.project)) && !issuable.persisted? %p diff --git a/db/migrate/20150529111607_add_user_oauth_applications_to_application_settings.rb b/db/migrate/20150529111607_add_user_oauth_applications_to_application_settings.rb new file mode 100644 index 00000000000..6a78294f0b2 --- /dev/null +++ b/db/migrate/20150529111607_add_user_oauth_applications_to_application_settings.rb @@ -0,0 +1,5 @@ +class AddUserOauthApplicationsToApplicationSettings < ActiveRecord::Migration + def change + add_column :application_settings, :user_oauth_applications, :bool, default: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 1ab91256406..dfd93d056e9 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: 20150516060434) do +ActiveRecord::Schema.define(version: 20150529111607) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -33,6 +33,7 @@ ActiveRecord::Schema.define(version: 20150516060434) do t.integer "default_project_visibility" t.integer "default_snippet_visibility" t.text "restricted_signup_domains" + t.boolean "user_oauth_applications", default: true end create_table "broadcast_messages", force: true do |t| diff --git a/features/dashboard/group.feature b/features/dashboard/group.feature index cf4b8d7283b..e3c01db2ebb 100644 --- a/features/dashboard/group.feature +++ b/features/dashboard/group.feature @@ -24,7 +24,8 @@ Feature: Dashboard Group When I visit dashboard groups page Then I should see group "Owned" in group list Then I should see group "Guest" in group list - Then I should not see the "Leave" button for group "Owned" + When I click on the "Leave" button for group "Owned" + Then I should see the "Can not leave message" @javascript Scenario: Guest should be able to leave from group diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index 7a831901607..eb091c291e9 100644 --- a/features/project/merge_requests.feature +++ b/features/project/merge_requests.feature @@ -207,3 +207,11 @@ Feature: Project Merge Requests Then I should see that I am subscribed When I click button "Unsubscribe" Then I should see that I am unsubscribed + + @javascript + Scenario: I can change the target branch + Given I visit merge request page "Bug NS-04" + And I click link "Edit" for the merge request + When I click the "Target branch" dropdown + And I select a new target branch + Then I should see new target branch changes diff --git a/features/steps/dashboard/dashboard.rb b/features/steps/dashboard/dashboard.rb index 8508b2a8096..bb1f2f444f9 100644 --- a/features/steps/dashboard/dashboard.rb +++ b/features/steps/dashboard/dashboard.rb @@ -23,8 +23,8 @@ class Spinach::Features::Dashboard < Spinach::FeatureSteps step 'I see prefilled new Merge Request page' do current_path.should == new_namespace_project_merge_request_path(@project.namespace, @project) find("#merge_request_target_project_id").value.should == @project.id.to_s - find("#merge_request_source_branch").value.should == "fix" - find("#merge_request_target_branch").value.should == "master" + find("input#merge_request_source_branch").value.should == "fix" + find("input#merge_request_target_branch").value.should == "master" end step 'user with name "John Doe" joined project "Shop"' do diff --git a/features/steps/dashboard/group.rb b/features/steps/dashboard/group.rb index 8384df2fb59..aeea49320ff 100644 --- a/features/steps/dashboard/group.rb +++ b/features/steps/dashboard/group.rb @@ -60,4 +60,8 @@ class Spinach::Features::DashboardGroup < Spinach::FeatureSteps page.should have_content "Samurai" page.should have_content "Tokugawa Shogunate" end + + step 'I should see the "Can not leave message"' do + page.should have_content "You can not leave Owned group because you're the last owner" + end end diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index 48bb316e203..4ca7cf5e5fe 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -305,6 +305,20 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps fill_in 'issue_search', with: "Fe" end + step 'I click the "Target branch" dropdown' do + first('.target_branch').click + end + + step 'I select a new target branch' do + select "feature", from: "merge_request_target_branch" + click_button 'Save' + end + + step 'I should see new target branch changes' do + page.should have_content 'From fix into feature' + page.should have_content 'Target branch changed from master to feature' + end + def merge_request @merge_request ||= MergeRequest.find_by!(title: "Bug NS-05") end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 0a0760056cf..c75173c1452 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -20,7 +20,8 @@ describe MergeRequests::UpdateService do description: 'Also please fix', assignee_id: user2.id, state_event: 'close', - label_ids: [label.id] + label_ids: [label.id], + target_branch: 'target' } end @@ -39,6 +40,7 @@ describe MergeRequests::UpdateService do it { expect(@merge_request).to be_closed } it { expect(@merge_request.labels.count).to eq(1) } it { expect(@merge_request.labels.first.title).to eq('Bug') } + it { expect(@merge_request.target_branch).to eq('target') } it 'should execute hooks with update action' do expect(service).to have_received(:execute_hooks). @@ -77,6 +79,13 @@ describe MergeRequests::UpdateService do expect(note).not_to be_nil expect(note.note).to eq 'Title changed from **Old title** to **New title**' end + + it 'creates system note about branch change' do + note = find_note('Target') + + expect(note).not_to be_nil + expect(note.note).to eq 'Target branch changed from `master` to `target`' + end end end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 0dcc94e8bd4..700286b585a 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -228,6 +228,20 @@ describe SystemNoteService do end end + describe '.change_branch' do + subject { described_class.change_branch(noteable, project, author, 'target', old_branch, new_branch) } + let(:old_branch) { 'old_branch'} + let(:new_branch) { 'new_branch'} + + it_behaves_like 'a system note' + + context 'when target branch name changed' do + it 'sets the note text' do + expect(subject.note).to eq "Target branch changed from `#{old_branch}` to `#{new_branch}`" + end + end + end + describe '.cross_reference' do subject { described_class.cross_reference(noteable, mentioner, author) } |