From 8b35b208372e2394953226b7e4342d860a695f84 Mon Sep 17 00:00:00 2001 From: Steven Thonus Date: Tue, 28 Jan 2014 22:36:50 +0100 Subject: [PATCH 01/54] first setup to protect protected branched to force updates --- lib/api/internal.rb | 4 +++- lib/gitlab/git_access.rb | 21 +++++++++++++-------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/api/internal.rb b/lib/api/internal.rb index bcf97574673..06c66ba0b35 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -10,6 +10,7 @@ module API # project - project path with namespace # action - git action (git-upload-pack or git-receive-pack) # ref - branch name + # forced_push - forced_push # get "/allowed" do # Check for *.wiki repositories. @@ -35,7 +36,8 @@ module API project, params[:ref], params[:oldrev], - params[:newrev] + params[:newrev], + params[:forced_push] ) end diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 1ab8f9213a3..a0401290cc0 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -5,7 +5,7 @@ module Gitlab attr_reader :params, :project, :git_cmd, :user - def allowed?(actor, cmd, project, ref = nil, oldrev = nil, newrev = nil) + def allowed?(actor, cmd, project, ref = nil, oldrev = nil, newrev = nil, forced_push = false) case cmd when *DOWNLOAD_COMMANDS if actor.is_a? User @@ -19,12 +19,12 @@ module Gitlab end when *PUSH_COMMANDS if actor.is_a? User - push_allowed?(actor, project, ref, oldrev, newrev) + push_allowed?(actor, project, ref, oldrev, newrev, forced_push) elsif actor.is_a? DeployKey # Deploy key not allowed to push return false elsif actor.is_a? Key - push_allowed?(actor.user, project, ref, oldrev, newrev) + push_allowed?(actor.user, project, ref, oldrev, newrev, forced_push) else raise 'Wrong actor' end @@ -41,13 +41,18 @@ module Gitlab end end - def push_allowed?(user, project, ref, oldrev, newrev) + def push_allowed?(user, project, ref, oldrev, newrev, forced_push) if user && user_allowed?(user) + action = if project.protected_branch?(ref) - :push_code_to_protected_branches - else - :push_code - end + if forced_push + :force_push_code_to_protected_branches + else + :push_code_to_protected_branches + end + else + :push_code + end user.can?(action, project) else false From fcdb34373fea4cccc635bbe92f70e47d093b80a8 Mon Sep 17 00:00:00 2001 From: Yatish Mehta Date: Fri, 28 Mar 2014 14:01:06 +0530 Subject: [PATCH 02/54] Consistent format for class objects in javascript --- app/assets/javascripts/commit/file.js.coffee | 6 +++--- app/assets/javascripts/commit/image-file.js.coffee | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/commit/file.js.coffee b/app/assets/javascripts/commit/file.js.coffee index a45ee58d1b4..4db9116a9de 100644 --- a/app/assets/javascripts/commit/file.js.coffee +++ b/app/assets/javascripts/commit/file.js.coffee @@ -1,7 +1,7 @@ class CommitFile - + constructor: (file) -> if $('.image', file).length new ImageFile(file) - -this.CommitFile = CommitFile \ No newline at end of file + +@CommitFile = CommitFile diff --git a/app/assets/javascripts/commit/image-file.js.coffee b/app/assets/javascripts/commit/image-file.js.coffee index f901a9e28ff..607b85eb45c 100644 --- a/app/assets/javascripts/commit/image-file.js.coffee +++ b/app/assets/javascripts/commit/image-file.js.coffee @@ -125,4 +125,4 @@ class ImageFile img.on 'load', => callback.call(this, domImg.naturalWidth, domImg.naturalHeight) -this.ImageFile = ImageFile \ No newline at end of file +@ImageFile = ImageFile From bbea1d1a72cbe0d2b53316e6d67c361a58207de7 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Tue, 1 Apr 2014 16:05:26 +0200 Subject: [PATCH 03/54] Include the Sidekiq version in gitlab:env:info --- lib/tasks/gitlab/info.rake | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/tasks/gitlab/info.rake b/lib/tasks/gitlab/info.rake index 690f414f3b3..72452e1d8ea 100644 --- a/lib/tasks/gitlab/info.rake +++ b/lib/tasks/gitlab/info.rake @@ -24,6 +24,7 @@ namespace :gitlab do puts "Gem Version:\t#{gem_version || "unknown".red}" puts "Bundler Version:#{bunder_version || "unknown".red}" puts "Rake Version:\t#{rake_version || "unknown".red}" + puts "Sidekiq Version:#{Sidekiq::VERSION}" # check database adapter From 03b14bb814a655b1ba13f318f67b78147ec47546 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Tue, 1 Apr 2014 16:46:27 +0200 Subject: [PATCH 04/54] Use the new Sidekiq syntax to specify queues This syntax is compatible with the current Sidekiq version we use (2.17.0) and it protects us against future breakage by https://github.com/mperham/sidekiq/commit/040eda558bc64139c846c781d3de0b79603e3035 --- script/background_jobs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/background_jobs b/script/background_jobs index 52732f5532b..e0140e9689b 100755 --- a/script/background_jobs +++ b/script/background_jobs @@ -37,7 +37,7 @@ function start_no_deamonize function start_sidekiq { - bundle exec sidekiq -q post_receive,mailer,system_hook,project_web_hook,gitlab_shell,common,default -e $RAILS_ENV -P $sidekiq_pidfile $@ >> $sidekiq_logfile 2>&1 + bundle exec sidekiq -q post_receive -q mailer -q system_hook -q project_web_hook -q gitlab_shell -q common -q default -e $RAILS_ENV -P $sidekiq_pidfile $@ >> $sidekiq_logfile 2>&1 } function load_ok From cf788f3d97658fe874d850ae450bcca8f10b1678 Mon Sep 17 00:00:00 2001 From: Marc Radulescu Date: Mon, 31 Mar 2014 16:39:12 +0200 Subject: [PATCH 05/54] fix JIRA documentation by explaining how to reference JIRA issues in GitLab comments --- doc/integration/external-issue-tracker.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/integration/external-issue-tracker.md b/doc/integration/external-issue-tracker.md index 3212ebd64b5..6b34826da52 100644 --- a/doc/integration/external-issue-tracker.md +++ b/doc/integration/external-issue-tracker.md @@ -2,7 +2,7 @@ GitLab has a great issue tracker but you can also use an external issue tracker - the 'Issues' link on the GitLab project pages takes you to the appropriate JIRA issue index; - clicking 'New issue' on the project dashboard creates a new JIRA issue; -- textual references to PROJECT-1234 in comments, commit messages get turned into HTML links to the corresponding JIRA issue. +- To reference JIRA issue PROJECT-1234 in comments, use syntax #PROJECT-1234. Commit messages get turned into HTML links to the corresponding JIRA issue. ![jira screenshot](jira-intergration-points.png) From f4f0a7e03e64e3fffb9b14ba5583e2c6b7c6441b Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Fri, 28 Mar 2014 17:15:58 +0100 Subject: [PATCH 06/54] Add a failing test. --- spec/services/notification_service_spec.rb | 35 +++++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index fbd73a7086f..0c25f7a71d0 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -57,15 +57,42 @@ describe NotificationService do Notify.should_not_receive(:note_issue_email) notification.new_note(mentioned_note) end + end - def should_email(user_id) - Notify.should_receive(:note_issue_email).with(user_id, note.id) + describe 'new note on issue in project that belongs to a group' do + let(:group) { create(:group) } + + before do + note.project.namespace_id = group.id + note.project.group.add_user(@u_watcher, UsersGroup::MASTER) + note.project.save + user_project = note.project.users_projects.find_by_user_id(@u_watcher.id) + user_project.notification_level = Notification::N_PARTICIPATING + user_project.save + user_group = note.project.group.users_groups.find_by_user_id(@u_watcher.id) + user_group.notification_level = Notification::N_GLOBAL + user_group.save end - def should_not_email(user_id) - Notify.should_not_receive(:note_issue_email).with(user_id, note.id) + it do + should_email(note.noteable.author_id) + should_email(note.noteable.assignee_id) + should_email(@u_mentioned.id) + should_not_email(@u_watcher.id) + should_not_email(note.author_id) + should_not_email(@u_participating.id) + should_not_email(@u_disabled.id) + notification.new_note(note) end end + + def should_email(user_id) + Notify.should_receive(:note_issue_email).with(user_id, note.id) + end + + def should_not_email(user_id) + Notify.should_not_receive(:note_issue_email).with(user_id, note.id) + end end context 'commit note' do From ce30a56f333f61c01314cf32e15381080a9b14d3 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Sat, 29 Mar 2014 10:56:13 +0100 Subject: [PATCH 07/54] Add or remove watchers from list based on their notification settings. --- app/services/notification_service.rb | 46 +++++++++++++++++++--------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index d85398809c2..4d6616b969d 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -178,22 +178,24 @@ class NotificationService # Get project users with WATCH notification level def project_watchers(project) - # Gather all user ids that have WATCH notification setting for project - project_notification_uids = project_notification_list(project, Notification::N_WATCH) + # Gather all user that have WATCH notification setting for project and group + project_uids, group_uids = project_and_group_watchers(project, Notification::N_WATCH) - # Gather all user ids that have WATCH notification setting for group - group_notification_uids = group_notification_list(project, Notification::N_WATCH) + # Gather all user that have WATCH as their GLOBAL setting + global_setting_project, global_setting_group = project_and_group_watchers(project, Notification::N_GLOBAL) + global_uids = [global_setting_project, global_setting_group].flatten.uniq + global_watchers_ids = User.where(id: global_uids, notification_level: Notification::N_WATCH).pluck(:id) - # Gather all user ids that have GLOBAL setting - global_notification_uids = global_notification_list(project) + # Select watchers based on what the project setting is + project_watchers, user_ids = select_watchers(global_watchers_ids, global_setting_project, project_uids) - project_and_group_uids = [project_notification_uids, group_notification_uids].flatten.uniq - group_and_project_watchers = User.where(id: project_and_group_uids) + # Select watchers based on what the group setting is + group_watchers, ids = select_watchers(user_ids, global_setting_group, group_uids) - # Find all users that have WATCH as their GLOBAL setting - global_watchers = User.where(id: global_notification_uids, notification_level: Notification::N_WATCH) + project_watchers.concat(group_watchers.concat(ids)).uniq + watchers = User.where(id: project_watchers) - [group_and_project_watchers, global_watchers].flatten.uniq + watchers.to_a end def project_notification_list(project, notification_level) @@ -208,11 +210,25 @@ class NotificationService end end - def global_notification_list(project) + def select_watchers(user_ids, global_setting, setting) + watchers = [] + user_ids.each do |i| + if setting.include?(i) + watchers << i + elsif global_setting.include?(i) + watchers << i + else + user_ids.delete(i) + end + end + [watchers, user_ids] + end + + def project_and_group_watchers(project, notification_level) [ - project_notification_list(project, Notification::N_GLOBAL), - group_notification_list(project, Notification::N_GLOBAL) - ].flatten + project_notification_list(project, notification_level), + group_notification_list(project, notification_level) + ] end # Remove users with disabled notifications from array From 528a641e9ee5f72f3f5a1da1e1a31b9b61262efd Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 1 Apr 2014 13:07:23 +0200 Subject: [PATCH 08/54] Use group notification settings while honoring project settings. --- app/services/notification_service.rb | 73 ++++++++++++++++------------ 1 file changed, 41 insertions(+), 32 deletions(-) diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 4d6616b969d..dc5f4aa8822 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -178,28 +178,28 @@ class NotificationService # Get project users with WATCH notification level def project_watchers(project) - # Gather all user that have WATCH notification setting for project and group - project_uids, group_uids = project_and_group_watchers(project, Notification::N_WATCH) + project_members = project_notification_list(project) - # Gather all user that have WATCH as their GLOBAL setting - global_setting_project, global_setting_group = project_and_group_watchers(project, Notification::N_GLOBAL) - global_uids = [global_setting_project, global_setting_group].flatten.uniq - global_watchers_ids = User.where(id: global_uids, notification_level: Notification::N_WATCH).pluck(:id) + project_global= project_notification_list(project, Notification::N_GLOBAL) + group_global = group_notification_list(project, Notification::N_GLOBAL) + global_watch = User.where( + id: [project_global, group_global].flatten.uniq, + notification_level: Notification::N_WATCH + ).pluck(:id) - # Select watchers based on what the project setting is - project_watchers, user_ids = select_watchers(global_watchers_ids, global_setting_project, project_uids) + project_watch = watch_project(project, project_global, global_watch) + group_watch = watch_group(project, project_members, group_global, global_watch) - # Select watchers based on what the group setting is - group_watchers, ids = select_watchers(user_ids, global_setting_group, group_uids) - - project_watchers.concat(group_watchers.concat(ids)).uniq - watchers = User.where(id: project_watchers) - - watchers.to_a + User.where(id: project_watch.concat(group_watch).uniq).to_a end - def project_notification_list(project, notification_level) - project.users_projects.where(notification_level: notification_level).pluck(:user_id) + def project_notification_list(project, notification_level=nil) + project_members = project.users_projects + if notification_level + project_members.where(notification_level: notification_level).pluck(:user_id) + else + project_members.pluck(:user_id) + end end def group_notification_list(project, notification_level) @@ -210,25 +210,34 @@ class NotificationService end end - def select_watchers(user_ids, global_setting, setting) - watchers = [] - user_ids.each do |i| - if setting.include?(i) - watchers << i - elsif global_setting.include?(i) - watchers << i - else - user_ids.delete(i) + def watch_project(project, global_setting, watch_global) + uids = project_notification_list(project, Notification::N_WATCH) + + global_setting.each do |i| + if watch_global.include?(i) + uids << i end end - [watchers, user_ids] + uids.uniq end - def project_and_group_watchers(project, notification_level) - [ - project_notification_list(project, notification_level), - group_notification_list(project, notification_level) - ] + def watch_group(project, project_members, global_setting, watch_global) + uids = group_notification_list(project, Notification::N_WATCH) + # Group setting is watch, add to watchers list user is not project member + watch = [] + uids.each do |i| + if project_members.exclude?(i) + watch << i + end + end + + global_setting.each do |i| + if project_members.exclude?(i) && watch_global.include?(i) + watch << i + end + end + + watch.uniq end # Remove users with disabled notifications from array From 109ab1a58baec790dceb318e96142132cd5b053c Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 1 Apr 2014 13:50:20 +0200 Subject: [PATCH 09/54] Add comments, fix style, better names for notification methods. --- app/services/notification_service.rb | 75 ++++++++++++++++------------ 1 file changed, 43 insertions(+), 32 deletions(-) diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index dc5f4aa8822..06140c5afeb 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -178,23 +178,21 @@ class NotificationService # Get project users with WATCH notification level def project_watchers(project) - project_members = project_notification_list(project) + project_members = users_project_notification(project) - project_global= project_notification_list(project, Notification::N_GLOBAL) - group_global = group_notification_list(project, Notification::N_GLOBAL) - global_watch = User.where( - id: [project_global, group_global].flatten.uniq, - notification_level: Notification::N_WATCH - ).pluck(:id) + users_with_project_level_global = users_project_notification(project, Notification::N_GLOBAL) + users_with_group_level_global = users_group_notification(project, Notification::N_GLOBAL) + users = users_with_global_level_watch([users_with_project_level_global, users_with_group_level_global].flatten.uniq) - project_watch = watch_project(project, project_global, global_watch) - group_watch = watch_group(project, project_members, group_global, global_watch) + users_with_project_setting = select_users_project_setting(project, users_with_project_level_global, users) + users_with_group_setting = select_users_group_setting(project, project_members, users_with_group_level_global, users) - User.where(id: project_watch.concat(group_watch).uniq).to_a + User.where(id: users_with_project_setting.concat(users_with_group_setting).uniq).to_a end - def project_notification_list(project, notification_level=nil) + def users_project_notification(project, notification_level=nil) project_members = project.users_projects + if notification_level project_members.where(notification_level: notification_level).pluck(:user_id) else @@ -202,7 +200,7 @@ class NotificationService end end - def group_notification_list(project, notification_level) + def users_group_notification(project, notification_level) if project.group project.group.users_groups.where(notification_level: notification_level).pluck(:user_id) else @@ -210,34 +208,47 @@ class NotificationService end end - def watch_project(project, global_setting, watch_global) - uids = project_notification_list(project, Notification::N_WATCH) - - global_setting.each do |i| - if watch_global.include?(i) - uids << i - end - end - uids.uniq + def users_with_global_level_watch(ids) + User.where( + id: ids, + notification_level: Notification::N_WATCH + ).pluck(:id) end - def watch_group(project, project_members, global_setting, watch_global) - uids = group_notification_list(project, Notification::N_WATCH) - # Group setting is watch, add to watchers list user is not project member - watch = [] - uids.each do |i| - if project_members.exclude?(i) - watch << i + # Build a list of users based on project notifcation settings + def select_users_project_setting(project, global_setting, users_global_level_watch) + users = users_project_notification(project, Notification::N_WATCH) + + # If project setting is global, add to watch list if global setting is watch + global_setting.each do |user_id| + if users_global_level_watch.include?(user_id) + users << user_id end end - global_setting.each do |i| - if project_members.exclude?(i) && watch_global.include?(i) - watch << i + users + end + + # Build a list of users based on group notifcation settings + def select_users_group_setting(project, project_members, global_setting, users_global_level_watch) + uids = users_group_notification(project, Notification::N_WATCH) + + # Group setting is watch, add to users list if user is not project member + users = [] + uids.each do |user_id| + if project_members.exclude?(user_id) + users << user_id end end - watch.uniq + # Group setting is global, add to users list if global setting is watch + global_setting.each do |user_id| + if project_members.exclude?(user_id) && users_global_level_watch.include?(user_id) + users << user_id + end + end + + users end # Remove users with disabled notifications from array From f732e6c8da83c46ac85dfd913f2adddaa1c8148d Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 1 Apr 2014 18:37:45 +0200 Subject: [PATCH 10/54] Pin sidekiq version to 2.17.0. --- Gemfile | 2 +- Gemfile.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index 397165f668f..c65ff814497 100644 --- a/Gemfile +++ b/Gemfile @@ -102,7 +102,7 @@ gem "acts-as-taggable-on" # Background jobs gem 'slim' gem 'sinatra', require: nil -gem 'sidekiq' +gem 'sidekiq', '2.17.0' # HTTP requests gem "httparty" diff --git a/Gemfile.lock b/Gemfile.lock index 1a0bce98ac5..956cf1c4984 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -653,7 +653,7 @@ DEPENDENCIES select2-rails settingslogic shoulda-matchers (~> 2.1.0) - sidekiq + sidekiq (= 2.17.0) simplecov sinatra six From b06f43288aa3f8e617e6534ce28e9a12107981d9 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Wed, 2 Apr 2014 12:26:09 +0200 Subject: [PATCH 11/54] Pass the required parameters on search submit. --- app/views/projects/issues/_head.html.haml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/views/projects/issues/_head.html.haml b/app/views/projects/issues/_head.html.haml index 0b7697622b0..f1e866c8e9d 100644 --- a/app/views/projects/issues/_head.html.haml +++ b/app/views/projects/issues/_head.html.haml @@ -20,6 +20,11 @@ = form_tag project_issues_path(@project), method: :get, id: "issue_search_form", class: 'pull-left issue-search-form' do .append-right-10.hidden-xs.hidden-sm = search_field_tag :issue_search, nil, { placeholder: 'Filter by title or description', class: 'form-control issue_search search-text-input input-mn-300' } + = hidden_field_tag :state, params['state'] + = hidden_field_tag :scope, params['scope'] + = hidden_field_tag :assignee_id, params['assignee_id'] + = hidden_field_tag :milestone_id, params['milestone_id'] + = hidden_field_tag :label_id, params['label_id'] - if can? current_user, :write_issue, @project = link_to new_project_issue_path(@project, issue: { assignee_id: params[:assignee_id], milestone_id: params[:milestone_id]}), class: "btn btn-new pull-left", title: "New Issue", id: "new_issue_link" do %i.icon-plus From cfd9fd30d60c5a880785acda27e9f3d55b17e4ef Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 13:38:35 +0300 Subject: [PATCH 12/54] Move code for issue creation to service. The goal of suych refactoring is to get rid of observers. Its much easier to test and code when object creation and all other related actions done in one class instead of splited across observers, callbacks etc. Signed-off-by: Dmitriy Zaporozhets --- app/controllers/projects/issues_controller.rb | 4 +--- app/observers/issue_observer.rb | 7 ------ app/services/base_service.rb | 12 ++++++++++ app/services/issues/create_service.rb | 23 +++++++++++++++++++ lib/api/issues.rb | 20 ++++++++-------- spec/services/issues/create_service_spec.rb | 22 ++++++++++++++++++ 6 files changed, 67 insertions(+), 21 deletions(-) create mode 100644 app/services/issues/create_service.rb create mode 100644 spec/services/issues/create_service_spec.rb diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index eef849d8209..ca85ba6b257 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -59,9 +59,7 @@ class Projects::IssuesController < Projects::ApplicationController end def create - @issue = @project.issues.new(params[:issue]) - @issue.author = current_user - @issue.save + @issue = Issues::CreateService.new(project, current_user, params[:issue]).execute respond_to do |format| format.html do diff --git a/app/observers/issue_observer.rb b/app/observers/issue_observer.rb index 30da1f83da7..c2132ddca55 100644 --- a/app/observers/issue_observer.rb +++ b/app/observers/issue_observer.rb @@ -1,11 +1,4 @@ class IssueObserver < BaseObserver - def after_create(issue) - notification.new_issue(issue, current_user) - event_service.open_issue(issue, current_user) - issue.create_cross_references!(issue.project, current_user) - execute_hooks(issue) - end - def after_close(issue, transition) notification.close_issue(issue, current_user) event_service.close_issue(issue, current_user) diff --git a/app/services/base_service.rb b/app/services/base_service.rb index 610f0474872..9ad80923152 100644 --- a/app/services/base_service.rb +++ b/app/services/base_service.rb @@ -16,4 +16,16 @@ class BaseService def can?(object, action, subject) abilities.allowed?(object, action, subject) end + + def notification_service + NotificationService.new + end + + def event_service + EventCreateService.new + end + + def log_info message + Gitlab::AppLogger.info message + end end diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb new file mode 100644 index 00000000000..37f440fc40e --- /dev/null +++ b/app/services/issues/create_service.rb @@ -0,0 +1,23 @@ +module Issues + class CreateService < BaseService + def execute + issue = project.issues.new(params) + issue.author = current_user + + if issue.save + notification_service.new_issue(issue, current_user) + event_service.open_issue(issue, current_user) + issue.create_cross_references!(issue.project, current_user) + execute_hooks(issue) + end + + issue + end + + private + + def execute_hooks(issue) + issue.project.execute_hooks(issue.to_hook_data, :issue_hooks) + end + end +end diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 3d15c35b8cc..169c58b0075 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -48,17 +48,15 @@ module API # Example Request: # POST /projects/:id/issues post ":id/issues" do - set_current_user_for_thread do - required_attributes! [:title] - attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id] - attrs[:label_list] = params[:labels] if params[:labels].present? - @issue = user_project.issues.new attrs - @issue.author = current_user - if @issue.save - present @issue, with: Entities::Issue - else - not_found! - end + required_attributes! [:title] + attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id] + attrs[:label_list] = params[:labels] if params[:labels].present? + issue = ::Issues::CreateService.new(user_project, current_user, attrs).execute + + if issue.valid? + present issue, with: Entities::Issue + else + not_found! end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb new file mode 100644 index 00000000000..7e2d5ad2e81 --- /dev/null +++ b/spec/services/issues/create_service_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Issues::CreateService do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + + describe :execute do + context "valid params" do + before do + project.team << [user, :master] + opts = { + title: 'Awesome issue', + description: 'please fix' + } + + @issue = Issues::CreateService.new(project, user, opts).execute + end + + it { @issue.should be_valid } + end + end +end From cc3ea3d3736db8d089e6e94d8f863d47631d0963 Mon Sep 17 00:00:00 2001 From: dosire Date: Wed, 2 Apr 2014 12:46:17 +0200 Subject: [PATCH 13/54] Adapt the layout of the doc readme to fit the help page. --- doc/README.md | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/doc/README.md b/doc/README.md index d85a2d854d3..9d39a1b0cdd 100644 --- a/doc/README.md +++ b/doc/README.md @@ -1,18 +1,26 @@ ## The GitLab Documentation covers the following subjects -+ [API](api/README.md) -+ [Development](development/README.md) -+ [Install](install/README.md) -+ [Integration](integration/external-issue-tracker.md) -+ [Legal](legal/README.md) -+ [Markdown](markdown/markdown.md) -+ [Permissions](permissions/permissions.md) -+ [Public access](public_access/public_access.md) -+ [Raketasks](raketasks/README.md) -+ [Release](release/README.md) -+ [Security](security/README.md) -+ [SSH](ssh/README.md) -+ [System hooks](system_hooks/system_hooks.md) -+ [Update](update/README.md) -+ [Web hooks](web_hooks/web_hooks.md) -+ [Workflow](workflow/workflow.md) +User documentation + ++ [API](api/README.md) Explore how you can access GitLab via a simple and powerful API. ++ [Markdown](markdown/markdown.md) Learn what you can do with GitLab's advanced formatting system. ++ [Permissions](permissions/permissions.md) Learn what each role in a project (guest/reporter/developer/master/owner) can do. ++ [Public access](public_access/public_access.md) Learn how you can allow public and internal access to a project. ++ [SSH](ssh/README.md) Setup your ssh keys and deploy keys for secure access to your projects. ++ [Web hooks](web_hooks/web_hooks.md) Let GitLab notify you when new code has been pushed to your project. ++ [Workflow](workflow/workflow.md) Learn how to use Git and GitLab together. + +Administrator documentation + ++ [Install](install/README.md) Requirements, directory structures and manual installation. ++ [Integration](integration/external-issue-tracker.md) How to integrate JIRA and Redmine. ++ [Raketasks](raketasks/README.md) Explore what GitLab has in store for you to make administration easier. ++ [System hooks](system_hooks/system_hooks.md) Let GitLab notify you when certain management tasks need to be carried out. ++ [Security](security/README.md) Learn what you can do to further secure your GitLab instance. ++ [Update](update/README.md) Update guides to upgrade your installation. + +Contributor documentation + ++ [Development](development/README.md) Explains the architecture and the guidelines for shell commands. ++ [Legal](legal/README.md) Contributor license agreements. ++ [Release](release/README.md) How to make the monthly and security releases. From 7aacc08a92c2bbac74c45d9b506a801f3d947984 Mon Sep 17 00:00:00 2001 From: dosire Date: Wed, 2 Apr 2014 12:47:07 +0200 Subject: [PATCH 14/54] Make headers of the doc bold. --- doc/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/README.md b/doc/README.md index 9d39a1b0cdd..3c76f9e4f82 100644 --- a/doc/README.md +++ b/doc/README.md @@ -1,6 +1,6 @@ ## The GitLab Documentation covers the following subjects -User documentation +**User documentation** + [API](api/README.md) Explore how you can access GitLab via a simple and powerful API. + [Markdown](markdown/markdown.md) Learn what you can do with GitLab's advanced formatting system. @@ -10,7 +10,7 @@ User documentation + [Web hooks](web_hooks/web_hooks.md) Let GitLab notify you when new code has been pushed to your project. + [Workflow](workflow/workflow.md) Learn how to use Git and GitLab together. -Administrator documentation +**Administrator documentation** + [Install](install/README.md) Requirements, directory structures and manual installation. + [Integration](integration/external-issue-tracker.md) How to integrate JIRA and Redmine. @@ -19,7 +19,7 @@ Administrator documentation + [Security](security/README.md) Learn what you can do to further secure your GitLab instance. + [Update](update/README.md) Update guides to upgrade your installation. -Contributor documentation +**Contributor documentation** + [Development](development/README.md) Explains the architecture and the guidelines for shell commands. + [Legal](legal/README.md) Contributor license agreements. From d54e3f361ad61a9bd767ec2d4ab00277d4a06c6d Mon Sep 17 00:00:00 2001 From: dosire Date: Wed, 2 Apr 2014 12:47:45 +0200 Subject: [PATCH 15/54] Remove top header. --- doc/README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/README.md b/doc/README.md index 3c76f9e4f82..dfa3211d01b 100644 --- a/doc/README.md +++ b/doc/README.md @@ -1,5 +1,3 @@ -## The GitLab Documentation covers the following subjects - **User documentation** + [API](api/README.md) Explore how you can access GitLab via a simple and powerful API. From c4e81ed9de6a5bbfe089e9b61ca0400167e489f3 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 13:54:41 +0300 Subject: [PATCH 16/54] Move update issue code to separate service Signed-off-by: Dmitriy Zaporozhets --- app/controllers/projects/issues_controller.rb | 3 +-- app/observers/issue_observer.rb | 10 -------- app/services/issues/base_service.rb | 19 +++++++++++++++ app/services/issues/create_service.rb | 6 ----- app/services/issues/update_service.rb | 19 +++++++++++++++ lib/api/issues.rb | 20 ++++++++-------- spec/services/issues/create_service_spec.rb | 1 + spec/services/issues/update_service_spec.rb | 24 +++++++++++++++++++ 8 files changed, 74 insertions(+), 28 deletions(-) create mode 100644 app/services/issues/base_service.rb create mode 100644 app/services/issues/update_service.rb create mode 100644 spec/services/issues/update_service_spec.rb diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index ca85ba6b257..4e7a716bfe4 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -74,8 +74,7 @@ class Projects::IssuesController < Projects::ApplicationController end def update - @issue.update_attributes(params[:issue]) - @issue.reset_events_cache + @issue = Issues::UpdateService.new(project, current_user, params[:issue]).execute(issue) respond_to do |format| format.js diff --git a/app/observers/issue_observer.rb b/app/observers/issue_observer.rb index c2132ddca55..b4880b12fd7 100644 --- a/app/observers/issue_observer.rb +++ b/app/observers/issue_observer.rb @@ -12,16 +12,6 @@ class IssueObserver < BaseObserver execute_hooks(issue) end - def after_update(issue) - if issue.is_being_reassigned? - notification.reassigned_issue(issue, current_user) - create_assignee_note(issue) - end - - issue.notice_added_references(issue.project, current_user) - execute_hooks(issue) - end - protected # Create issue note with service comment like 'Status changed to closed' diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb new file mode 100644 index 00000000000..e04c1c6fb7a --- /dev/null +++ b/app/services/issues/base_service.rb @@ -0,0 +1,19 @@ +module Issues + class BaseService < ::BaseService + + private + + # Create issue note with service comment like 'Status changed to closed' + def create_note(issue) + Note.create_status_change_note(issue, issue.project, current_user, issue.state, current_commit) + end + + def create_assignee_note(issue) + Note.create_assignee_change_note(issue, issue.project, current_user, issue.assignee) + end + + def execute_hooks(issue) + issue.project.execute_hooks(issue.to_hook_data, :issue_hooks) + end + end +end diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 37f440fc40e..8008086dae9 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -13,11 +13,5 @@ module Issues issue end - - private - - def execute_hooks(issue) - issue.project.execute_hooks(issue.to_hook_data, :issue_hooks) - end end end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb new file mode 100644 index 00000000000..4db9dee11d3 --- /dev/null +++ b/app/services/issues/update_service.rb @@ -0,0 +1,19 @@ +module Issues + class UpdateService < BaseService + def execute(issue) + if issue.update_attributes(params) + issue.reset_events_cache + + if issue.is_being_reassigned? + notification.reassigned_issue(issue, current_user) + create_assignee_note(issue) + end + + issue.notice_added_references(issue.project, current_user) + execute_hooks(issue) + end + + issue + end + end +end diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 169c58b0075..f50be3a815d 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -74,18 +74,18 @@ module API # Example Request: # PUT /projects/:id/issues/:issue_id put ":id/issues/:issue_id" do - set_current_user_for_thread do - @issue = user_project.issues.find(params[:issue_id]) - authorize! :modify_issue, @issue + issue = user_project.issues.find(params[:issue_id]) + authorize! :modify_issue, issue - attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id, :state_event] - attrs[:label_list] = params[:labels] if params[:labels].present? + attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id, :state_event] + attrs[:label_list] = params[:labels] if params[:labels].present? - if @issue.update_attributes attrs - present @issue, with: Entities::Issue - else - not_found! - end + issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue) + + if issue.valid? + present issue, with: Entities::Issue + else + not_found! end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 7e2d5ad2e81..90720be5ded 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -17,6 +17,7 @@ describe Issues::CreateService do end it { @issue.should be_valid } + it { @issue.title.should == 'Awesome issue' } end end end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb new file mode 100644 index 00000000000..9bfc0f674de --- /dev/null +++ b/spec/services/issues/update_service_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe Issues::UpdateService do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + let(:issue) { create(:issue) } + + describe :execute do + context "valid params" do + before do + project.team << [user, :master] + opts = { + title: 'New title', + description: 'Also please fix' + } + + @issue = Issues::UpdateService.new(project, user, opts).execute(issue) + end + + it { @issue.should be_valid } + it { @issue.title.should == 'New title' } + end + end +end From 3a959a68ec6c6b18107855979a783d1529105a02 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 14:11:55 +0300 Subject: [PATCH 17/54] Remove unused gems Signed-off-by: Dmitriy Zaporozhets --- Gemfile | 2 -- Gemfile.lock | 6 ------ 2 files changed, 8 deletions(-) diff --git a/Gemfile b/Gemfile index c65ff814497..ab8a27a8571 100644 --- a/Gemfile +++ b/Gemfile @@ -12,8 +12,6 @@ gem "rails", "~> 4.0.0" gem "protected_attributes" gem 'rails-observers' -gem 'actionpack-page_caching' -gem 'actionpack-action_caching' # Default values for AR models gem "default_value_for", "~> 3.0.0" diff --git a/Gemfile.lock b/Gemfile.lock index 956cf1c4984..4acdb539f09 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -27,10 +27,6 @@ GEM erubis (~> 2.7.0) rack (~> 1.5.2) rack-test (~> 0.6.2) - actionpack-action_caching (1.1.0) - actionpack (>= 4.0.0, < 5.0) - actionpack-page_caching (1.0.2) - actionpack (>= 4.0.0, < 5) activemodel (4.0.3) activesupport (= 4.0.3) builder (~> 3.1.0) @@ -567,8 +563,6 @@ PLATFORMS DEPENDENCIES ace-rails-ap - actionpack-action_caching - actionpack-page_caching acts-as-taggable-on annotate (~> 2.6.0.beta2) asciidoctor From 09d24a1f67bd5fa50b05582c54ca2f5b75ea9b65 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 14:16:37 +0300 Subject: [PATCH 18/54] Better title suggestion for new MR Ex. if branch named "refactor-models" then title will be "Refactor models" Signed-off-by: Dmitriy Zaporozhets --- app/helpers/merge_requests_helper.rb | 2 +- app/views/projects/merge_requests/branch_from.js.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index ba25a87f392..00ec34ae547 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -20,7 +20,7 @@ module MergeRequestsHelper target_project_id: target_project.id, source_branch: event.branch_name, target_branch: target_project.repository.root_ref, - title: event.branch_name.humanize + title: event.branch_name.titleize.humanize } end diff --git a/app/views/projects/merge_requests/branch_from.js.haml b/app/views/projects/merge_requests/branch_from.js.haml index 1b1082baafe..693c2057a0f 100644 --- a/app/views/projects/merge_requests/branch_from.js.haml +++ b/app/views/projects/merge_requests/branch_from.js.haml @@ -3,5 +3,5 @@ var mrTitle = $('#merge_request_title'); if(mrTitle.val().length == 0) { - mrTitle.val("#{params[:ref].humanize}"); + mrTitle.val("#{params[:ref].titleize.humanize}"); } From f38eddc69e12baf7ba3eec54852744075555fec0 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 14:57:00 +0300 Subject: [PATCH 19/54] Replace custom jquery css with native one. Fixes wrong assets path for calendar in production Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/application.scss | 3 +- app/assets/stylesheets/generic/jquery.scss | 20 ++ app/assets/stylesheets/jquery.ui.gitlab.css | 257 -------------------- app/assets/stylesheets/main/fonts.scss | 1 + 4 files changed, 23 insertions(+), 258 deletions(-) create mode 100644 app/assets/stylesheets/generic/jquery.scss delete mode 100644 app/assets/stylesheets/jquery.ui.gitlab.css diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 4b7103010bb..eb5d17651b1 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -2,7 +2,7 @@ * This is a manifest file that'll automatically include all the stylesheets available in this directory * and any sub-directories. You're free to add application-wide styles to this file and they'll appear at * the top of the compiled file, but it's generally better to create a new file per style scope. - *= require jquery.ui.gitlab + *= require jquery.ui.datepicker *= require jquery.atwho *= require select2 *= require highlightjs.min @@ -43,6 +43,7 @@ @import "generic/forms.scss"; @import "generic/selects.scss"; @import "generic/highlight.scss"; +@import "generic/jquery.scss"; /** * Page specific styles (issues, projects etc): diff --git a/app/assets/stylesheets/generic/jquery.scss b/app/assets/stylesheets/generic/jquery.scss new file mode 100644 index 00000000000..423cb906d0a --- /dev/null +++ b/app/assets/stylesheets/generic/jquery.scss @@ -0,0 +1,20 @@ +.ui-widget { + font-family: $regular_font; + font-size: $font-size-base; + + &.ui-datepicker-inline { + border: 1px solid #DDD; + padding: 10px; + width: 270px; + + .ui-datepicker-header { + background: #EEE; + border-color: #DDD; + } + + .ui-datepicker-calendar td a { + padding: 5px; + text-align: center; + } + } +} diff --git a/app/assets/stylesheets/jquery.ui.gitlab.css b/app/assets/stylesheets/jquery.ui.gitlab.css deleted file mode 100644 index 5c51600ba67..00000000000 --- a/app/assets/stylesheets/jquery.ui.gitlab.css +++ /dev/null @@ -1,257 +0,0 @@ -/* Interaction Cues -----------------------------------*/ -.ui-state-disabled { cursor: default !important; } - - -/* Icons -----------------------------------*/ - -/* states and images */ -.ui-icon { display: block; text-indent: -99999px; overflow: hidden; background-repeat: no-repeat; } - - -/* Misc visuals -----------------------------------*/ - -/* Overlays */ -.ui-widget-overlay { position: absolute; top: 0; left: 0; width: 100%; height: 100%; } - - -/* - * jQuery UI CSS Framework 1.8.7 - * - * Copyright 2010, AUTHORS.txt (http://jqueryui.com/about) - * Dual licensed under the MIT or GPL Version 2 licenses. - * http://jquery.org/license - * - * http://docs.jquery.com/UI/Theming/API - * - * To view and modify this theme, visit http://jqueryui.com/themeroller/?ctl=themeroller - */ - - -/* Component containers -----------------------------------*/ -.ui-widget { font-family: Arial,sans-serif; font-size: 1.1em; } -.ui-widget .ui-widget { font-size: 1em; } -.ui-widget input, .ui-widget select, .ui-widget textarea, .ui-widget button { font-family: Arial,sans-serif; font-size: 1em; } -.ui-widget-content { border: 1px solid #CCC; background: #ffffff; color: #4F4F4F; } -.ui-widget-content a { color: #4F4F4F; } -.ui-widget-header { border: 1px solid #B6B6B6; color: #4F4F4F; font-weight: bold; } -.ui-widget-header { - background: #ededed url(bg_fallback.png) 0 0 repeat-x; /* Old browsers */ - background: -moz-linear-gradient(top, #ededed 0%, #c4c4c4 100%); /* FF3.6+ */ - background: -webkit-gradient(linear, left top, left bottom, color-stop(0%,#ededed), color-stop(100%,#c4c4c4)); /* Chrome,Safari4+ */ - background: -webkit-linear-gradient(top, #ededed 0%,#c4c4c4 100%); /* Chrome10+,Safari5.1+ */ - background: -o-linear-gradient(top, #ededed 0%,#c4c4c4 100%); /* Opera11.10+ */ - background: -ms-linear-gradient(top, #ededed 0%,#c4c4c4 100%); /* IE10+ */ - background: linear-gradient(top, #ededed 0%,#c4c4c4 100%); /* W3C */ -} -.ui-widget-header a { color: #4F4F4F; } - -/* Interaction states -----------------------------------*/ -.ui-state-default, .ui-widget-content .ui-state-default, .ui-widget-header .ui-state-default { border: 1px solid #B6B6B6; font-weight: normal; color: #4F4F4F; } -.ui-state-default, .ui-widget-content .ui-state-default, .ui-widget-header .ui-state-default { - background: #ededed url(bg_fallback.png) 0 0 repeat-x; /* Old browsers */ - background: -moz-linear-gradient(top, #ededed 0%, #c4c4c4 100%); /* FF3.6+ */ - background: -webkit-gradient(linear, left top, left bottom, color-stop(0%,#ededed), color-stop(100%,#c4c4c4)); /* Chrome,Safari4+ */ - background: -webkit-linear-gradient(top, #ededed 0%,#c4c4c4 100%); /* Chrome10+,Safari5.1+ */ - background: -o-linear-gradient(top, #ededed 0%,#c4c4c4 100%); /* Opera11.10+ */ - background: -ms-linear-gradient(top, #ededed 0%,#c4c4c4 100%); /* IE10+ */ - background: linear-gradient(top, #ededed 0%,#c4c4c4 100%); /* W3C */ - -webkit-box-shadow: 0 1px 0 rgba(255,255,255,0.6) inset; - -moz-box-shadow: 0 1px 0 rgba(255,255,255,0.6) inset; - box-shadow: 0 1px 0 rgba(255,255,255,0.6) inset; -} -.ui-state-default a, .ui-state-default a:link, .ui-state-default a:visited { color: #4F4F4F; text-decoration: none; } -.ui-state-hover, .ui-widget-content .ui-state-hover, .ui-widget-header .ui-state-hover, .ui-state-focus, .ui-widget-content .ui-state-focus, .ui-widget-header .ui-state-focus { border: 1px solid #9D9D9D; font-weight: normal; color: #313131; } -.ui-state-hover a, .ui-state-hover a:hover { color: #313131; text-decoration: none; } -.ui-state-active, .ui-widget-content .ui-state-active, .ui-widget-header .ui-state-active { - outline: none; - color: #1c4257; border: 1px solid #7096ab; - background: #ededed url(bg_fallback.png) 0 -50px repeat-x; /* Old browsers */ - background: -moz-linear-gradient(top, #b9e0f5 0%, #92bdd6 100%); /* FF3.6+ */ - background: -webkit-gradient(linear, left top, left bottom, color-stop(0%,#b9e0f5), color-stop(100%,#92bdd6)); /* Chrome,Safari4+ */ - background: -webkit-linear-gradient(top, #b9e0f5 0%,#92bdd6 100%); /* Chrome10+,Safari5.1+ */ - background: -o-linear-gradient(top, #b9e0f5 0%,#92bdd6 100%); /* Opera11.10+ */ - background: -ms-linear-gradient(top, #b9e0f5 0%,#92bdd6 100%); /* IE10+ */ - background: linear-gradient(top, #b9e0f5 0%,#92bdd6 100%); /* W3C */ - -webkit-box-shadow: none; - -moz-box-shadow: none; - box-shadow: none; -} -.ui-state-active a, .ui-state-active a:link, .ui-state-active a:visited { color: #313131; text-decoration: none; } -.ui-widget :active { outline: none; } - -/* Interaction Cues -----------------------------------*/ -.ui-state-highlight, .ui-widget-content .ui-state-highlight, .ui-widget-header .ui-state-highlight { border: 1px solid #d2dbf4; background: #f4f8fd; color: #0d2054; -moz-border-radius: 0 !important; -webkit-border-radius: 0 !important; border-radius: 0 !important; } -.ui-state-highlight a, .ui-widget-content .ui-state-highlight a,.ui-widget-header .ui-state-highlight a { color: #363636; } -.ui-state-error, .ui-widget-content .ui-state-error, .ui-widget-header .ui-state-error { border: 1px solid #e2d0d0; background: #fcf0f0; color: #280b0b; -moz-border-radius: 0 !important; -webkit-border-radius: 0 !important; border-radius: 0 !important; } -.ui-state-error a, .ui-widget-content .ui-state-error a, .ui-widget-header .ui-state-error a { color: #cd0a0a; } -.ui-state-error-text, .ui-widget-content .ui-state-error-text, .ui-widget-header .ui-state-error-text { color: #cd0a0a; } -.ui-priority-primary, .ui-widget-content .ui-priority-primary, .ui-widget-header .ui-priority-primary { font-weight: bold; } -.ui-priority-secondary, .ui-widget-content .ui-priority-secondary, .ui-widget-header .ui-priority-secondary { opacity: .7; filter:Alpha(Opacity=70); font-weight: normal; } -.ui-state-disabled, .ui-widget-content .ui-state-disabled, .ui-widget-header .ui-state-disabled { opacity: .35; filter:Alpha(Opacity=35); background-image: none; } - -/* Icons -----------------------------------*/ - -/* states and images */ -.ui-icon { width: 16px; height: 16px; background-image: url(ui-icons_222222_256x240.png); } -.ui-widget-content .ui-icon {background-image: url(ui-icons_222222_256x240.png); } -.ui-widget-header .ui-icon {background-image: url(ui-icons_222222_256x240.png); } -.ui-state-default .ui-icon { background-image: url(ui-icons_454545_256x240.png); } -.ui-state-hover .ui-icon, .ui-state-focus .ui-icon {background-image: url(ui-icons_454545_256x240.png); } -.ui-state-active .ui-icon {background-image: url(ui-icons_454545_256x240.png); } -.ui-state-highlight .ui-icon {background-image: url(ui-icons_454545_256x240.png); } -.ui-state-error .ui-icon, .ui-state-error-text .ui-icon { background: url(icon_sprite.png) -16px 0 no-repeat !important; } -.ui-state-highlight .ui-icon, .ui-state-error .ui-icon { margin-top: -1px; } - - -/* Misc visuals -----------------------------------*/ - -/* Overlays */ -.ui-widget-overlay { background: #262b33; opacity: .70;filter:Alpha(Opacity=70); } -.ui-widget-shadow { margin: -8px 0 0 -8px; padding: 8px; background: #000000; opacity: .30;filter:Alpha(Opacity=30); -moz-border-radius: 8px; -webkit-border-radius: 8px; border-radius: 8px; } -/* - * jQuery UI Selectable 1.8.7 - * - * Copyright 2010, AUTHORS.txt (http://jqueryui.com/about) - * Dual licensed under the MIT or GPL Version 2 licenses. - * http://jquery.org/license - * - * http://docs.jquery.com/UI/Selectable#theming - */ -.ui-selectable-helper { position: absolute; z-index: 100; border:1px dotted black; } -/* - * jQuery UI Autocomplete 1.8.7 - * - * Copyright 2010, AUTHORS.txt (http://jqueryui.com/about) - * Dual licensed under the MIT or GPL Version 2 licenses. - * http://jquery.org/license - * - * http://docs.jquery.com/UI/Autocomplete#theming - */ -.ui-autocomplete { - position: absolute; cursor: default; z-index: 3; - -moz-border-radius: 0; - -webkit-border-radius: 0; - border-radius: 0; - -moz-box-shadow: 0 1px 5px rgba(0,0,0,0.3); - -webkit-box-shadow: 0 1px 5px rgba(0,0,0,0.3); - box-shadow: 0 1px 5px rgba(0,0,0,0.3); -} - -/* workarounds */ -* html .ui-autocomplete { width:1px; } /* without this, the menu expands to 100% in IE6 */ - -/* - * jQuery UI Menu 1.8.7 - * - * Copyright 2010, AUTHORS.txt (http://jqueryui.com/about) - * Dual licensed under the MIT or GPL Version 2 licenses. - * http://jquery.org/license - * - * http://docs.jquery.com/UI/Menu#theming - */ -.ui-menu { - list-style:none; - padding: 1px; - margin: 0; - display:block; - float: left; -} -.ui-menu .ui-menu { - margin-top: -3px; -} -.ui-menu .ui-menu-item { - margin:0; - padding: 0; - zoom: 1; - float: left; - clear: left; - width: 100%; -} -.ui-menu .ui-menu-item a { - text-decoration:none; - display:block; - padding:.2em .4em; - line-height:1.5; - zoom:1; - color: #666; - font-size: 13px; -} -.ui-menu .ui-menu-item a.ui-state-hover, -.ui-menu .ui-menu-item a.ui-state-active { - font-weight: normal; - margin: -1px; - background: #D9EDF7; - color: #3A89A3; - text-shadow: 0px 1px 1px #fff; - border: none; - border: 1px solid #ADE; - cursor: pointer; - font-weight: bold; -} - -/* - * jQuery UI Datepicker 1.8.7 - * - * Copyright 2010, AUTHORS.txt (http://jqueryui.com/about) - * Dual licensed under the MIT or GPL Version 2 licenses. - * http://jquery.org/license - * - * http://docs.jquery.com/UI/Datepicker#theming - */ -.ui-datepicker { - width: 17em; - padding: 0; - display: none; - border-color: #DDDDDD; - border: none; - box-shadow: none; -} -.ui-datepicker .ui-datepicker-header { - position:relative; - padding:.35em 0; - border: none; - border-bottom: 1px solid #B6B6B6; - -moz-border-radius: 0; - -webkit-border-radius: 0; - border-radius: 0; - margin-bottom: 10px; - border: 1px solid #bbb; - -webkit-box-shadow: 0 0 0 3px #F1F1F1; - -moz-box-shadow: 0 0 0 3px #f1f1f1; - -ms-box-shadow: 0 0 0 3px #f1f1f1; - -o-box-shadow: 0 0 0 3px #f1f1f1; - box-shadow: 0 0 0 3px #F1F1F1; -} - -.ui-datepicker .ui-datepicker-prev, .ui-datepicker .ui-datepicker-next { position:absolute; top: 6px; width: 1.8em; height: 1.8em; } -.ui-datepicker .ui-datepicker-prev-hover, .ui-datepicker .ui-datepicker-next-hover { border: 1px none; } -.ui-datepicker .ui-datepicker-prev { left:2px; } -.ui-datepicker .ui-datepicker-next { right:2px; } -.ui-datepicker .ui-datepicker-prev span { background-position: 0px -32px !important; } -.ui-datepicker .ui-datepicker-next span { background-position: -16px -32px !important; } -.ui-datepicker .ui-datepicker-prev-hover span { background-position: 0px -48px !important; } -.ui-datepicker .ui-datepicker-next-hover span { background-position: -16px -48px !important; } -.ui-datepicker .ui-datepicker-prev span, .ui-datepicker .ui-datepicker-next span { display: block; position: absolute; left: 50%; margin-left: -8px; top: 50%; margin-top: -8px; background: url(icon_sprite.png) no-repeat; } -.ui-datepicker .ui-datepicker-title { margin: 0 2.3em; line-height: 1.8em; text-align: center; font-size: 12px; text-shadow: 0 1px 0 rgba(255,255,255,0.6); } -.ui-datepicker .ui-datepicker-title select { font-size:1em; margin:1px 0; } -.ui-datepicker select.ui-datepicker-month-year {width: 100%;} -.ui-datepicker select.ui-datepicker-month, -.ui-datepicker select.ui-datepicker-year { width: 49%;} -.ui-datepicker table {width: 100%; font-size: .9em; border-collapse: collapse; margin:0 0 .4em; } -.ui-datepicker th { padding: .7em .3em; text-align: center; font-weight: bold; border: 0; } -.ui-datepicker td { border: 0; padding: 1px; line-height: 24px; background-color: #FFF!important; } -.ui-datepicker td span, .ui-datepicker td a { display: block; padding: .2em; text-align: right; text-decoration: none; } -.ui-datepicker .ui-datepicker-buttonpane { background-image: none; margin: .7em 0 0 0; padding:0 .2em; border-left: 0; border-right: 0; border-bottom: 0; } -.ui-datepicker .ui-datepicker-buttonpane button { float: right; margin: .5em .2em .4em; cursor: pointer; padding: .2em .6em .3em .6em; width:auto; overflow:visible; } -.ui-datepicker .ui-datepicker-buttonpane button.ui-datepicker-current { float:left; } -.ui-datepicker table .ui-state-highlight { border-color: #ADE; } -.ui-datepicker-calendar .ui-state-default { background: transparent; border-color: #FFF; } -.ui-datepicker-calendar .ui-state-active { background: #D9EDF7; border-color: #ADE; color: #3A89A3; font-weight: bold; text-shadow: 0 1px 1px #fff; } diff --git a/app/assets/stylesheets/main/fonts.scss b/app/assets/stylesheets/main/fonts.scss index 8cc9986415c..d90274a0db9 100644 --- a/app/assets/stylesheets/main/fonts.scss +++ b/app/assets/stylesheets/main/fonts.scss @@ -1,2 +1,3 @@ /** Typo **/ $monospace_font: 'Menlo', 'Liberation Mono', 'Consolas', 'Courier New', 'andale mono', 'lucida console', monospace; +$regular_font: "Helvetica Neue", Helvetica, Arial, sans-serif; From 0d41f6f0a3ab23cee63e349eda5fb79240734dd4 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 15:37:57 +0300 Subject: [PATCH 20/54] Remove issue observer Signed-off-by: Dmitriy Zaporozhets --- app/observers/issue_observer.rb | 29 ----------------------------- config/application.rb | 1 - 2 files changed, 30 deletions(-) delete mode 100644 app/observers/issue_observer.rb diff --git a/app/observers/issue_observer.rb b/app/observers/issue_observer.rb deleted file mode 100644 index b4880b12fd7..00000000000 --- a/app/observers/issue_observer.rb +++ /dev/null @@ -1,29 +0,0 @@ -class IssueObserver < BaseObserver - def after_close(issue, transition) - notification.close_issue(issue, current_user) - event_service.close_issue(issue, current_user) - create_note(issue) - execute_hooks(issue) - end - - def after_reopen(issue, transition) - event_service.reopen_issue(issue, current_user) - create_note(issue) - execute_hooks(issue) - end - - protected - - # Create issue note with service comment like 'Status changed to closed' - def create_note(issue) - Note.create_status_change_note(issue, issue.project, current_user, issue.state, current_commit) - end - - def create_assignee_note(issue) - Note.create_assignee_change_note(issue, issue.project, current_user, issue.assignee) - end - - def execute_hooks(issue) - issue.project.execute_hooks(issue.to_hook_data, :issue_hooks) - end -end diff --git a/config/application.rb b/config/application.rb index a782dd1d01e..f7791b47b04 100644 --- a/config/application.rb +++ b/config/application.rb @@ -21,7 +21,6 @@ module Gitlab # Activate observers that should always be running. config.active_record.observers = :milestone_observer, :project_activity_cache_observer, - :issue_observer, :key_observer, :merge_request_observer, :note_observer, From cc773654883d34c70462eeeeb280453473c35e21 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 15:38:24 +0300 Subject: [PATCH 21/54] Create services for issue close and reopen Signed-off-by: Dmitriy Zaporozhets --- app/services/git_push_service.rb | 7 +++---- app/services/issues/base_service.rb | 5 ----- app/services/issues/close_service.rb | 20 ++++++++++++++++++++ app/services/issues/reopen_service.rb | 19 +++++++++++++++++++ app/services/issues/update_service.rb | 2 +- 5 files changed, 43 insertions(+), 10 deletions(-) create mode 100644 app/services/issues/close_service.rb create mode 100644 app/services/issues/reopen_service.rb diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index fcc03c3e4b8..351b446457d 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -86,10 +86,9 @@ class GitPushService author = commit_user(commit) if !issues_to_close.empty? && is_default_branch - Thread.current[:current_user] = author - Thread.current[:current_commit] = commit - - issues_to_close.each { |i| i.close && i.save } + issues_to_close.each do |issue| + Issues::CloseService.new(project, author, {}).execute(issue, commit) + end end # Create cross-reference notes for any other references. Omit any issues that were referenced in an diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index e04c1c6fb7a..2e1e1f7e0f0 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -3,11 +3,6 @@ module Issues private - # Create issue note with service comment like 'Status changed to closed' - def create_note(issue) - Note.create_status_change_note(issue, issue.project, current_user, issue.state, current_commit) - end - def create_assignee_note(issue) Note.create_assignee_change_note(issue, issue.project, current_user, issue.assignee) end diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb new file mode 100644 index 00000000000..780bc8e4130 --- /dev/null +++ b/app/services/issues/close_service.rb @@ -0,0 +1,20 @@ +module Issues + class CloseService < BaseService + def execute(issue, commit = nil) + if issue.close + notification_service.close_issue(issue, current_user) + event_service.close_issue(issue, current_user) + create_note(issue, commit) + execute_hooks(issue) + end + + issue + end + + private + + def create_note(issue, current_commit) + Note.create_status_change_note(issue, issue.project, current_user, issue.state, current_commit) + end + end +end diff --git a/app/services/issues/reopen_service.rb b/app/services/issues/reopen_service.rb new file mode 100644 index 00000000000..743a5d6c4a8 --- /dev/null +++ b/app/services/issues/reopen_service.rb @@ -0,0 +1,19 @@ +module Issues + class ReopenService < BaseService + def execute(issue) + if issue.reopen + event_service.reopen_issue(issue, current_user) + create_note(issue, commit) + execute_hooks(issue) + end + + issue + end + + private + + def create_note(issue) + Note.create_status_change_note(issue, issue.project, current_user, issue.state, nil) + end + end +end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 4db9dee11d3..4d8d71b4c9f 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -5,7 +5,7 @@ module Issues issue.reset_events_cache if issue.is_being_reassigned? - notification.reassigned_issue(issue, current_user) + notification_service.reassigned_issue(issue, current_user) create_assignee_note(issue) end From 1ad79393ad9090b6c8c4c9d43e6058af4216394c Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Wed, 2 Apr 2014 14:42:35 +0200 Subject: [PATCH 22/54] Add print.css to asset precompile array. --- config/application.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/config/application.rb b/config/application.rb index a782dd1d01e..4970c9921cf 100644 --- a/config/application.rb +++ b/config/application.rb @@ -64,6 +64,7 @@ module Gitlab config.assets.enabled = true config.assets.paths << Emoji.images_path config.assets.precompile << "emoji/*.png" + config.assets.precompile << "print.css" # Version of your assets, change this if you want to expire all your assets config.assets.version = '1.0' From c66c84a40d79be3d8d361753c13245093cf7e672 Mon Sep 17 00:00:00 2001 From: Job van der Voort Date: Wed, 2 Apr 2014 14:45:59 +0200 Subject: [PATCH 23/54] add project features to doc/workflow --- doc/workflow/project_features.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 doc/workflow/project_features.md diff --git a/doc/workflow/project_features.md b/doc/workflow/project_features.md new file mode 100644 index 00000000000..6a5ba2639c4 --- /dev/null +++ b/doc/workflow/project_features.md @@ -0,0 +1,22 @@ +## Issues + +Issues is a really powerful, but lightweight issue tracking system. You can make tickets, assign them to people, file them under milestones, order them with labels and have discussion in them. They integrate deeply into GitLab and are easily referenced from anywhere by using # and the issuenumber. Internally, at GitLab.com, we use this for all our project management needs. + +## Merge Requests + +Using a merge request, you can review and discuss code before it is merged in the branch of your code. As with issues, it can be assigned; people, issues, etc can be refereced; milestones attached. We see it as an integral part of working together on code and couldn't work without it. + + +## Wiki + +This is a separate system for documentation, built right into GitLab. It is source controlled and is very convenient if you don't want to keep you documentation in your source code, but you do want to keep it in your GitLab project. + + +## Wall + +For simple, project specific conversations, the wall can be used. It's very lightweight and simple and works well if you're not interested in using issues, but still want to occasionally communicate within a project. + + +## Snippets + +Snippets are little bits of code or text. This is a nice place to put code or text that is used semi-regularly within the project, but does not belong in source control. For example, a specific config file that is used by > the team that is only valid for the people that work on the code. From ed67ba966336c71583229063a12553f432643d64 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 15:50:50 +0300 Subject: [PATCH 24/54] Add support for close/reopen actions in update service Signed-off-by: Dmitriy Zaporozhets --- app/services/issues/reopen_service.rb | 2 +- app/services/issues/update_service.rb | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/app/services/issues/reopen_service.rb b/app/services/issues/reopen_service.rb index 743a5d6c4a8..e1e5a97229c 100644 --- a/app/services/issues/reopen_service.rb +++ b/app/services/issues/reopen_service.rb @@ -3,7 +3,7 @@ module Issues def execute(issue) if issue.reopen event_service.reopen_issue(issue, current_user) - create_note(issue, commit) + create_note(issue) execute_hooks(issue) end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 4d8d71b4c9f..9e73953bdcc 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -1,7 +1,16 @@ module Issues class UpdateService < BaseService def execute(issue) - if issue.update_attributes(params) + state = params.delete('state_event') + + case state + when 'reopen' + Issues::ReopenService.new(project, current_user, {}).execute(issue) + when 'close' + Issues::CloseService.new(project, current_user, {}).execute(issue) + end + + if params.present? && issue.update_attributes(params) issue.reset_events_cache if issue.is_being_reassigned? From 7d8d9bd1a16e8cf1d87bc7e5115d6c1e7eb14c75 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 15:51:37 +0300 Subject: [PATCH 25/54] Remove issue observer tests Signed-off-by: Dmitriy Zaporozhets --- spec/observers/issue_observer_spec.rb | 99 --------------------------- 1 file changed, 99 deletions(-) delete mode 100644 spec/observers/issue_observer_spec.rb diff --git a/spec/observers/issue_observer_spec.rb b/spec/observers/issue_observer_spec.rb deleted file mode 100644 index 9a0a2c4329c..00000000000 --- a/spec/observers/issue_observer_spec.rb +++ /dev/null @@ -1,99 +0,0 @@ -require 'spec_helper' - -describe IssueObserver do - let(:some_user) { create :user } - let(:assignee) { create :user } - let(:author) { create :user } - let(:mock_issue) { create(:issue, assignee: assignee, author: author) } - - - before { subject.stub(:current_user).and_return(some_user) } - before { subject.stub(:current_commit).and_return(nil) } - before { subject.stub(notification: double('NotificationService').as_null_object) } - before { mock_issue.project.stub_chain(:repository, :commit).and_return(nil) } - - subject { IssueObserver.instance } - - describe '#after_create' do - it 'trigger notification to send emails' do - subject.should_receive(:notification) - - subject.after_create(mock_issue) - end - - it 'should create cross-reference notes' do - other_issue = create(:issue) - mock_issue.stub(references: [other_issue]) - - Note.should_receive(:create_cross_reference_note).with(other_issue, mock_issue, - some_user, mock_issue.project) - subject.after_create(mock_issue) - end - end - - context '#after_close' do - context 'a status "closed"' do - before { mock_issue.stub(state: 'closed') } - - it 'note is created if the issue is being closed' do - Note.should_receive(:create_status_change_note).with(mock_issue, mock_issue.project, some_user, 'closed', nil) - - subject.after_close(mock_issue, nil) - end - - it 'trigger notification to send emails' do - subject.notification.should_receive(:close_issue).with(mock_issue, some_user) - subject.after_close(mock_issue, nil) - end - - it 'appends a mention to the closing commit if one is present' do - commit = double('commit', gfm_reference: 'commit 123456') - subject.stub(current_commit: commit) - - Note.should_receive(:create_status_change_note).with(mock_issue, mock_issue.project, some_user, 'closed', commit) - - subject.after_close(mock_issue, nil) - end - end - - context 'a status "reopened"' do - before { mock_issue.stub(state: 'reopened') } - - it 'note is created if the issue is being reopened' do - Note.should_receive(:create_status_change_note).with(mock_issue, mock_issue.project, some_user, 'reopened', nil) - - subject.after_reopen(mock_issue, nil) - end - end - end - - context '#after_update' do - before(:each) do - mock_issue.stub(:is_being_reassigned?).and_return(false) - end - - context 'notification' do - it 'triggered if the issue is being reassigned' do - mock_issue.should_receive(:is_being_reassigned?).and_return(true) - subject.should_receive(:notification) - - subject.after_update(mock_issue) - end - - it 'is not triggered if the issue is not being reassigned' do - mock_issue.should_receive(:is_being_reassigned?).and_return(false) - subject.should_not_receive(:notification) - - subject.after_update(mock_issue) - end - end - - context 'cross-references' do - it 'notices added references' do - mock_issue.should_receive(:notice_added_references) - - subject.after_update(mock_issue) - end - end - end -end From 1b5fb4ac2187c0a9cd775686740615a9e2311646 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 16:08:49 +0300 Subject: [PATCH 26/54] Fix assignee change in Issues::UpdateService Signed-off-by: Dmitriy Zaporozhets --- app/services/issues/update_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 9e73953bdcc..06c38d80740 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -13,7 +13,7 @@ module Issues if params.present? && issue.update_attributes(params) issue.reset_events_cache - if issue.is_being_reassigned? + if issue.previous_changes.include?('assignee_id') notification_service.reassigned_issue(issue, current_user) create_assignee_note(issue) end From 03b8dcce87c976a37a4de044e9f7cdfb8d64a3b7 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Wed, 2 Apr 2014 15:25:26 +0200 Subject: [PATCH 27/54] Change the satellites directory permission in the install guide, add to rake check task. --- CHANGELOG | 1 + doc/install/installation.md | 1 + lib/tasks/gitlab/check.rake | 24 ++++++++++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index d936986a779..1b742a4d9b2 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -6,6 +6,7 @@ v 6.8.0 - Drop all tables before restoring a Postgres backup - Make the repository downloads path configurable - Create branches via API (sponsored by O'Reilly Media) + - Changed permission of gitlab-satellites directory not to be world accessible v 6.7.2 - Fix upgrader script diff --git a/doc/install/installation.md b/doc/install/installation.md index addb21b50e0..efcba2f69bf 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -202,6 +202,7 @@ You can change `6-6-stable` to `master` if you want the *bleeding edge* version, # Create directory for satellites sudo -u git -H mkdir /home/git/gitlab-satellites + sudo chmod o-rwx /home/git/gitlab-satellites # Create directories for sockets/pids and make sure GitLab can write to them sudo -u git -H mkdir tmp/pids/ diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 3b9b2531bf7..e9258cc626b 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -342,6 +342,7 @@ namespace :gitlab do check_repo_base_is_not_symlink check_repo_base_user_and_group check_repo_base_permissions + check_satellites_permissions check_update_hook_is_up_to_date check_repos_update_hooks_is_link check_gitlab_shell_self_test @@ -443,6 +444,29 @@ namespace :gitlab do end end + def check_satellites_permissions + print "Satellites access is drwxr-x---? ... " + + satellites_path = Gitlab.config.satellites.path + unless File.exists?(satellites_path) + puts "can't check because of previous errors".magenta + return + end + + if File.stat(satellites_path).mode.to_s(8).ends_with?("0750") + puts "yes".green + else + puts "no".red + try_fixing_it( + "sudo chmod u+rwx,g+rx,o-rwx #{satellites_path}", + ) + for_more_information( + see_installation_guide_section "GitLab" + ) + fix_and_rerun + end + end + def check_repo_base_user_and_group gitlab_shell_ssh_user = Gitlab.config.gitlab_shell.ssh_user gitlab_shell_owner_group = Gitlab.config.gitlab_shell.owner_group From 928fbeeec057692d923146994c4b8dff57024417 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 16:33:07 +0300 Subject: [PATCH 28/54] More tests for Isses services Signed-off-by: Dmitriy Zaporozhets --- spec/services/issues/close_service_spec.rb | 35 +++++++++++++++++++++ spec/services/issues/update_service_spec.rb | 24 ++++++++++++-- 2 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 spec/services/issues/close_service_spec.rb diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb new file mode 100644 index 00000000000..d4f2cc1339b --- /dev/null +++ b/spec/services/issues/close_service_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe Issues::CloseService do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:issue) { create(:issue, assignee: user2) } + + before do + project.team << [user, :master] + project.team << [user2, :developer] + end + + describe :execute do + context "valid params" do + before do + @issue = Issues::CloseService.new(project, user, {}).execute(issue) + end + + it { @issue.should be_valid } + it { @issue.should be_closed } + + it 'should send email to user2 about assign of new issue' do + email = ActionMailer::Base.deliveries.last + email.to.first.should == user2.email + email.subject.should include(issue.title) + end + + it 'should create system note about issue reassign' do + note = @issue.notes.last + note.note.should include "Status changed to closed" + end + end + end +end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 9bfc0f674de..347560414e7 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -3,15 +3,22 @@ require 'spec_helper' describe Issues::UpdateService do let(:project) { create(:empty_project) } let(:user) { create(:user) } + let(:user2) { create(:user) } let(:issue) { create(:issue) } + before do + project.team << [user, :master] + project.team << [user2, :developer] + end + describe :execute do context "valid params" do before do - project.team << [user, :master] opts = { title: 'New title', - description: 'Also please fix' + description: 'Also please fix', + assignee_id: user2.id, + state_event: 'close' } @issue = Issues::UpdateService.new(project, user, opts).execute(issue) @@ -19,6 +26,19 @@ describe Issues::UpdateService do it { @issue.should be_valid } it { @issue.title.should == 'New title' } + it { @issue.assignee.should == user2 } + it { @issue.should be_closed } + + it 'should send email to user2 about assign of new issue' do + email = ActionMailer::Base.deliveries.last + email.to.first.should == user2.email + email.subject.should include(issue.title) + end + + it 'should create system note about issue reassign' do + note = @issue.notes.last + note.note.should include "Reassigned to \@#{user2.username}" + end end end end From 80bc7c4ca536e1f3e6192ae8779ba3f8f443cef4 Mon Sep 17 00:00:00 2001 From: dosire Date: Wed, 2 Apr 2014 16:10:53 +0200 Subject: [PATCH 29/54] More explicit warning since we still see many people running into problems with this. --- doc/install/installation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/install/installation.md b/doc/install/installation.md index addb21b50e0..61903de0fe1 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -93,7 +93,7 @@ Then select 'Internet Site' and press enter to confirm the hostname. # 2. Ruby -The use of ruby version managers such as [RVM](http://rvm.io/), [rbenv](https://github.com/sstephenson/rbenv) or [chruby](https://github.com/postmodern/chruby) with GitLab in production frequently leads to hard to diagnose problems. Version managers are not supported and we stronly advise everyone to follow the instructions below to use a system ruby. +The use of ruby version managers such as [RVM](http://rvm.io/), [rbenv](https://github.com/sstephenson/rbenv) or [chruby](https://github.com/postmodern/chruby) with GitLab in production frequently leads to hard to diagnose problems. For example, GitLab Shell is called from OpenSSH and having a version manager can prevent pushing and pulling over SSH. Version managers are not supported and we stronly advise everyone to follow the instructions below to use a system ruby. Remove the old Ruby 1.8 if present From 3acad539becb29a778bc438003db9e8e260b6411 Mon Sep 17 00:00:00 2001 From: Job van der Voort Date: Wed, 2 Apr 2014 16:15:55 +0200 Subject: [PATCH 30/54] workflow index; newlines; small description --- doc/README.md | 2 +- doc/workflow/README.md | 2 ++ doc/workflow/project_features.md | 23 ++++++++++++++++++----- 3 files changed, 21 insertions(+), 6 deletions(-) create mode 100644 doc/workflow/README.md diff --git a/doc/README.md b/doc/README.md index d85a2d854d3..3b942abc399 100644 --- a/doc/README.md +++ b/doc/README.md @@ -15,4 +15,4 @@ + [System hooks](system_hooks/system_hooks.md) + [Update](update/README.md) + [Web hooks](web_hooks/web_hooks.md) -+ [Workflow](workflow/workflow.md) ++ [Workflow](workflow/README.md) diff --git a/doc/workflow/README.md b/doc/workflow/README.md new file mode 100644 index 00000000000..efc28d06e71 --- /dev/null +++ b/doc/workflow/README.md @@ -0,0 +1,2 @@ ++ [Workflow](workflow/workflow.md) ++ [Project Features](workflow/project_features.md) diff --git a/doc/workflow/project_features.md b/doc/workflow/project_features.md index 6a5ba2639c4..25fe4032570 100644 --- a/doc/workflow/project_features.md +++ b/doc/workflow/project_features.md @@ -1,22 +1,35 @@ +When in a Project -> Settings, you will find Features on the bottom of the page that you can toggle. +Below you will find a more elaborate explanation of each of these. + + ## Issues -Issues is a really powerful, but lightweight issue tracking system. You can make tickets, assign them to people, file them under milestones, order them with labels and have discussion in them. They integrate deeply into GitLab and are easily referenced from anywhere by using # and the issuenumber. Internally, at GitLab.com, we use this for all our project management needs. +Issues is a really powerful, but lightweight issue tracking system. +You can make tickets, assign them to people, file them under milestones, order them with labels and have discussion in them. +They integrate deeply into GitLab and are easily referenced from anywhere by using # and the issuenumber. +At GitLab.com, we use this for all our project management needs. ## Merge Requests -Using a merge request, you can review and discuss code before it is merged in the branch of your code. As with issues, it can be assigned; people, issues, etc can be refereced; milestones attached. We see it as an integral part of working together on code and couldn't work without it. +Using a merge request, you can review and discuss code before it is merged in the branch of your code. +As with issues, it can be assigned; people, issues, etc. can be refereced; milestones attached. +We see it as an integral part of working together on code and couldn't work without it. ## Wiki -This is a separate system for documentation, built right into GitLab. It is source controlled and is very convenient if you don't want to keep you documentation in your source code, but you do want to keep it in your GitLab project. +This is a separate system for documentation, built right into GitLab. +It is source controlled and is very convenient if you don't want to keep you documentation in your source code, but you do want to keep it in your GitLab project. ## Wall -For simple, project specific conversations, the wall can be used. It's very lightweight and simple and works well if you're not interested in using issues, but still want to occasionally communicate within a project. +For simple, project specific conversations, the wall can be used. +It's very lightweight and simple and works well if you're not interested in using issues, but still want to occasionally communicate within a project. ## Snippets -Snippets are little bits of code or text. This is a nice place to put code or text that is used semi-regularly within the project, but does not belong in source control. For example, a specific config file that is used by > the team that is only valid for the people that work on the code. +Snippets are little bits of code or text. +This is a nice place to put code or text that is used semi-regularly within the project, but does not belong in source control. +For example, a specific config file that is used by > the team that is only valid for the people that work on the code. From 49f977d675b44b6aae0f491fbbf86dd1ee05eb64 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 19:55:23 +0300 Subject: [PATCH 31/54] Fix tests Signed-off-by: Dmitriy Zaporozhets --- app/services/issues/close_service.rb | 2 +- app/services/issues/create_service.rb | 2 +- app/services/issues/reopen_service.rb | 2 +- app/services/issues/update_service.rb | 2 +- spec/services/git_push_service_spec.rb | 8 +------- 5 files changed, 5 insertions(+), 11 deletions(-) diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index 780bc8e4130..85c0226ccab 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -1,5 +1,5 @@ module Issues - class CloseService < BaseService + class CloseService < Issues::BaseService def execute(issue, commit = nil) if issue.close notification_service.close_issue(issue, current_user) diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 8008086dae9..d6137833bb9 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -1,5 +1,5 @@ module Issues - class CreateService < BaseService + class CreateService < Issues::BaseService def execute issue = project.issues.new(params) issue.author = current_user diff --git a/app/services/issues/reopen_service.rb b/app/services/issues/reopen_service.rb index e1e5a97229c..a931398aff6 100644 --- a/app/services/issues/reopen_service.rb +++ b/app/services/issues/reopen_service.rb @@ -1,5 +1,5 @@ module Issues - class ReopenService < BaseService + class ReopenService < Issues::BaseService def execute(issue) if issue.reopen event_service.reopen_issue(issue, current_user) diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 06c38d80740..b562c401fd4 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -1,5 +1,5 @@ module Issues - class UpdateService < BaseService + class UpdateService < Issues::BaseService def execute(issue) state = params.delete('state_event') diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 90738c681fa..6b89f213bec 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -170,16 +170,10 @@ describe GitPushService do Issue.find(issue.id).should be_closed end - it "passes the closing commit as a thread-local" do - service.execute(project, user, @oldrev, @newrev, @ref) - - Thread.current[:current_commit].should == closing_commit - end - it "doesn't create cross-reference notes for a closing reference" do expect { service.execute(project, user, @oldrev, @newrev, @ref) - }.not_to change { Note.where(project_id: project.id, system: true).count } + }.not_to change { Note.where(project_id: project.id, system: true, commit_id: closing_commit.id).count } end it "doesn't close issues when pushed to non-default branches" do From 599344583bcf57eff46b36d68e6536188d41c906 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 21:02:02 +0300 Subject: [PATCH 32/54] Remove EmailsObserver Signed-off-by: Dmitriy Zaporozhets --- app/models/email.rb | 11 ++++++++--- app/observers/email_observer.rb | 5 ----- 2 files changed, 8 insertions(+), 8 deletions(-) delete mode 100644 app/observers/email_observer.rb diff --git a/app/models/email.rb b/app/models/email.rb index 22e71e4f107..b92c1841063 100644 --- a/app/models/email.rb +++ b/app/models/email.rb @@ -13,14 +13,15 @@ class Email < ActiveRecord::Base # Relations # belongs_to :user - + # # Validations # validates :user_id, presence: true validates :email, presence: true, email: { strict_mode: true }, uniqueness: true validate :unique_email, if: ->(email) { email.email_changed? } - + + after_create :notify before_validation :cleanup_email def cleanup_email @@ -30,4 +31,8 @@ class Email < ActiveRecord::Base def unique_email self.errors.add(:email, 'has already been taken') if User.exists?(email: self.email) end -end \ No newline at end of file + + def notify + NotificationService.new.new_email(self) + end +end diff --git a/app/observers/email_observer.rb b/app/observers/email_observer.rb deleted file mode 100644 index 026ad8b1d9a..00000000000 --- a/app/observers/email_observer.rb +++ /dev/null @@ -1,5 +0,0 @@ -class EmailObserver < BaseObserver - def after_create(email) - notification.new_email(email) - end -end From 77c0a6db4baac1bd91e3636b32b3ffa9d0373211 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 21:03:31 +0300 Subject: [PATCH 33/54] Remove email observer specs Signed-off-by: Dmitriy Zaporozhets --- spec/observers/email_observer_spec.rb | 17 ----------------- 1 file changed, 17 deletions(-) delete mode 100644 spec/observers/email_observer_spec.rb diff --git a/spec/observers/email_observer_spec.rb b/spec/observers/email_observer_spec.rb deleted file mode 100644 index 599b9a6ffba..00000000000 --- a/spec/observers/email_observer_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -require 'spec_helper' - -describe EmailObserver do - let(:email) { create(:email) } - - before { subject.stub(notification: double('NotificationService').as_null_object) } - - subject { EmailObserver.instance } - - describe '#after_create' do - it 'trigger notification to send emails' do - subject.should_receive(:notification) - - subject.after_create(email) - end - end -end From 6ae2529041258c2d63a992298c9708cc03efd8d1 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 21:13:05 +0300 Subject: [PATCH 34/54] Remove KeysObserver Signed-off-by: Dmitriy Zaporozhets --- app/models/key.rb | 24 ++++++++++++++++++++++++ app/observers/key_observer.rb | 19 ------------------- config/application.rb | 1 - spec/models/key_spec.rb | 14 ++++++++++++++ spec/observers/key_observer_spec.rb | 23 ----------------------- 5 files changed, 38 insertions(+), 43 deletions(-) delete mode 100644 app/observers/key_observer.rb delete mode 100644 spec/observers/key_observer_spec.rb diff --git a/app/models/key.rb b/app/models/key.rb index 29a76f53f3d..4202d79a956 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -29,6 +29,10 @@ class Key < ActiveRecord::Base delegate :name, :email, to: :user, prefix: true + after_create :add_to_shell + after_create :notify_user + after_destroy :remove_from_shell + def strip_white_space self.key = key.strip unless key.blank? end @@ -42,6 +46,26 @@ class Key < ActiveRecord::Base "key-#{id}" end + def add_to_shell + GitlabShellWorker.perform_async( + :add_key, + shell_id, + key + ) + end + + def notify_user + NotificationService.new.new_key(self) + end + + def remove_from_shell + GitlabShellWorker.perform_async( + :remove_key, + shell_id, + key, + ) + end + private def generate_fingerpint diff --git a/app/observers/key_observer.rb b/app/observers/key_observer.rb deleted file mode 100644 index c013594e787..00000000000 --- a/app/observers/key_observer.rb +++ /dev/null @@ -1,19 +0,0 @@ -class KeyObserver < BaseObserver - def after_create(key) - GitlabShellWorker.perform_async( - :add_key, - key.shell_id, - key.key - ) - - notification.new_key(key) - end - - def after_destroy(key) - GitlabShellWorker.perform_async( - :remove_key, - key.shell_id, - key.key, - ) - end -end diff --git a/config/application.rb b/config/application.rb index 3933c046af0..439f98ff052 100644 --- a/config/application.rb +++ b/config/application.rb @@ -21,7 +21,6 @@ module Gitlab # Activate observers that should always be running. config.active_record.observers = :milestone_observer, :project_activity_cache_observer, - :key_observer, :merge_request_observer, :note_observer, :project_observer, diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 9c872c02a53..c1c6c2f31b7 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -68,4 +68,18 @@ describe Key do build(:invalid_key).should_not be_valid end end + + context 'callbacks' do + it 'should add new key to authorized_file' do + @key = build(:personal_key, id: 7) + GitlabShellWorker.should_receive(:perform_async).with(:add_key, @key.shell_id, @key.key) + @key.save + end + + it 'should remove key from authorized_file' do + @key = create(:personal_key) + GitlabShellWorker.should_receive(:perform_async).with(:remove_key, @key.shell_id, @key.key) + @key.destroy + end + end end diff --git a/spec/observers/key_observer_spec.rb b/spec/observers/key_observer_spec.rb deleted file mode 100644 index b304bf1fcb7..00000000000 --- a/spec/observers/key_observer_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper' - -describe KeyObserver do - before do - @key = create(:personal_key) - - @observer = KeyObserver.instance - end - - context :after_create do - it do - GitlabShellWorker.should_receive(:perform_async).with(:add_key, @key.shell_id, @key.key) - @observer.after_create(@key) - end - end - - context :after_destroy do - it do - GitlabShellWorker.should_receive(:perform_async).with(:remove_key, @key.shell_id, @key.key) - @observer.after_destroy(@key) - end - end -end From 65cd9829eecd2485f17eaadf84107631d5a723d9 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 21:35:59 +0300 Subject: [PATCH 35/54] Remove MergeRequest observer Signed-off-by: Dmitriy Zaporozhets --- app/observers/merge_request_observer.rb | 43 ------ config/application.rb | 1 - spec/observers/merge_request_observer_spec.rb | 131 ------------------ 3 files changed, 175 deletions(-) delete mode 100644 app/observers/merge_request_observer.rb delete mode 100644 spec/observers/merge_request_observer_spec.rb diff --git a/app/observers/merge_request_observer.rb b/app/observers/merge_request_observer.rb deleted file mode 100644 index 04ee30b4976..00000000000 --- a/app/observers/merge_request_observer.rb +++ /dev/null @@ -1,43 +0,0 @@ -class MergeRequestObserver < BaseObserver - def after_create(merge_request) - event_service.open_mr(merge_request, current_user) - notification.new_merge_request(merge_request, current_user) - merge_request.create_cross_references!(merge_request.project, current_user) - execute_hooks(merge_request) - end - - def after_close(merge_request, transition) - event_service.close_mr(merge_request, current_user) - notification.close_mr(merge_request, current_user) - create_note(merge_request) - execute_hooks(merge_request) - end - - def after_reopen(merge_request, transition) - event_service.reopen_mr(merge_request, current_user) - create_note(merge_request) - execute_hooks(merge_request) - merge_request.reload_code - merge_request.mark_as_unchecked - end - - def after_update(merge_request) - notification.reassigned_merge_request(merge_request, current_user) if merge_request.is_being_reassigned? - - merge_request.notice_added_references(merge_request.project, current_user) - execute_hooks(merge_request) - end - - private - - # Create merge request note with service comment like 'Status changed to closed' - def create_note(merge_request) - Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil) - end - - def execute_hooks(merge_request) - if merge_request.project - merge_request.project.execute_hooks(merge_request.to_hook_data, :merge_request_hooks) - end - end -end diff --git a/config/application.rb b/config/application.rb index 439f98ff052..76b19eeb529 100644 --- a/config/application.rb +++ b/config/application.rb @@ -21,7 +21,6 @@ module Gitlab # Activate observers that should always be running. config.active_record.observers = :milestone_observer, :project_activity_cache_observer, - :merge_request_observer, :note_observer, :project_observer, :system_hook_observer, diff --git a/spec/observers/merge_request_observer_spec.rb b/spec/observers/merge_request_observer_spec.rb deleted file mode 100644 index 18df8b78513..00000000000 --- a/spec/observers/merge_request_observer_spec.rb +++ /dev/null @@ -1,131 +0,0 @@ -require 'spec_helper' - -describe MergeRequestObserver do - let(:some_user) { create :user } - let(:assignee) { create :user } - let(:author) { create :user } - let(:project) { create :project } - let(:mr_mock) { double(:merge_request, id: 42, assignee: assignee, author: author).as_null_object } - let(:assigned_mr) { create(:merge_request, assignee: assignee, author: author, source_project: project) } - let(:unassigned_mr) { create(:merge_request, author: author, source_project: project) } - let(:closed_assigned_mr) { create(:closed_merge_request, assignee: assignee, author: author, source_project: project) } - let(:closed_unassigned_mr) { create(:closed_merge_request, author: author, source_project: project) } - - before { subject.stub(:current_user).and_return(some_user) } - before { subject.stub(notification: double('NotificationService').as_null_object) } - before { mr_mock.stub(:author_id) } - before { mr_mock.stub(:source_project) } - before { mr_mock.stub(:source_project) } - before { mr_mock.stub(:project) } - before { mr_mock.stub(:create_cross_references!).and_return(true) } - before { Repository.any_instance.stub(commit: nil) } - - before(:each) { enable_observers } - after(:each) { disable_observers } - - subject { MergeRequestObserver.instance } - - describe '#after_create' do - it 'trigger notification service' do - subject.should_receive(:notification) - subject.after_create(mr_mock) - end - - it 'creates cross-reference notes' do - project = create :project - mr_mock.stub(title: "this mr references !#{assigned_mr.id}", project: project) - mr_mock.should_receive(:create_cross_references!).with(project, some_user) - - subject.after_create(mr_mock) - end - end - - context '#after_update' do - before(:each) do - mr_mock.stub(:is_being_reassigned?).and_return(false) - mr_mock.stub(:notice_added_references) - end - - it 'is called when a merge request is changed' do - changed = create(:merge_request, source_project: project) - subject.should_receive(:after_update) - - MergeRequest.observers.enable :merge_request_observer do - changed.title = 'I changed' - changed.save - end - end - - it 'checks for new references' do - mr_mock.should_receive(:notice_added_references) - - subject.after_update(mr_mock) - end - - context 'a notification' do - it 'is sent if the merge request is being reassigned' do - mr_mock.should_receive(:is_being_reassigned?).and_return(true) - subject.should_receive(:notification) - - subject.after_update(mr_mock) - end - - it 'is not sent if the merge request is not being reassigned' do - mr_mock.should_receive(:is_being_reassigned?).and_return(false) - subject.should_not_receive(:notification) - - subject.after_update(mr_mock) - end - end - end - - context '#after_close' do - context 'a status "closed"' do - it 'note is created if the merge request is being closed' do - Note.should_receive(:create_status_change_note).with(assigned_mr, assigned_mr.source_project, some_user, 'closed', nil) - - assigned_mr.close - end - - it 'notification is delivered only to author if the merge request is being closed' do - Note.should_receive(:create_status_change_note).with(unassigned_mr, unassigned_mr.source_project, some_user, 'closed', nil) - - unassigned_mr.close - end - end - end - - context '#after_reopen' do - context 'a status "reopened"' do - it 'note is created if the merge request is being reopened' do - Note.should_receive(:create_status_change_note).with(closed_assigned_mr, closed_assigned_mr.source_project, some_user, 'reopened', nil) - - closed_assigned_mr.reopen - end - - it 'notification is delivered only to author if the merge request is being reopened' do - Note.should_receive(:create_status_change_note).with(closed_unassigned_mr, closed_unassigned_mr.source_project, some_user, 'reopened', nil) - - closed_unassigned_mr.reopen - end - end - end - - describe "Merge Request created" do - def self.it_should_be_valid_event - it { @event.should_not be_nil } - it { @event.should_not be_nil } - it { @event.project.should == project } - it { @event.project.should == project } - end - - before do - @merge_request = create(:merge_request, source_project: project, target_project: project) - @event = Event.last - end - - it_should_be_valid_event - it { @event.action.should == Event::CREATED } - it { @event.target.should == @merge_request } - end -end From 3c867dfa8efec28155158e3b2b7dbb21ba3ce0a1 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 21:51:17 +0300 Subject: [PATCH 36/54] MergeRequest services for create, update, close and reopen Signed-off-by: Dmitriy Zaporozhets --- app/services/merge_requests/base_service.rb | 16 +++++++++ app/services/merge_requests/close_service.rb | 18 ++++++++++ app/services/merge_requests/create_service.rb | 18 ++++++++++ app/services/merge_requests/reopen_service.rb | 15 +++++++++ app/services/merge_requests/update_service.rb | 33 +++++++++++++++++++ 5 files changed, 100 insertions(+) create mode 100644 app/services/merge_requests/base_service.rb create mode 100644 app/services/merge_requests/close_service.rb create mode 100644 app/services/merge_requests/create_service.rb create mode 100644 app/services/merge_requests/reopen_service.rb create mode 100644 app/services/merge_requests/update_service.rb diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb new file mode 100644 index 00000000000..a1261972157 --- /dev/null +++ b/app/services/merge_requests/base_service.rb @@ -0,0 +1,16 @@ +module MergeRequests + class BaseService < ::BaseService + + private + + def create_note(merge_request) + Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil) + end + + def execute_hooks(merge_request) + if merge_request.project + merge_request.project.execute_hooks(merge_request.to_hook_data, :merge_request_hooks) + end + end + end +end diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb new file mode 100644 index 00000000000..64e37a23e6b --- /dev/null +++ b/app/services/merge_requests/close_service.rb @@ -0,0 +1,18 @@ +module MergeRequests + class CloseService < MergeRequests::BaseService + def execute(merge_request, commit = nil) + # If we close MergeRequest we want to ignore validation + # so we can close broken one (Ex. fork project removed) + merge_request.allow_broken = true + + if merge_request.close + event_service.close_mr(merge_request, current_user) + notification_service.close_mr(merge_request, current_user) + create_note(merge_request) + execute_hooks(merge_request) + end + + merge_request + end + end +end diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb new file mode 100644 index 00000000000..3717d4d1fc4 --- /dev/null +++ b/app/services/merge_requests/create_service.rb @@ -0,0 +1,18 @@ +module MergeReques + class CreateService < MergeRequests::BaseService + def execute + merge_request = MergeRequest.new(params) + merge_request.source_project = project + merge_request.author = current_user + + if merge_request.save + event_service.open_mr(merge_request, current_user) + notification_service.new_merge_request(merge_request, current_user) + merge_request.create_cross_references!(merge_request.project, current_user) + execute_hooks(merge_request) + end + + merge_request + end + end +end diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb new file mode 100644 index 00000000000..2eb13d3e0e1 --- /dev/null +++ b/app/services/merge_requests/reopen_service.rb @@ -0,0 +1,15 @@ +module MergeRequests + class ReopenService < MergeRequests::BaseService + def execute(merge_request) + if merge_request.reopen + event_service.reopen_mr(merge_request, current_user) + create_note(merge_request) + execute_hooks(merge_request) + merge_request.reload_code + merge_request.mark_as_unchecked + end + + merge_request + end + end +end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb new file mode 100644 index 00000000000..743930f5e8b --- /dev/null +++ b/app/services/merge_requests/update_service.rb @@ -0,0 +1,33 @@ +module MergeRequests + class UpdateService < MergeRequests::BaseService + def execute(merge_request) + # We dont allow change of source/target projects + # after merge request was created + params.delete(:source_project_id) + params.delete(:target_project_id) + + state = params.delete('state_event') + + case state + when 'reopen' + MergeRequests::ReopenService.new(project, current_user, {}).execute(merge_request) + when 'close' + MergeRequests::CloseService.new(project, current_user, {}).execute(merge_request) + end + + if params.present? && merge_request.update_attributes(params) + merge_request.reset_events_cache + + if merge_request.previous_changes.include?('assignee_id') + notification_service.reassigned_merge_request(merge_request, current_user) + create_assignee_note(merge_request) + end + + merge_request.notice_added_references(merge_request.project, current_user) + execute_hooks(merge_request) + end + + merge_request + end + end +end From 9ee697dd6847de0bd1a38714df7d8bb509534d20 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 2 Apr 2014 21:51:53 +0300 Subject: [PATCH 37/54] Use MergeRequest services in API and controllers Signed-off-by: Dmitriy Zaporozhets --- .../projects/merge_requests_controller.rb | 30 ++-------- app/models/merge_request.rb | 15 +++++ app/models/project.rb | 4 ++ .../projects/merge_requests/_form.html.haml | 2 +- lib/api/merge_requests.rb | 56 ++++++------------- 5 files changed, 41 insertions(+), 66 deletions(-) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index e6edceb1fbc..c090dd917c2 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -76,10 +76,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def create - @merge_request = MergeRequest.new(params[:merge_request]) - @merge_request.author = current_user @target_branches ||= [] - if @merge_request.save + @merge_request = MergeRequests::CreateService.new(project, current_user, params[:merge_request]).execute + + if @merge_request.valid? redirect_to [@merge_request.target_project, @merge_request], notice: 'Merge request was successfully created.' else @source_project = @merge_request.source_project @@ -89,29 +89,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def update - # If we close MergeRequest we want to ignore validation - # so we can close broken one (Ex. fork project removed) - if params[:merge_request] == {"state_event"=>"close"} - @merge_request.allow_broken = true - - if @merge_request.close - opts = { notice: 'Merge request was successfully closed.' } - else - opts = { alert: 'Failed to close merge request.' } - end - - redirect_to [@merge_request.target_project, @merge_request], opts - return - end - - # We dont allow change of source/target projects - # after merge request was created - params[:merge_request].delete(:source_project_id) - params[:merge_request].delete(:target_project_id) - - if @merge_request.update_attributes(params[:merge_request]) - @merge_request.reset_events_cache + @merge_request = MergeRequests::UpdateService.new(project, current_user, params[:merge_request]).execute(@merge_request) + if @merge_request.valid? respond_to do |format| format.js format.html do diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5c2b0656d40..ffed03307ec 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -97,6 +97,7 @@ class MergeRequest < ActiveRecord::Base validates :target_project, presence: true validates :target_branch, presence: true validate :validate_branches + validate :validate_fork scope :of_group, ->(group) { where("source_project_id in (:group_project_ids) OR target_project_id in (:group_project_ids)", group_project_ids: group.project_ids) } scope :of_user_team, ->(team) { where("(source_project_id in (:team_project_ids) OR target_project_id in (:team_project_ids) AND assignee_id in (:team_member_ids))", team_project_ids: team.project_ids, team_member_ids: team.member_ids) } @@ -125,6 +126,20 @@ class MergeRequest < ActiveRecord::Base end end + def validate_fork + if target_projet == source_project + true + else + # If source and target projects are different + # we should check if source project is actually a fork of target project + if source_project.forked_from?(target_project) + true + else + errors.add :base, "Source project is not a fork of target project" + end + end + end + def update_merge_request_diff if source_branch_changed? || target_branch_changed? reload_code diff --git a/app/models/project.rb b/app/models/project.rb index 769ab217625..79066e1c54a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -552,4 +552,8 @@ class Project < ActiveRecord::Base gitlab_shell.update_repository_head(self.path_with_namespace, branch) reload_default_branch end + + def forked_from?(project) + forked? && project == forked_from_project + end end diff --git a/app/views/projects/merge_requests/_form.html.haml b/app/views/projects/merge_requests/_form.html.haml index 22502760e50..0fe2d1d9801 100644 --- a/app/views/projects/merge_requests/_form.html.haml +++ b/app/views/projects/merge_requests/_form.html.haml @@ -33,7 +33,7 @@ .col-sm-10 .clearfix .pull-left - - projects = @project.forked_from_project.nil? ? [@project] : [ @project,@project.forked_from_project] + - projects = @project.forked_from_project.nil? ? [@project] : [@project, @project.forked_from_project] = f.select(:target_project_id, options_from_collection_for_select(projects, 'id', 'path_with_namespace', f.object.target_project_id), {}, { class: 'target_project select2 span3', disabled: @merge_request.persisted? }) .pull-left   diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 3a1a00d0719..e2d2d034444 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -13,14 +13,6 @@ module API end not_found! end - - def not_fork?(target_project_id, user_project) - target_project_id.nil? || target_project_id == user_project.id.to_s - end - - def target_matches_fork(target_project_id,user_project) - user_project.forked? && user_project.forked_from_project.id.to_s == target_project_id - end end # List merge requests @@ -70,29 +62,15 @@ module API # POST /projects/:id/merge_requests # post ":id/merge_requests" do - set_current_user_for_thread do - authorize! :write_merge_request, user_project - required_attributes! [:source_branch, :target_branch, :title] - attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id, :description] - merge_request = user_project.merge_requests.new(attrs) - merge_request.author = current_user - merge_request.source_project = user_project - target_project_id = attrs[:target_project_id] - if not_fork?(target_project_id, user_project) - merge_request.target_project = user_project - else - if target_matches_fork(target_project_id,user_project) - merge_request.target_project = Project.find_by(id: attrs[:target_project_id]) - else - render_api_error!('(Bad Request) Specified target project that is not the source project, or the source fork of the project.', 400) - end - end + authorize! :write_merge_request, user_project + required_attributes! [:source_branch, :target_branch, :title] + attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id, :description] + merge_request = ::MergeRequests::CreateService.new(user_project, current_user, attrs).execute - if merge_request.save - present merge_request, with: Entities::MergeRequest - else - handle_merge_request_errors! merge_request.errors - end + if merge_request.valid? + present merge_request, with: Entities::MergeRequest + else + handle_merge_request_errors! merge_request.errors end end @@ -111,17 +89,15 @@ module API # PUT /projects/:id/merge_request/:merge_request_id # put ":id/merge_request/:merge_request_id" do - set_current_user_for_thread do - attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :state_event, :description] - merge_request = user_project.merge_requests.find(params[:merge_request_id]) + attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :state_event, :description] + merge_request = user_project.merge_requests.find(params[:merge_request_id]) + authorize! :modify_merge_request, merge_request + merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, attrs).execute(merge_request) - authorize! :modify_merge_request, merge_request - - if merge_request.update_attributes attrs - present merge_request, with: Entities::MergeRequest - else - handle_merge_request_errors! merge_request.errors - end + if merge_request.valid? + present merge_request, with: Entities::MergeRequest + else + handle_merge_request_errors! merge_request.errors end end From 14ac0746712ac11e823a856b4c15cd5d91ea541e Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Thu, 3 Apr 2014 08:43:40 +0200 Subject: [PATCH 38/54] Remove modernizr gem. --- Gemfile | 1 - Gemfile.lock | 3 --- 2 files changed, 4 deletions(-) diff --git a/Gemfile b/Gemfile index ab8a27a8571..1088a91b933 100644 --- a/Gemfile +++ b/Gemfile @@ -159,7 +159,6 @@ gem 'select2-rails' gem 'jquery-atwho-rails', "~> 0.3.3" gem "jquery-rails", "2.1.3" gem "jquery-ui-rails", "2.0.2" -gem "modernizr", "2.6.2" gem "raphael-rails", "~> 2.1.2" gem 'bootstrap-sass', '~> 3.0' gem "font-awesome-rails", '~> 3.2' diff --git a/Gemfile.lock b/Gemfile.lock index 4acdb539f09..009edd3ea3c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -285,8 +285,6 @@ GEM method_source (0.8.2) mime-types (1.25.1) minitest (4.7.5) - modernizr (2.6.2) - sprockets (~> 2.0) multi_json (1.8.4) multi_xml (0.5.5) multipart-post (1.2.0) @@ -616,7 +614,6 @@ DEPENDENCIES launchy letter_opener minitest (~> 4.7.0) - modernizr (= 2.6.2) mysql2 nprogress-rails omniauth (~> 1.1.3) From 7ab46b969f5c817acc849e0ed6ce23d25bd5fa09 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Thu, 3 Apr 2014 08:49:34 +0200 Subject: [PATCH 39/54] Do not require modernizr. --- app/assets/javascripts/application.js.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/app/assets/javascripts/application.js.coffee b/app/assets/javascripts/application.js.coffee index 5042221abe4..a22ff6dec31 100644 --- a/app/assets/javascripts/application.js.coffee +++ b/app/assets/javascripts/application.js.coffee @@ -18,7 +18,6 @@ #= require turbolinks #= require jquery.turbolinks #= require bootstrap -#= require modernizr #= require select2 #= require raphael #= require g.raphael-min From 9b598fa62d73c5a66d7fed872db943859e2d6a5a Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 3 Apr 2014 10:36:10 +0300 Subject: [PATCH 40/54] Fix MR fork validation and services load Signed-off-by: Dmitriy Zaporozhets --- app/models/merge_request.rb | 4 +++- app/services/merge_requests/create_service.rb | 2 +- app/services/merge_requests/update_service.rb | 4 ++++ features/steps/dashboard/merge_requests.rb | 6 +++--- features/support/env.rb | 2 -- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index ffed03307ec..0decc7782ee 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -127,7 +127,9 @@ class MergeRequest < ActiveRecord::Base end def validate_fork - if target_projet == source_project + return true unless target_project && source_project + + if target_project == source_project true else # If source and target projects are different diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 3717d4d1fc4..6f7121a51d0 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -1,4 +1,4 @@ -module MergeReques +module MergeRequests class CreateService < MergeRequests::BaseService def execute merge_request = MergeRequest.new(params) diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 743930f5e8b..bbedca6ffd6 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -1,3 +1,7 @@ +require_relative 'base_service' +require_relative 'reopen_service' +require_relative 'close_service' + module MergeRequests class UpdateService < MergeRequests::BaseService def execute(merge_request) diff --git a/features/steps/dashboard/merge_requests.rb b/features/steps/dashboard/merge_requests.rb index 62d84506c49..e198bc0cf9c 100644 --- a/features/steps/dashboard/merge_requests.rb +++ b/features/steps/dashboard/merge_requests.rb @@ -53,15 +53,15 @@ class DashboardMergeRequests < Spinach::FeatureSteps end def assigned_merge_request - @assigned_merge_request ||= create :merge_request, assignee: current_user, target_project: project + @assigned_merge_request ||= create :merge_request, assignee: current_user, target_project: project, source_project: project end def authored_merge_request - @authored_merge_request ||= create :merge_request, author: current_user, target_project: project + @authored_merge_request ||= create :merge_request, source_branch: 'simple_merge_request', author: current_user, target_project: project, source_project: project end def other_merge_request - @other_merge_request ||= create :merge_request, target_project: project + @other_merge_request ||= create :merge_request, source_branch: '2_3_notes_fix', target_project: project, source_project: project end def project diff --git a/features/support/env.rb b/features/support/env.rb index 7b11f5a7c6f..a5b297775db 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -52,6 +52,4 @@ Spinach.hooks.before_run do RSpec::Mocks::setup self include FactoryGirl::Syntax::Methods - MergeRequestObserver.any_instance.stub(current_user: create(:user)) end - From 770747af68b82642fa98e69c79c9574321504167 Mon Sep 17 00:00:00 2001 From: dosire Date: Thu, 3 Apr 2014 10:09:52 +0200 Subject: [PATCH 41/54] Specify that the tag should be annotated. --- doc/release/monthly.md | 4 ++-- doc/release/security.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/release/monthly.md b/doc/release/monthly.md index 08149b4da86..2949e320f3c 100644 --- a/doc/release/monthly.md +++ b/doc/release/monthly.md @@ -61,9 +61,9 @@ After making the release branch new commits are cherry-picked from master. When * 1-7th: official merge window (see contributing guide) * 8-14th: work on bugfixes, sponsored features and GitLab EE * 15th: code freeze (stop merging into master except essential bugfixes) -* 18th: release candidate 1 (VERSION x.x.0.rc1, tag and tweet about x.x.0.rc1, release on GitLab Cloud) +* 18th: release candidate 1 (VERSION x.x.0.rc1, annotated tag and tweet about x.x.0.rc1, release on GitLab Cloud) * 20st: optional release candidate 2 (x.x.0.rc2, only if rc1 had problems) -* 22nd: release (VERSION x.x.0, create x-x-stable branch, tag, blog and tweet) +* 22nd: release (VERSION x.x.0, create x-x-stable branch, annotated tag tag, blog and tweet) * 23nd: optional patch releases (x.x.1, x.x.2, etc., only if there are serious problems) * 24-end of month: release GitLab EE and GitLab CI diff --git a/doc/release/security.md b/doc/release/security.md index 8e5a7e32099..56a44b5d1da 100644 --- a/doc/release/security.md +++ b/doc/release/security.md @@ -18,7 +18,7 @@ Please report suspected security vulnerabilities in private to support@gitlab.co 1. Create feature branches for the blog post on GitLab.com and link them from the code branch 1. Merge the code feature branch into master 1. Cherry-pick the code into the latest stable branch -1. Create a git tag vX.X.X for CE and another patch release for EE +1. Create an annotated tag vX.X.X for CE and another patch release for EE 1. Push the code and the tags to all the CE and EE repositories 1. Apply the patch to GitLab Cloud and the private GitLab development server 1. Merge and publish the blog posts From fa27b633f8bbeda837651d99a9e2109b1390a889 Mon Sep 17 00:00:00 2001 From: dosire Date: Thu, 3 Apr 2014 10:13:14 +0200 Subject: [PATCH 42/54] Update changelog in master. --- CHANGELOG | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 1b742a4d9b2..64a40ca6936 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,6 +8,11 @@ v 6.8.0 - Create branches via API (sponsored by O'Reilly Media) - Changed permission of gitlab-satellites directory not to be world accessible +v 6.7.3 + - Fix the merge notification email not being sent (Pierre de La Morinerie) + - Drop all tables before restoring a Postgres backup + - Remove yanked modernizr gem + v 6.7.2 - Fix upgrader script From c31b703110620c438d724561ab6d5a5a44b08e19 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 3 Apr 2014 11:47:56 +0300 Subject: [PATCH 43/54] Fix tests that dont respect project-fork relation Signed-off-by: Dmitriy Zaporozhets --- spec/finders/merge_requests_finder_spec.rb | 8 ++++---- spec/models/note_spec.rb | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 0bd2ccafcc1..94b4d4c4ff4 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -5,10 +5,10 @@ describe MergeRequestsFinder do let(:user2) { create :user } let(:project1) { create(:project) } - let(:project2) { create(:project) } + let(:project2) { create(:project, forked_from_project: project1) } - let!(:merge_request1) { create(:merge_request, :simple, author: user, source_project: project1, target_project: project2) } - let!(:merge_request2) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1) } + let!(:merge_request1) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1) } + let!(:merge_request2) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1, state: 'closed') } let!(:merge_request3) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project2) } before do @@ -21,7 +21,7 @@ describe MergeRequestsFinder do it 'should filter by scope' do params = { scope: 'authored', state: 'opened' } merge_requests = MergeRequestsFinder.new.execute(user, params) - merge_requests.size.should == 3 + merge_requests.size.should == 2 end it 'should filter by project' do diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 6be8a6a13f6..4cdda1feb31 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -209,7 +209,7 @@ describe Note do let(:project) { create(:project) } let(:author) { create(:user) } let(:issue) { create(:issue, project: project) } - let(:mergereq) { create(:merge_request, target_project: project) } + let(:mergereq) { create(:merge_request, :simple, target_project: project, source_project: project) } let(:commit) { project.repository.commit } # Test all of {issue, merge request, commit} in both the referenced and referencing From e1dd76d5a52391626d538bf9803a32c1f0e83287 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 3 Apr 2014 11:49:12 +0300 Subject: [PATCH 44/54] Fix tests that dont respect project-fork relation. pt2 Signed-off-by: Dmitriy Zaporozhets --- spec/lib/gitlab/satellite/merge_action_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/gitlab/satellite/merge_action_spec.rb b/spec/lib/gitlab/satellite/merge_action_spec.rb index ef06c742846..41d3321b173 100644 --- a/spec/lib/gitlab/satellite/merge_action_spec.rb +++ b/spec/lib/gitlab/satellite/merge_action_spec.rb @@ -13,7 +13,7 @@ describe 'Gitlab::Satellite::MergeAction' do end let(:project) { create(:project, namespace: create(:group)) } - let(:fork_project) { create(:project, namespace: create(:group)) } + let(:fork_project) { create(:project, namespace: create(:group), forked_from_project: project) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:merge_request_fork) { create(:merge_request, source_project: fork_project, target_project: project) } From 9b6224f99cacdce3bcbec9a2c1ef4d8af296c56c Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 3 Apr 2014 12:05:21 +0300 Subject: [PATCH 45/54] More test fixes Signed-off-by: Dmitriy Zaporozhets --- app/services/merge_requests/create_service.rb | 1 + spec/requests/api/merge_requests_spec.rb | 20 ++++++++----------- spec/services/notification_service_spec.rb | 4 ++-- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 6f7121a51d0..d1bf827f3fc 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -3,6 +3,7 @@ module MergeRequests def execute merge_request = MergeRequest.new(params) merge_request.source_project = project + merge_request.target_project ||= project merge_request.author = current_user if merge_request.save diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 138f218d46c..9530f7ceb04 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -6,7 +6,7 @@ describe API::API do after(:each) { ActiveRecord::Base.observers.disable(:user_observer) } let(:user) { create(:user) } let!(:project) {create(:project, creator_id: user.id, namespace: user.namespace) } - let!(:merge_request) { create(:merge_request, author: user, assignee: user, source_project: project, target_project: project, title: "Test") } + let!(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, target_project: project, title: "Test") } let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") } before { project.team << [user, :reporters] @@ -79,16 +79,12 @@ describe API::API do end context 'forked projects' do - let!(:user2) {create(:user)} - let!(:forked_project_link) { build(:forked_project_link) } - let!(:fork_project) { create(:project, forked_project_link: forked_project_link, namespace: user2.namespace, creator_id: user2.id) } - let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) } + let!(:user2) { create(:user) } + let!(:fork_project) { create(:project, forked_from_project: project, namespace: user2.namespace, creator_id: user2.id) } + let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) } before :each do |each| fork_project.team << [user2, :reporters] - forked_project_link.forked_from_project = project - forked_project_link.forked_to_project = fork_project - forked_project_link.save! end it "should return merge_request" do @@ -127,16 +123,16 @@ describe API::API do response.status.should == 400 end - it "should return 400 when target_branch is specified and not a forked project" do + it "should return 404 when target_branch is specified and not a forked project" do post api("/projects/#{project.id}/merge_requests", user), title: 'Test merge_request', target_branch: 'master', source_branch: 'stable', author: user, target_project_id: fork_project.id - response.status.should == 400 + response.status.should == 404 end - it "should return 400 when target_branch is specified and for a different fork" do + it "should return 404 when target_branch is specified and for a different fork" do post api("/projects/#{fork_project.id}/merge_requests", user2), title: 'Test merge_request', target_branch: 'master', source_branch: 'stable', author: user2, target_project_id: unrelated_project.id - response.status.should == 400 + response.status.should == 404 end it "should return 201 when target_branch is specified and for the same project" do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 0c25f7a71d0..57a4240f7a2 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -5,7 +5,7 @@ describe NotificationService do describe 'Keys' do describe :new_key do - let(:key) { create(:personal_key) } + let!(:key) { create(:personal_key) } it { notification.new_key(key).should be_true } @@ -18,7 +18,7 @@ describe NotificationService do describe 'Email' do describe :new_email do - let(:email) { create(:email) } + let!(:email) { create(:email) } it { notification.new_email(email).should be_true } From 0e7665e09b75a5ddeed16df894f31a632d7338aa Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Thu, 3 Apr 2014 13:24:05 +0200 Subject: [PATCH 46/54] Only send email when project access level changes. --- app/observers/users_project_observer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/observers/users_project_observer.rb b/app/observers/users_project_observer.rb index 93233898cc8..44c72b30187 100644 --- a/app/observers/users_project_observer.rb +++ b/app/observers/users_project_observer.rb @@ -10,7 +10,7 @@ class UsersProjectObserver < BaseObserver end def after_update(users_project) - notification.update_team_member(users_project) + notification.update_team_member(users_project) if users_project.project_access_changed? end def after_destroy(users_project) From 53504928427ad6ffcc451bff9da86d9fbad580b8 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 3 Apr 2014 15:03:45 +0300 Subject: [PATCH 47/54] Fix support for force_push Signed-off-by: Dmitriy Zaporozhets --- lib/gitlab/git_access.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index a0401290cc0..eefdb1833fc 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -43,9 +43,8 @@ module Gitlab def push_allowed?(user, project, ref, oldrev, newrev, forced_push) if user && user_allowed?(user) - action = if project.protected_branch?(ref) - if forced_push + if forced_push.to_s == 'true' :force_push_code_to_protected_branches else :push_code_to_protected_branches From 8cc5e85543b0a553536ebe70f01fa29e0b0f60bd Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 3 Apr 2014 17:06:11 +0300 Subject: [PATCH 48/54] Mention protected branch force push Signed-off-by: Dmitriy Zaporozhets --- CHANGELOG | 1 + app/views/projects/protected_branches/index.html.haml | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 64a40ca6936..59e3e22524b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,6 +7,7 @@ v 6.8.0 - Make the repository downloads path configurable - Create branches via API (sponsored by O'Reilly Media) - Changed permission of gitlab-satellites directory not to be world accessible + - Protected branch does not allow force push v 6.7.3 - Fix the merge notification email not being sent (Pierre de La Morinerie) diff --git a/app/views/projects/protected_branches/index.html.haml b/app/views/projects/protected_branches/index.html.haml index 8b100766e97..4a6e8943a9f 100644 --- a/app/views/projects/protected_branches/index.html.haml +++ b/app/views/projects/protected_branches/index.html.haml @@ -9,6 +9,7 @@ %ul %li keep stable branches secured %li forced code review before merge to protected branches + %li prevents branch from force push %p Read more about project permissions #{link_to "here", help_permissions_path, class: "underlined-link"} - if can? current_user, :admin_project, @project From eca269f7f98b3a2455e7788c884bcb927ea693b1 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Thu, 3 Apr 2014 16:35:33 +0200 Subject: [PATCH 49/54] Fix user_project observer test. --- spec/observers/users_project_observer_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/observers/users_project_observer_spec.rb b/spec/observers/users_project_observer_spec.rb index be277b4dbd2..b024465e8c3 100644 --- a/spec/observers/users_project_observer_spec.rb +++ b/spec/observers/users_project_observer_spec.rb @@ -21,7 +21,7 @@ describe UsersProjectObserver do it "should send email to user" do subject.should_receive(:notification) - @users_project.update_attribute(:project_access, UsersProject::MASTER) + @users_project.update_attribute(:project_access, UsersProject::OWNER) end it "should not called after UsersProject destroyed" do From 860f159d41b7a013108a111fefa224e365f4a337 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 3 Apr 2014 18:16:05 +0300 Subject: [PATCH 50/54] Add note when MR assignee changes. Add more tests to MR services Signed-off-by: Dmitriy Zaporozhets --- app/services/merge_requests/base_service.rb | 4 ++ .../merge_requests/close_service_spec.rb | 35 +++++++++++++++ .../merge_requests/create_service_spec.rb | 25 +++++++++++ .../merge_requests/update_service_spec.rb | 44 +++++++++++++++++++ 4 files changed, 108 insertions(+) create mode 100644 spec/services/merge_requests/close_service_spec.rb create mode 100644 spec/services/merge_requests/create_service_spec.rb create mode 100644 spec/services/merge_requests/update_service_spec.rb diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index a1261972157..c77f5d664ef 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -3,6 +3,10 @@ module MergeRequests private + def create_assignee_note(merge_request) + Note.create_assignee_change_note(merge_request, merge_request.project, current_user, merge_request.assignee) + end + def create_note(merge_request) Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil) end diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb new file mode 100644 index 00000000000..a504f916b08 --- /dev/null +++ b/spec/services/merge_requests/close_service_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe MergeRequests::CloseService do + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:merge_request) { create(:merge_request, assignee: user2) } + let(:project) { merge_request.project } + + before do + project.team << [user, :master] + project.team << [user2, :developer] + end + + describe :execute do + context "valid params" do + before do + @merge_request = MergeRequests::CloseService.new(project, user, {}).execute(merge_request) + end + + it { @merge_request.should be_valid } + it { @merge_request.should be_closed } + + it 'should send email to user2 about assign of new merge_request' do + email = ActionMailer::Base.deliveries.last + email.to.first.should == user2.email + email.subject.should include(merge_request.title) + end + + it 'should create system note about merge_request reassign' do + note = @merge_request.notes.last + note.note.should include "Status changed to closed" + end + end + end +end diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb new file mode 100644 index 00000000000..cebeb0644d0 --- /dev/null +++ b/spec/services/merge_requests/create_service_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe MergeRequests::CreateService do + let(:project) { create(:project) } + let(:user) { create(:user) } + + describe :execute do + context "valid params" do + before do + project.team << [user, :master] + opts = { + title: 'Awesome merge_request', + description: 'please fix', + source_branch: 'stable', + target_branch: 'master' + } + + @merge_request = MergeRequests::CreateService.new(project, user, opts).execute + end + + it { @merge_request.should be_valid } + it { @merge_request.title.should == 'Awesome merge_request' } + end + end +end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb new file mode 100644 index 00000000000..af5d3a3dc81 --- /dev/null +++ b/spec/services/merge_requests/update_service_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe MergeRequests::UpdateService do + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:merge_request) { create(:merge_request, :simple) } + let(:project) { merge_request.project } + + before do + project.team << [user, :master] + project.team << [user2, :developer] + end + + describe :execute do + context "valid params" do + before do + opts = { + title: 'New title', + description: 'Also please fix', + assignee_id: user2.id, + state_event: 'close' + } + + @merge_request = MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) + end + + it { @merge_request.should be_valid } + it { @merge_request.title.should == 'New title' } + it { @merge_request.assignee.should == user2 } + it { @merge_request.should be_closed } + + it 'should send email to user2 about assign of new merge_request' do + email = ActionMailer::Base.deliveries.last + email.to.first.should == user2.email + email.subject.should include(merge_request.title) + end + + it 'should create system note about merge_request reassign' do + note = @merge_request.notes.last + note.note.should include "Reassigned to \@#{user2.username}" + end + end + end +end From 04324908fb14fe2bb0600a164a60870100122d3f Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Fri, 4 Apr 2014 09:36:25 +0200 Subject: [PATCH 51/54] Add to update guides satellites group permission. --- doc/install/installation.md | 2 +- doc/update/6.0-to-6.7.md | 3 +++ doc/update/6.6-to-6.7.md | 3 +++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/doc/install/installation.md b/doc/install/installation.md index b640996c28c..bc194d33927 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -202,7 +202,7 @@ You can change `6-6-stable` to `master` if you want the *bleeding edge* version, # Create directory for satellites sudo -u git -H mkdir /home/git/gitlab-satellites - sudo chmod o-rwx /home/git/gitlab-satellites + sudo chmod u+rwx,g+rx,o-rwx /home/git/gitlab-satellites # Create directories for sockets/pids and make sure GitLab can write to them sudo -u git -H mkdir tmp/pids/ diff --git a/doc/update/6.0-to-6.7.md b/doc/update/6.0-to-6.7.md index 5023e34f189..aa1b388fa9a 100644 --- a/doc/update/6.0-to-6.7.md +++ b/doc/update/6.0-to-6.7.md @@ -80,6 +80,9 @@ sudo -u git -H bundle exec rake migrate_iids RAILS_ENV=production # Clean up assets and cache sudo -u git -H bundle exec rake assets:clean assets:precompile cache:clear RAILS_ENV=production + +# Close access to gitlab-satellites for others +sudo chmod u+rwx,g+rx,o-rwx /home/git/gitlab-satellites ``` ### 6. Update config files diff --git a/doc/update/6.6-to-6.7.md b/doc/update/6.6-to-6.7.md index 8a16e5d67be..0f39c037c9f 100644 --- a/doc/update/6.6-to-6.7.md +++ b/doc/update/6.6-to-6.7.md @@ -63,6 +63,9 @@ sudo cp lib/support/init.d/gitlab /etc/init.d/gitlab # Update the logrotate configuration (keep logs for 90 days instead of 52 weeks) sudo cp lib/support/logrotate/gitlab /etc/logrotate.d/gitlab + +# Close access to gitlab-satellites for others +sudo chmod u+rwx,g+rx,o-rwx /home/git/gitlab-satellites ``` From 68436e964fa1220c3fa040fa501324a729eabb70 Mon Sep 17 00:00:00 2001 From: Remy van Elst Date: Fri, 4 Apr 2014 10:19:40 +0200 Subject: [PATCH 52/54] Add one line upgrade command. --- doc/update/upgrader.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/update/upgrader.md b/doc/update/upgrader.md index 305ef961be5..fd45154ac82 100644 --- a/doc/update/upgrader.md +++ b/doc/update/upgrader.md @@ -40,3 +40,10 @@ To make sure you didn't miss anything run a more thorough check with: sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production If all items are green, then congratulations upgrade is complete! + + +### One line upgrade command + +You've read through the entire guide, and probably did all the steps manually. Here is a one liner for convenience, the next time you upgrade: + + cd /home/git/gitlab; sudo -u git -H bundle exec rake gitlab:backup:create RAILS_ENV=production; sudo service gitlab stop; sudo -u git -H ruby script/upgrade.rb -y; sudo service gitlab start; sudo service nginx restart; sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production From e159a5615721f4742c39872e02cb316dd511b2e3 Mon Sep 17 00:00:00 2001 From: dosire Date: Fri, 4 Apr 2014 15:52:07 +0200 Subject: [PATCH 53/54] Ask for small feature requests. --- CONTRIBUTING.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bdf5e09831a..d960f8e4333 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,7 +29,9 @@ If something is wrong but it is not a regression compared to older versions of G When submitting an issue please conform to the issue submission guidelines listed below. Not all issues will be addressed and your issue is more likely to be addressed if you submit a merge request which partially or fully addresses the issue. -Do not use the issue tracker for feature requests. We have a specific [feature request forum](http://feedback.gitlab.com) for this purpose. +Do not use the issue tracker for feature requests. +We have a specific [feature request forum](http://feedback.gitlab.com) for this purpose. +Please keep feature requests as small and simple as possible, complex ones might be edited to make them small and simple. Please send a merge request with a tested solution or a merge request with a failing test instead of opening an issue if you can. If you're unsure where to post, post to the [mailing list](https://groups.google.com/forum/#!forum/gitlabhq) or [Stack Overflow](http://stackoverflow.com/questions/tagged/gitlab) first. There are a lot of helpful GitLab users there who may be able to help you quickly. If your particular issue turns out to be a bug, it will find its way from there. From bb9ec71127965fc92f98a7f0aeacf666424861c8 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 4 Apr 2014 16:22:11 +0200 Subject: [PATCH 54/54] Explain how existing users can enable LDAP --- doc/integration/ldap.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 doc/integration/ldap.md diff --git a/doc/integration/ldap.md b/doc/integration/ldap.md new file mode 100644 index 00000000000..b52c4a4b5e4 --- /dev/null +++ b/doc/integration/ldap.md @@ -0,0 +1,14 @@ +# GitLab LDAP integration + +GitLab can be configured to allow your users to sign with their LDAP credentials to integrate with e.g. Active Directory. +The first time a user signs in with LDAP credentials, GitLab will create a new GitLab user associated with the LDAP Distinguished Name (DN) of the LDAP user. +GitLab user attributes such as nickname and email will be copied from the LDAP user entry. + +## Enabling LDAP sign-in for existing GitLab users + +When a user signs in to GitLab with LDAP for the first time, and their LDAP email address is the primary email address of an existing GitLab user, then the LDAP DN will be associated with the existing user. +If the LDAP email attribute is not found in GitLab's database, a new user is created. + +In other words, if an existing GitLab user wants to enable LDAP sign-in for themselves, they should check that their GitLab email address matches their LDAP email address, and then sign into GitLab via their LDAP credentials. +GitLab recognizes the following LDAP attributes as email addresses: `mail`, `email` and `userPrincipalName`. +If multiple LDAP email attributes are present, e.g. `mail: foo@bar.com` and `email: foo@example.com`, then the first attribute found wins -- in this case `foo@bar.com`.