diff options
25 files changed, 218 insertions, 60 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c23a7a3bf0e..ac8390074f4 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -8,7 +8,7 @@ before_script: - touch log/application.log - touch log/test.log - bundle install --without postgres production --jobs $(nproc) "${FLAGS[@]}" - - bundle exec rake db:reset db:create RAILS_ENV=test + - RAILS_ENV=test bundle exec rake db:drop db:create db:schema:load db:migrate spec:feature: script: @@ -118,7 +118,7 @@ flay: - mysql bundler:audit: - script: + script: - "bundle exec bundle-audit update" - "bundle exec bundle-audit check" tags: diff --git a/CHANGELOG b/CHANGELOG index 5795395beeb..adda5373c3c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 8.5.0 (unreleased) v 8.4.0 (unreleased) - Ensure Gravatar host looks like an actual host + - Consider re-assign as a mention from a notification point of view - Add pagination headers to already paginated API resources - Properly generate diff of orphan commits, like the first commit in a repository - Improve the consistency of commit titles, branch names, tag names, issue/MR titles, on their respective project pages @@ -65,6 +66,8 @@ v 8.4.0 (unreleased) - Add build artifacts browser - Improve UX in builds artifacts browser - Increase default size of `data` column in `events` table when using MySQL + - Expose button to CI Lint tool on project builds page + - Fix: Creator should be added as a master of the project on creation v 8.3.4 - Use gitlab-workhorse 0.5.4 (fixes API routing bug) diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index 8e6922c9231..b7638c86bfa 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -1,5 +1,5 @@ .filter-item { - margin-right: 15px; + margin-right: 6px; } @media (min-width: 800px) { diff --git a/app/assets/stylesheets/pages/commits.scss b/app/assets/stylesheets/pages/commits.scss index 800df95cff3..818fd03e2ae 100644 --- a/app/assets/stylesheets/pages/commits.scss +++ b/app/assets/stylesheets/pages/commits.scss @@ -36,6 +36,10 @@ li.commit { line-height: 20px; margin-bottom: 2px; + .btn-clipboard { + margin-top: -1px; + } + .notes_count { float: right; margin-right: 10px; diff --git a/app/assets/stylesheets/pages/search.scss b/app/assets/stylesheets/pages/search.scss index 3aaa96da609..bdcf1897522 100644 --- a/app/assets/stylesheets/pages/search.scss +++ b/app/assets/stylesheets/pages/search.scss @@ -3,6 +3,10 @@ border-bottom: 1px solid #DDD; padding-bottom: 15px; margin-bottom: 15px; + + .term { + height: 22px; + } } } diff --git a/app/models/project.rb b/app/models/project.rb index 7e131151513..cb668e0c2f7 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -468,12 +468,9 @@ class Project < ActiveRecord::Base !external_issue_tracker end - def external_issues_trackers - services.select(&:issue_tracker?).reject(&:default?) - end - def external_issue_tracker - @external_issues_tracker ||= external_issues_trackers.find(&:activated?) + @external_issue_tracker ||= + services.issue_trackers.active.without_defaults.first end def can_have_issues_tracker_id? diff --git a/app/models/project_services/ci_service.rb b/app/models/project_services/ci_service.rb index c3f70d1f972..e10b5529b42 100644 --- a/app/models/project_services/ci_service.rb +++ b/app/models/project_services/ci_service.rb @@ -23,14 +23,12 @@ # List methods you need to implement to get your CI service # working with GitLab Merge Requests class CiService < Service - def category - :ci - end - + default_value_for :category, 'ci' + def valid_token?(token) self.respond_to?(:token) && self.token.present? && self.token == token end - + def supported_events %w(push) end diff --git a/app/models/project_services/gitlab_issue_tracker_service.rb b/app/models/project_services/gitlab_issue_tracker_service.rb index 7aa04309f54..05436cd0f79 100644 --- a/app/models/project_services/gitlab_issue_tracker_service.rb +++ b/app/models/project_services/gitlab_issue_tracker_service.rb @@ -24,9 +24,7 @@ class GitlabIssueTrackerService < IssueTrackerService prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url - def default? - true - end + default_value_for :default, true def to_param 'gitlab' diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index ed201979d39..25045224ce5 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -23,12 +23,10 @@ class IssueTrackerService < Service validates :project_url, :issues_url, :new_issue_url, presence: true, if: :activated? - def category - :issue_tracker - end + default_value_for :category, 'issue_tracker' def default? - false + default end def issue_url(iid) diff --git a/app/models/service.rb b/app/models/service.rb index 24f4bf7646e..721273250ea 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -43,6 +43,9 @@ class Service < ActiveRecord::Base validates :project_id, presence: true, unless: Proc.new { |service| service.template? } scope :visible, -> { where.not(type: ['GitlabIssueTrackerService', 'GitlabCiService']) } + scope :issue_trackers, -> { where(category: 'issue_tracker') } + scope :active, -> { where(active: true) } + scope :without_defaults, -> { where(default: false) } scope :push_hooks, -> { where(push_events: true, active: true) } scope :tag_push_hooks, -> { where(tag_push_events: true, active: true) } @@ -51,6 +54,8 @@ class Service < ActiveRecord::Base scope :note_hooks, -> { where(note_events: true, active: true) } scope :build_hooks, -> { where(build_events: true, active: true) } + default_value_for :category, 'common' + def activated? active end @@ -60,7 +65,7 @@ class Service < ActiveRecord::Base end def category - :common + read_attribute(:category).to_sym end def initialize_properties @@ -153,7 +158,7 @@ class Service < ActiveRecord::Base # Returns a hash of the properties that have been assigned a new value since last save, # indicating their original values (attr => original value). - # ActiveRecord does not provide a mechanism to track changes in serialized keys, + # ActiveRecord does not provide a mechanism to track changes in serialized keys, # so we need a specific implementation for service properties. # This allows to track changes to properties set with the accessor methods, # but not direct manipulation of properties hash. @@ -164,7 +169,7 @@ class Service < ActiveRecord::Base def reset_updated_properties @updated_properties = nil end - + def async_execute(data) return unless supported_events.include?(data[:object_kind]) diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index e4edc55bf69..ca8a41d93b8 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -376,10 +376,10 @@ class NotificationService end def reassign_resource_email(target, project, current_user, method) - previous_assignee_id = previous_record(target, "assignee_id") + previous_assignee_id = previous_record(target, 'assignee_id') previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id - recipients = build_recipients(target, project, current_user, [previous_assignee]) + recipients = build_recipients(target, project, current_user, action: :reassign, previous_assignee: previous_assignee) recipients.each do |recipient| mailer.send( @@ -400,22 +400,27 @@ class NotificationService end end - def build_recipients(target, project, current_user, extra_recipients = nil) + def build_recipients(target, project, current_user, action: nil, previous_assignee: nil) recipients = target.participants(current_user) - recipients = recipients.concat(extra_recipients).compact.uniq if extra_recipients - recipients = add_project_watchers(recipients, project) recipients = reject_mention_users(recipients, project) - recipients = reject_muted_users(recipients, project) + # Re-assign is considered as a mention of the new assignee so we add the + # new assignee to the list of recipients after we rejected users with + # the "on mention" notification level + if action == :reassign + recipients << previous_assignee if previous_assignee + recipients << target.assignee + end + + recipients = reject_muted_users(recipients, project) recipients = add_subscribed_users(recipients, target) recipients = reject_unsubscribed_users(recipients, target) recipients.delete(current_user) - recipients = recipients.uniq - recipients + recipients.uniq end def mailer diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index a6820183bee..c94d7ab710f 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -95,7 +95,7 @@ module Projects system_hook_service.execute_hooks_for(@project, :create) unless @project.group - @project.team << [current_user, :master, current_user] + @project.team << [current_user, :master] end @project.import_start if @project.import? diff --git a/app/views/projects/artifacts/browse.html.haml b/app/views/projects/artifacts/browse.html.haml index 9dc7b2c2e1f..b70c776a2b2 100644 --- a/app/views/projects/artifacts/browse.html.haml +++ b/app/views/projects/artifacts/browse.html.haml @@ -23,6 +23,10 @@ .center Empty :javascript - $(document).on('click', 'tr[data-link]', function(e) { + $('.tree-holder').on('click', 'tr[data-link] a', function(e) { + e.stopImmediatePropagation(); + }); + + $('.tree-holder').on('click', 'tr[data-link]', function(e) { window.location = this.dataset.link; }); diff --git a/app/views/projects/builds/index.html.haml b/app/views/projects/builds/index.html.haml index 5d18c0d803a..bbb6944a65a 100644 --- a/app/views/projects/builds/index.html.haml +++ b/app/views/projects/builds/index.html.haml @@ -6,7 +6,12 @@ - if can?(current_user, :manage_builds, @project) .pull-left.hidden-xs - if @all_builds.running_or_pending.any? - = link_to 'Cancel running', cancel_all_namespace_project_builds_path(@project.namespace, @project), data: { confirm: 'Are you sure?' }, class: 'btn btn-danger', method: :post + = link_to 'Cancel running', cancel_all_namespace_project_builds_path(@project.namespace, @project), + data: { confirm: 'Are you sure?' }, class: 'btn btn-danger', method: :post + + = link_to ci_lint_path, class: 'btn btn-default' do + = icon('wrench') + %span CI Lint %ul.nav-links %li{class: ('active' if @scope.nil?)} diff --git a/config/environments/development.rb b/config/environments/development.rb index 257c163720a..689694a3480 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -16,6 +16,9 @@ Rails.application.configure do # Print deprecation notices to the Rails logger config.active_support.deprecation = :log + # Raise an error on page load if there are pending migrations + config.active_record.migration_error = :page_load + # Only use best-standards-support built into browsers config.action_dispatch.best_standards_support = :builtin diff --git a/db/migrate/20160119111158_add_services_category.rb b/db/migrate/20160119111158_add_services_category.rb new file mode 100644 index 00000000000..a9110a8418b --- /dev/null +++ b/db/migrate/20160119111158_add_services_category.rb @@ -0,0 +1,39 @@ +class AddServicesCategory < ActiveRecord::Migration + def up + add_column :services, :category, :string, default: 'common', null: false + + category = quote_column_name('category') + type = quote_column_name('type') + + execute <<-EOF +UPDATE services +SET #{category} = 'issue_tracker' +WHERE #{type} IN ( + 'CustomIssueTrackerService', + 'GitlabIssueTrackerService', + 'IssueTrackerService', + 'JiraService', + 'RedmineService' +); +EOF + + execute <<-EOF +UPDATE services +SET #{category} = 'ci' +WHERE #{type} IN ( + 'BambooService', + 'BuildkiteService', + 'CiService', + 'DroneCiService', + 'GitlabCiService', + 'TeamcityService' +); + EOF + + add_index :services, :category + end + + def down + remove_column :services, :category + end +end diff --git a/db/migrate/20160119112418_add_services_default.rb b/db/migrate/20160119112418_add_services_default.rb new file mode 100644 index 00000000000..69a42d7b873 --- /dev/null +++ b/db/migrate/20160119112418_add_services_default.rb @@ -0,0 +1,20 @@ +class AddServicesDefault < ActiveRecord::Migration + def up + add_column :services, :default, :boolean, default: false + + default = quote_column_name('default') + type = quote_column_name('type') + + execute <<-EOF +UPDATE services +SET #{default} = true +WHERE #{type} = 'GitlabIssueTrackerService' +EOF + + add_index :services, :default + end + + def down + remove_column :services, :default + end +end diff --git a/db/schema.rb b/db/schema.rb index 9045135dd9a..a08181b910f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160113111034) do +ActiveRecord::Schema.define(version: 20160119112418) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -725,20 +725,24 @@ ActiveRecord::Schema.define(version: 20160113111034) do t.string "type" t.string "title" t.integer "project_id" - t.datetime "created_at" - t.datetime "updated_at" - t.boolean "active", default: false, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.boolean "active", null: false t.text "properties" - t.boolean "template", default: false - t.boolean "push_events", default: true - t.boolean "issues_events", default: true - t.boolean "merge_requests_events", default: true - t.boolean "tag_push_events", default: true - t.boolean "note_events", default: true, null: false - t.boolean "build_events", default: false, null: false - end - + t.boolean "template", default: false + t.boolean "push_events", default: true + t.boolean "issues_events", default: true + t.boolean "merge_requests_events", default: true + t.boolean "tag_push_events", default: true + t.boolean "note_events", default: true, null: false + t.boolean "build_events", default: false, null: false + t.string "category", default: "common", null: false + t.boolean "default", default: false + end + + add_index "services", ["category"], name: "index_services_on_category", using: :btree add_index "services", ["created_at", "id"], name: "index_services_on_created_at_and_id", using: :btree + add_index "services", ["default"], name: "index_services_on_default", using: :btree add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree add_index "services", ["template"], name: "index_services_on_template", using: :btree diff --git a/features/project/builds/artifacts.feature b/features/project/builds/artifacts.feature index 4f68e44fd77..1185854453a 100644 --- a/features/project/builds/artifacts.feature +++ b/features/project/builds/artifacts.feature @@ -7,21 +7,21 @@ Feature: Project Builds Artifacts Scenario: I download build artifacts Given recent build has artifacts available - When I visit recent build summary page + When I visit recent build details page And I click artifacts download button Then download of build artifacts archive starts Scenario: I browse build artifacts Given recent build has artifacts available And recent build has artifacts metadata available - When I visit recent build summary page + When I visit recent build details page And I click artifacts browse button Then I should see content of artifacts archive Scenario: I browse subdirectory of build artifacts Given recent build has artifacts available And recent build has artifacts metadata available - When I visit recent build summary page + When I visit recent build details page And I click artifacts browse button And I click link to subdirectory within build artifacts Then I should see content of subdirectory within artifacts archive @@ -30,7 +30,7 @@ Feature: Project Builds Artifacts Given recent build has artifacts available And recent build has artifacts metadata available And recent build artifacts contain directory with UTF-8 characters - When I visit recent build summary page + When I visit recent build details page And I click artifacts browse button And I navigate to directory with UTF-8 characters in name Then I should see content of directory with UTF-8 characters in name @@ -39,7 +39,7 @@ Feature: Project Builds Artifacts Given recent build has artifacts available And recent build has artifacts metadata available And recent build artifacts contain directory with invalid UTF-8 characters - When I visit recent build summary page + When I visit recent build details page And I click artifacts browse button And I navigate to parent directory of directory with invalid name Then I should not see directory with invalid name on the list @@ -47,7 +47,7 @@ Feature: Project Builds Artifacts Scenario: I download a single file from build artifacts Given recent build has artifacts available And recent build has artifacts metadata available - When I visit recent build summary page + When I visit recent build details page And I click artifacts browse button And I click a link to file within build artifacts Then download of a file extracted from build artifacts should start diff --git a/features/project/builds/summary.feature b/features/project/builds/summary.feature index e90ea592aab..b69d279517b 100644 --- a/features/project/builds/summary.feature +++ b/features/project/builds/summary.feature @@ -5,7 +5,11 @@ Feature: Project Builds Summary And project has CI enabled And project has a recent build - Scenario: I browse build summary page - When I visit recent build summary page - Then I see summary for build + Scenario: I browse build details page + When I visit recent build details page + Then I see details of a build And I see build trace + + Scenario: I browse project builds page + When I visit project builds page + Then I see button to CI Lint diff --git a/features/steps/project/builds/summary.rb b/features/steps/project/builds/summary.rb index 2439d48fbef..036bc0a499e 100644 --- a/features/steps/project/builds/summary.rb +++ b/features/steps/project/builds/summary.rb @@ -4,11 +4,18 @@ class Spinach::Features::ProjectBuildsSummary < Spinach::FeatureSteps include SharedBuilds include RepoHelpers - step 'I see summary for build' do + step 'I see details of a build' do expect(page).to have_content "Build ##{@build.id}" end step 'I see build trace' do expect(page).to have_css '#build-trace' end + + step 'I see button to CI Lint' do + page.within('.controls') do + ci_lint_tool_link = page.find_link('CI Lint') + expect(ci_lint_tool_link[:href]).to eq ci_lint_path + end + end end diff --git a/features/steps/shared/builds.rb b/features/steps/shared/builds.rb index f88b01af84e..92bf362879b 100644 --- a/features/steps/shared/builds.rb +++ b/features/steps/shared/builds.rb @@ -10,10 +10,14 @@ module SharedBuilds @build = create :ci_build, commit: ci_commit end - step 'I visit recent build summary page' do + step 'I visit recent build details page' do visit namespace_project_build_path(@project.namespace, @project, @build) end + step 'I visit project builds page' do + visit namespace_project_builds_path(@project.namespace, @project) + end + step 'recent build has artifacts available' do artifacts = Rails.root + 'spec/fixtures/ci_build_artifacts.zip' archive = fixture_file_upload(artifacts, 'application/zip') diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 7f938780ab1..ea054255820 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -39,7 +39,6 @@ module Gitlab end use_db && ActiveRecord::Base.connection.active? && - !ActiveRecord::Migrator.needs_migration? && ActiveRecord::Base.connection.table_exists?('application_settings') rescue ActiveRecord::NoDatabaseError diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 6d219f35895..2d0b5df4224 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -227,7 +227,7 @@ describe NotificationService, services: true do end describe :reassigned_issue do - it 'should email new assignee' do + it 'emails new assignee' do notification.reassigned_issue(issue, @u_disabled) should_email(issue.assignee) @@ -238,6 +238,62 @@ describe NotificationService, services: true do should_not_email(@u_participating) should_not_email(@u_disabled) end + + it 'emails previous assignee even if he has the "on mention" notif level' do + issue.update_attribute(:assignee, @u_mentioned) + issue.update_attributes(assignee: @u_watcher) + notification.reassigned_issue(issue, @u_disabled) + + should_email(@u_mentioned) + should_email(@u_watcher) + should_email(@u_participant_mentioned) + should_email(@subscriber) + should_not_email(@unsubscriber) + should_not_email(@u_participating) + should_not_email(@u_disabled) + end + + it 'emails new assignee even if he has the "on mention" notif level' do + issue.update_attributes(assignee: @u_mentioned) + notification.reassigned_issue(issue, @u_disabled) + + expect(issue.assignee).to be @u_mentioned + should_email(issue.assignee) + should_email(@u_watcher) + should_email(@u_participant_mentioned) + should_email(@subscriber) + should_not_email(@unsubscriber) + should_not_email(@u_participating) + should_not_email(@u_disabled) + end + + it 'emails new assignee' do + issue.update_attribute(:assignee, @u_mentioned) + notification.reassigned_issue(issue, @u_disabled) + + expect(issue.assignee).to be @u_mentioned + should_email(issue.assignee) + should_email(@u_watcher) + should_email(@u_participant_mentioned) + should_email(@subscriber) + should_not_email(@unsubscriber) + should_not_email(@u_participating) + should_not_email(@u_disabled) + end + + it 'does not email new assignee if they are the current user' do + issue.update_attribute(:assignee, @u_mentioned) + notification.reassigned_issue(issue, @u_mentioned) + + expect(issue.assignee).to be @u_mentioned + should_email(@u_watcher) + should_email(@u_participant_mentioned) + should_email(@subscriber) + should_not_email(issue.assignee) + should_not_email(@unsubscriber) + should_not_email(@u_participating) + should_not_email(@u_disabled) + end end describe :close_issue do diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 5d0b18558b1..e43903dbd3c 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -32,6 +32,7 @@ describe Projects::CreateService, services: true do it { expect(@project).to be_valid } it { expect(@project.owner).to eq(@user) } + it { expect(@project.team.masters).to include(@user) } it { expect(@project.namespace).to eq(@user.namespace) } end |