diff --git a/CHANGELOG.md b/CHANGELOG.md index c1e0382c791..7b018fc0d57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. +## 8.14.0 (2016-11-22) + ## 8.13.0 (2016-10-22) - Fix save button on project pipeline settings page. (!6955) @@ -13,10 +15,13 @@ Please view this file on the master branch, on stable branches it's out of date. - Update runner version only when updating contacted_at - Add link from system note to compare with previous version - Use gitlab-shell v3.6.6 + - Ignore references to internal issues when using external issues tracker - Ability to resolve merge request conflicts with editor !6374 - Add `/projects/visible` API endpoint (Ben Boeckel) - Fix centering of custom header logos (Ashley Dumaine) + - Keep around commits only pipeline creation as pipeline data doesn't change over time - ExpireBuildArtifactsWorker query builds table without ordering enqueuing one job per build to cleanup + - Add group level labels. (!6425) - Add an example for testing a phoenix application with Gitlab CI in the docs (Manthan Mallikarjun) - Cancelled pipelines could be retried. !6927 - Updating verbiage on git basics to be more intuitive @@ -105,7 +110,6 @@ Please view this file on the master branch, on stable branches it's out of date. - Optimize GitHub importing for speed and memory - API: expose pipeline data in builds API (!6502, Guilherme Salazar) - Notify the Merger about merge after successful build (Dimitris Karakasilis) - - Reorder issue and merge request titles to show IDs first. !6503 (Greg Laubenstein) - Reduce queries needed to find users using their SSH keys when pushing commits - Prevent rendering the link to all when the author has no access (Katarzyna Kobierska Ula Budziszewska) - Fix broken repository 500 errors in project list @@ -123,8 +127,11 @@ Please view this file on the master branch, on stable branches it's out of date. - Cleanup Ci::ApplicationController. !6757 (Takuya Noguchi) - Fixes padding in all clipboard icons that have .btn class - Fix a typo in doc/api/labels.md + - Fix double-escaping in activities tab (Alexandre Maia) - API: all unknown routing will be handled with 404 Not Found - Add docs for request profiling + - Delete dynamic environments + - Fix buggy iOS tooltip layering behavior. - Make guests unable to view MRs on private projects ## 8.12.7 diff --git a/Gemfile b/Gemfile index 05166b6a828..46245ab62d1 100644 --- a/Gemfile +++ b/Gemfile @@ -29,7 +29,7 @@ gem 'omniauth-github', '~> 1.1.1' gem 'omniauth-gitlab', '~> 1.0.0' gem 'omniauth-google-oauth2', '~> 0.4.1' gem 'omniauth-kerberos', '~> 0.3.0', group: :kerberos -gem 'omniauth-saml', '~> 1.6.0' +gem 'omniauth-saml', '~> 1.7.0' gem 'omniauth-shibboleth', '~> 1.2.0' gem 'omniauth-twitter', '~> 1.2.0' gem 'omniauth_crowd', '~> 2.2.0' diff --git a/Gemfile.lock b/Gemfile.lock index a9892d1c130..442184b9228 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -473,9 +473,9 @@ GEM omniauth-oauth2 (1.3.1) oauth2 (~> 1.0) omniauth (~> 1.2) - omniauth-saml (1.6.0) + omniauth-saml (1.7.0) omniauth (~> 1.3) - ruby-saml (~> 1.3) + ruby-saml (~> 1.4) omniauth-shibboleth (1.2.1) omniauth (>= 1.0.0) omniauth-twitter (1.2.1) @@ -635,7 +635,7 @@ GEM crack (~> 0.4) ruby-prof (0.16.2) ruby-progressbar (1.8.1) - ruby-saml (1.3.0) + ruby-saml (1.4.1) nokogiri (>= 1.5.10) ruby_parser (3.8.2) sexp_processor (~> 4.1) @@ -915,7 +915,7 @@ DEPENDENCIES omniauth-gitlab (~> 1.0.0) omniauth-google-oauth2 (~> 0.4.1) omniauth-kerberos (~> 0.3.0) - omniauth-saml (~> 1.6.0) + omniauth-saml (~> 1.7.0) omniauth-shibboleth (~> 1.2.0) omniauth-twitter (~> 1.2.0) omniauth_crowd (~> 2.2.0) diff --git a/VERSION b/VERSION index dff4cd02d5f..919f462addc 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -8.13.0-pre +8.14.0-pre diff --git a/app/assets/javascripts/dispatcher.js.es6 b/app/assets/javascripts/dispatcher.js.es6 index 73691f40c74..afc0d6f8c62 100644 --- a/app/assets/javascripts/dispatcher.js.es6 +++ b/app/assets/javascripts/dispatcher.js.es6 @@ -168,6 +168,8 @@ shortcut_handler = new ShortcutsNavigation(); new ShortcutsBlob(true); break; + case 'groups:labels:new': + case 'groups:labels:edit': case 'projects:labels:new': case 'projects:labels:edit': new Labels(); diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index 2eb7c4ea211..c532737626c 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -266,7 +266,7 @@ }, fieldName: $dropdown.data('field-name'), id: function(label) { - if (label.id <= 0) return; + if (label.id <= 0) return label.title; if ($dropdown.hasClass('js-issuable-form-dropdown')) { return label.id; diff --git a/app/assets/javascripts/merge_request_widget.js.es6 b/app/assets/javascripts/merge_request_widget.js.es6 index fcadc4bc515..3ff6851d59b 100644 --- a/app/assets/javascripts/merge_request_widget.js.es6 +++ b/app/assets/javascripts/merge_request_widget.js.es6 @@ -17,6 +17,12 @@ View on <%- external_url_formatted %> + + + + Stop environment + + `; @@ -205,6 +211,11 @@ if ($(`.mr-state-widget #${ environment.id }`).length) return; const $template = $(DEPLOYMENT_TEMPLATE); if (!environment.external_url || !environment.external_url_formatted) $('.js-environment-link', $template).remove(); + + if (!environment.stop_url) { + $('.js-stop-env-link', $template).remove(); + } + if (environment.deployed_at && environment.deployed_at_formatted) { environment.deployed_at = $.timeago(environment.deployed_at) + '.'; } else { diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 13c1bbf0359..f49d7b92a00 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -167,7 +167,6 @@ */ &.code { padding: 0; - -webkit-overflow-scrolling: auto; // See https://gitlab.com/gitlab-org/gitlab-ce/issues/13987 } } } diff --git a/app/assets/stylesheets/framework/layout.scss b/app/assets/stylesheets/framework/layout.scss index 8bb047db2dd..7baa4296abf 100644 --- a/app/assets/stylesheets/framework/layout.scss +++ b/app/assets/stylesheets/framework/layout.scss @@ -27,3 +27,15 @@ body { .container-limited { max-width: $fixed-layout-width; } + + +/* The following prevents side effects related to iOS Safari's implementation of -webkit-overflow-scrolling: touch, +which is applied to the body by jquery.nicescroling plugin to force hardware acceleration for momentum scrolling. Side +effects are commonly related to inconsisent z-index behavior (e.g. tooltips). By applying the following to direct children +of the body element here, we negate cascading side effects but allow momentum scrolling to be applied to the body */ + +.navbar, +.page-gutter, +.page-with-sidebar { + -webkit-overflow-scrolling: auto; +} diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index bdc82a8f0f5..fe6421f8b3f 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -52,7 +52,6 @@ background: #fff; color: #333; border-radius: 0 0 3px 3px; - -webkit-overflow-scrolling: auto; .unfold { cursor: pointer; diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index 820cc0fc991..12ee0a5dc3d 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -38,6 +38,14 @@ color: $gl-dark-link-color; } + .stop-env-link { + color: $table-text-gray; + + .stop-env-icon { + font-size: 14px; + } + } + .deployment { .build-column { diff --git a/app/assets/stylesheets/pages/labels.scss b/app/assets/stylesheets/pages/labels.scss index 9bac6d46355..397f89f501a 100644 --- a/app/assets/stylesheets/pages/labels.scss +++ b/app/assets/stylesheets/pages/labels.scss @@ -66,7 +66,21 @@ text-overflow: ellipsis; vertical-align: middle; max-width: 100%; - } + } + } + + .label-type { + display: block; + margin-bottom: 10px; + margin-left: 50px; + + @media (min-width: $screen-sm-min) { + display: inline-block; + width: 100px; + margin-left: 10px; + margin-bottom: 0; + vertical-align: middle; + } } .label-description { @@ -209,6 +223,13 @@ } .label-subscribe-button { + .label-subscribe-button-icon { + &[disabled] { + opacity: 0.5; + pointer-events: none; + } + } + .label-subscribe-button-loading { display: none; } diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 101472278e2..35a1877df95 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -183,6 +183,15 @@ .ci-coverage { float: right; } + + .stop-env-container { + color: $gl-text-color; + float: right; + + a { + color: $gl-text-color; + } + } } .mr_source_commit, diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index bb32bc502e6..be86fa106f8 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -2,6 +2,7 @@ module IssuableActions extend ActiveSupport::Concern included do + before_action :labels, only: [:show, :new, :edit] before_action :authorize_destroy_issuable!, only: :destroy before_action :authorize_admin_issuable!, only: :bulk_update end @@ -25,6 +26,10 @@ module IssuableActions private + def labels + @labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute + end + def authorize_destroy_issuable! unless can?(current_user, :"destroy_#{issuable.to_ability_name}", issuable) return access_denied! diff --git a/app/controllers/dashboard/labels_controller.rb b/app/controllers/dashboard/labels_controller.rb index 2a88350a4ca..d5031da867a 100644 --- a/app/controllers/dashboard/labels_controller.rb +++ b/app/controllers/dashboard/labels_controller.rb @@ -1,9 +1,9 @@ class Dashboard::LabelsController < Dashboard::ApplicationController def index - labels = Label.where(project_id: projects).select(:id, :title, :color).uniq(:title) + labels = LabelsFinder.new(current_user).execute respond_to do |format| - format.json { render json: labels } + format.json { render json: labels.as_json(only: [:id, :title, :color]) } end end end diff --git a/app/controllers/groups/labels_controller.rb b/app/controllers/groups/labels_controller.rb new file mode 100644 index 00000000000..29528b2cfaa --- /dev/null +++ b/app/controllers/groups/labels_controller.rb @@ -0,0 +1,92 @@ +class Groups::LabelsController < Groups::ApplicationController + before_action :label, only: [:edit, :update, :destroy] + before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update, :destroy] + before_action :save_previous_label_path, only: [:edit] + + respond_to :html + + def index + respond_to do |format| + format.html do + @labels = @group.labels.page(params[:page]) + end + + format.json do + available_labels = LabelsFinder.new(current_user, group_id: @group.id).execute + render json: available_labels.as_json(only: [:id, :title, :color]) + end + end + end + + def new + @label = @group.labels.new + @previous_labels_path = previous_labels_path + end + + def create + @label = @group.labels.create(label_params) + + if @label.valid? + redirect_to group_labels_path(@group) + else + render :new + end + end + + def edit + @previous_labels_path = previous_labels_path + end + + def update + if @label.update_attributes(label_params) + redirect_back_or_group_labels_path + else + render :edit + end + end + + def destroy + @label.destroy + + respond_to do |format| + format.html do + redirect_to group_labels_path(@group), notice: 'Label was removed' + end + format.js + end + end + + protected + + def authorize_admin_labels! + return render_404 unless can?(current_user, :admin_label, @group) + end + + def authorize_read_labels! + return render_404 unless can?(current_user, :read_label, @group) + end + + def label + @label ||= @group.labels.find(params[:id]) + end + + def label_params + params.require(:label).permit(:title, :description, :color) + end + + def redirect_back_or_group_labels_path(options = {}) + redirect_to previous_labels_path, options + end + + def previous_labels_path + session.fetch(:previous_labels_path, fallback_path) + end + + def fallback_path + group_labels_path(@group) + end + + def save_previous_label_path + session[:previous_labels_path] = URI(request.referer || '').path + end +end diff --git a/app/controllers/projects/boards/issues_controller.rb b/app/controllers/projects/boards/issues_controller.rb index 6f73a5907a9..dc33e1405f2 100644 --- a/app/controllers/projects/boards/issues_controller.rb +++ b/app/controllers/projects/boards/issues_controller.rb @@ -72,10 +72,10 @@ module Projects def serialize_as_json(resource) resource.as_json( + labels: true, only: [:iid, :title, :confidential, :due_date], include: { assignee: { only: [:id, :name, :username], methods: [:avatar_url] }, - labels: { only: [:id, :title, :description, :color, :priority], methods: [:text_color] }, milestone: { only: [:id, :title] } }, user: current_user diff --git a/app/controllers/projects/boards/lists_controller.rb b/app/controllers/projects/boards/lists_controller.rb index 76ae41319c4..67e3c9add81 100644 --- a/app/controllers/projects/boards/lists_controller.rb +++ b/app/controllers/projects/boards/lists_controller.rb @@ -76,9 +76,8 @@ module Projects resource.as_json( only: [:id, :list_type, :position], methods: [:title], - include: { - label: { only: [:id, :title, :description, :color, :priority] } - }) + label: true + ) end end end diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 58678f96879..ea22b2dcc15 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -2,11 +2,19 @@ class Projects::EnvironmentsController < Projects::ApplicationController layout 'project' before_action :authorize_read_environment! before_action :authorize_create_environment!, only: [:new, :create] - before_action :authorize_update_environment!, only: [:edit, :update, :destroy] - before_action :environment, only: [:show, :edit, :update, :destroy] + before_action :authorize_create_deployment!, only: [:stop] + before_action :authorize_update_environment!, only: [:edit, :update] + before_action :environment, only: [:show, :edit, :update, :stop] def index - @environments = project.environments + @scope = params[:scope] + @all_environments = project.environments + @environments = + if @scope == 'stopped' + @all_environments.stopped + else + @all_environments.available + end end def show @@ -38,14 +46,11 @@ class Projects::EnvironmentsController < Projects::ApplicationController end end - def destroy - if @environment.destroy - flash[:notice] = 'Environment was successfully removed.' - else - flash[:alert] = 'Failed to remove environment.' - end + def stop + return render_404 unless @environment.stoppable? - redirect_to namespace_project_environments_path(project.namespace, project) + new_action = @environment.stop!(current_user) + redirect_to polymorphic_path([project.namespace.becomes(Namespace), project, new_action]) end private diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 96041b07647..cb649264146 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -26,7 +26,9 @@ class Projects::IssuesController < Projects::ApplicationController @issues = issues_collection @issues = @issues.page(params[:page]) - @labels = @project.labels.where(title: params[:label_name]) + if params[:label_name].present? + @labels = LabelsFinder.new(current_user, project_id: @project.id, title: params[:label_name]).execute + end respond_to do |format| format.html diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index a6626df4826..4f855134368 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/app/controllers/projects/labels_controller.rb @@ -3,21 +3,22 @@ class Projects::LabelsController < Projects::ApplicationController before_action :module_enabled before_action :label, only: [:edit, :update, :destroy] + before_action :find_labels, only: [:index, :set_priorities, :remove_priority] before_action :authorize_read_label! - before_action :authorize_admin_labels!, only: [ - :new, :create, :edit, :update, :generate, :destroy, :remove_priority, :set_priorities - ] + before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update, + :generate, :destroy, :remove_priority, + :set_priorities] respond_to :js, :html def index - @labels = @project.labels.unprioritized.page(params[:page]) - @prioritized_labels = @project.labels.prioritized + @prioritized_labels = @available_labels.prioritized(@project) + @labels = @available_labels.unprioritized(@project).page(params[:page]) respond_to do |format| format.html format.json do - render json: @project.labels + render json: @available_labels.as_json(only: [:id, :title, :color]) end end end @@ -36,7 +37,7 @@ class Projects::LabelsController < Projects::ApplicationController end else respond_to do |format| - format.html { render 'new' } + format.html { render :new } format.json { render json: { message: @label.errors.messages }, status: 400 } end end @@ -49,7 +50,7 @@ class Projects::LabelsController < Projects::ApplicationController if @label.update_attributes(label_params) redirect_to namespace_project_labels_path(@project.namespace, @project) else - render 'edit' + render :edit end end @@ -68,6 +69,7 @@ class Projects::LabelsController < Projects::ApplicationController def destroy @label.destroy + @labels = find_labels respond_to do |format| format.html do @@ -80,20 +82,24 @@ class Projects::LabelsController < Projects::ApplicationController def remove_priority respond_to do |format| - if label.update_attribute(:priority, nil) + label = @available_labels.find(params[:id]) + + if label.unprioritize!(project) format.json { render json: label } else - message = label.errors.full_messages.uniq.join('. ') - format.json { render json: { message: message }, status: :unprocessable_entity } + format.json { head :unprocessable_entity } end end end def set_priorities Label.transaction do - params[:label_ids].each_with_index do |label_id, index| - label = @project.labels.find_by_id(label_id) - label.update_attribute(:priority, index) if label + available_labels_ids = @available_labels.where(id: params[:label_ids]).pluck(:id) + label_ids = params[:label_ids].select { |id| available_labels_ids.include?(id.to_i) } + + label_ids.each_with_index do |label_id, index| + label = @available_labels.find(label_id) + label.prioritize!(project, index) end end @@ -119,6 +125,10 @@ class Projects::LabelsController < Projects::ApplicationController end alias_method :subscribable_resource, :label + def find_labels + @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute.includes(:priorities) + end + def authorize_admin_labels! return render_404 unless can?(current_user, :admin_label, @project) end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index a39b47b6d95..0f593d1a936 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -40,7 +40,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_requests = @merge_requests.page(params[:page]) @merge_requests = @merge_requests.preload(:target_project) - @labels = @project.labels.where(title: params[:label_name]) + if params[:label_name].present? + labels_params = { project_id: @project.id, title: params[:label_name] } + @labels = LabelsFinder.new(current_user, labels_params).execute + end respond_to do |format| format.html @@ -422,10 +425,16 @@ class Projects::MergeRequestsController < Projects::ApplicationController project = environment.project deployment = environment.first_deployment_for(@merge_request.diff_head_commit) + stop_url = + if environment.stoppable? && can?(current_user, :create_deployment, environment) + stop_namespace_project_environment_path(project.namespace, project, environment) + end + { id: environment.id, name: environment.name, url: namespace_project_environment_path(project.namespace, project, environment), + stop_url: stop_url, external_url: environment.external_url, external_url_formatted: environment.formatted_external_url, deployed_at: deployment.try(:created_at), @@ -483,13 +492,12 @@ class Projects::MergeRequestsController < Projects::ApplicationController @noteable = @merge_request @commits_count = @merge_request.commits.count - @pipeline = @merge_request.pipeline - @statuses = @pipeline.statuses.relevant if @pipeline - if @merge_request.locked_long_ago? @merge_request.unlock_mr @merge_request.close end + + define_pipelines_vars end # Discussion tab data is rendered on html responses of actions @@ -517,7 +525,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController def define_widget_vars @pipeline = @merge_request.pipeline - @pipelines = [@pipeline].compact end def define_commit_vars @@ -544,6 +551,15 @@ class Projects::MergeRequestsController < Projects::ApplicationController ) end + def define_pipelines_vars + @pipelines = @merge_request.all_pipelines + + if @pipelines.any? + @pipeline = @pipelines.first + @statuses = @pipeline.statuses.relevant + end + end + def define_new_vars @noteable = @merge_request @@ -559,10 +575,12 @@ class Projects::MergeRequestsController < Projects::ApplicationController @commit = @merge_request.diff_head_commit @base_commit = @merge_request.diff_base_commit - @pipeline = @merge_request.pipeline - @statuses = @pipeline.statuses.relevant if @pipeline @note_counts = Note.where(commit_id: @commits.map(&:id)). group(:commit_id).count + + @labels = LabelsFinder.new(current_user, project_id: @project.id).execute + + define_pipelines_vars end def invalid_mr diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 9f170428100..e27986ef95b 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -124,15 +124,12 @@ class IssuableFinder def labels return @labels if defined?(@labels) - if labels? && !filter_by_no_label? - @labels = Label.where(title: label_names) - - if projects - @labels = @labels.where(project: projects) + @labels = + if labels? && !filter_by_no_label? + LabelsFinder.new(current_user, project_ids: projects, title: label_names).execute + else + Label.none end - else - @labels = Label.none - end end def assignee? @@ -274,8 +271,10 @@ class IssuableFinder items = items.without_label else items = items.with_label(label_names, params[:sort]) + if projects - items = items.where(labels: { project_id: projects }) + label_ids = LabelsFinder.new(current_user, project_ids: projects).execute.select(:id) + items = items.where(labels: { id: label_ids }) end end end diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb new file mode 100644 index 00000000000..6ace14a4bb5 --- /dev/null +++ b/app/finders/labels_finder.rb @@ -0,0 +1,92 @@ +class LabelsFinder < UnionFinder + def initialize(current_user, params = {}) + @current_user = current_user + @params = params + end + + def execute(authorized_only: true) + @authorized_only = authorized_only + + items = find_union(label_ids, Label) + items = with_title(items) + sort(items) + end + + private + + attr_reader :current_user, :params, :authorized_only + + def label_ids + label_ids = [] + + if project + label_ids << project.group.labels if project.group.present? + label_ids << project.labels + else + label_ids << Label.where(group_id: projects.group_ids) + label_ids << Label.where(project_id: projects.select(:id)) + end + + label_ids + end + + def sort(items) + items.reorder(title: :asc) + end + + def with_title(items) + items = items.where(title: title) if title + items + end + + def group_id + params[:group_id].presence + end + + def project_id + params[:project_id].presence + end + + def projects_ids + params[:project_ids].presence + end + + def title + params[:title].presence || params[:name].presence + end + + def project + return @project if defined?(@project) + + if project_id + @project = find_project + else + @project = nil + end + + @project + end + + def find_project + if authorized_only + available_projects.find_by(id: project_id) + else + Project.find_by(id: project_id) + end + end + + def projects + return @projects if defined?(@projects) + + @projects = authorized_only ? available_projects : Project.all + @projects = @projects.in_namespace(group_id) if group_id + @projects = @projects.where(id: projects_ids) if projects_ids + @projects = @projects.reorder(nil) + + @projects + end + + def available_projects + @available_projects ||= ProjectsFinder.new.execute(current_user) + end +end diff --git a/app/helpers/award_emoji_helper.rb b/app/helpers/award_emoji_helper.rb index 493f14f6f9d..592ffe7b89f 100644 --- a/app/helpers/award_emoji_helper.rb +++ b/app/helpers/award_emoji_helper.rb @@ -4,7 +4,7 @@ module AwardEmojiHelper if awardable.is_a?(Note) # We render a list of notes very frequently and calling the specific method is a lot faster than the generic one (6.5x) - toggle_award_emoji_namespace_project_note_url(namespace_id: @project.namespace_id, project_id: @project.id, id: awardable.id) + toggle_award_emoji_namespace_project_note_url(namespace_id: @project.namespace, project_id: @project, id: awardable.id) else url_for([:toggle_award_emoji, @project.namespace.becomes(Namespace), @project, awardable]) end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 692fadd505f..03b2db1bc91 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -124,6 +124,10 @@ module IssuablesHelper end end + def issuable_filters_present + params[:search] || params[:author_id] || params[:assignee_id] || params[:milestone_title] || params[:label_name] + end + def issuables_count_for_state(issuable_type, state) issuables_finder = public_send("#{issuable_type}_finder") issuables_finder.params[:state] = state diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index b9f3d6c75c2..221a84b042f 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -4,9 +4,8 @@ module LabelsHelper # Link to a Label # # label - Label object to link to - # project - Project object which will be used as the context for the label's - # link. If omitted, defaults to `@project`, or the label's own - # project. + # subject - Project/Group object which will be used as the context for the + # label's link. If omitted, defaults to the label's own group/project. # type - The type of item the link will point to (:issue or # :merge_request). If omitted, defaults to :issue. # block - An optional block that will be passed to `link_to`, forming the @@ -15,15 +14,14 @@ module LabelsHelper # # Examples: # - # # Allow the generated link to use the label's own project + # # Allow the generated link to use the label's own subject # link_to_label(label) # - # # Force the generated link to use @project - # @project = Project.first - # link_to_label(label) + # # Force the generated link to use a provided group + # link_to_label(label, subject: Group.last) # # # Force the generated link to use a provided project - # link_to_label(label, project: Project.last) + # link_to_label(label, subject: Project.last) # # # Force the generated link to point to merge requests instead of issues # link_to_label(label, type: :merge_request) @@ -32,9 +30,8 @@ module LabelsHelper # link_to_label(label) { "My Custom Label Text" } # # Returns a String - def link_to_label(label, project: nil, type: :issue, tooltip: true, css_class: nil, &block) - project ||= @project || label.project - link = label_filter_path(project, label, type: type) + def link_to_label(label, subject: nil, type: :issue, tooltip: true, css_class: nil, &block) + link = label_filter_path(subject || label.subject, label, type: type) if block_given? link_to link, class: css_class, &block @@ -43,15 +40,40 @@ module LabelsHelper end end - def label_filter_path(project, label, type: issue) - send("namespace_project_#{type.to_s.pluralize}_path", - project.namespace, - project, - label_name: [label.name]) + def label_filter_path(subject, label, type: :issue) + case subject + when Group + send("#{type.to_s.pluralize}_group_path", + subject, + label_name: [label.name]) + when Project + send("namespace_project_#{type.to_s.pluralize}_path", + subject.namespace, + subject, + label_name: [label.name]) + end end - def project_label_names - @project.labels.pluck(:title) + def edit_label_path(label) + case label + when GroupLabel then edit_group_label_path(label.group, label) + when ProjectLabel then edit_namespace_project_label_path(label.project.namespace, label.project, label) + end + end + + def destroy_label_path(label) + case label + when GroupLabel then group_label_path(label.group, label) + when ProjectLabel then namespace_project_label_path(label.project.namespace, label.project, label) + end + end + + def toggle_subscription_data(label) + return unless label.is_a?(ProjectLabel) + + { + url: toggle_subscription_namespace_project_label_path(label.project.namespace, label.project, label) + } end def render_colored_label(label, label_suffix = '', tooltip: true) @@ -68,8 +90,8 @@ module LabelsHelper span.html_safe end - def render_colored_cross_project_label(label, tooltip: true) - label_suffix = label.project.name_with_namespace + def render_colored_cross_project_label(label, source_project = nil, tooltip: true) + label_suffix = source_project ? source_project.name_with_namespace : label.project.name_with_namespace label_suffix = " in #{escape_once(label_suffix)}" render_colored_label(label, label_suffix, tooltip: tooltip) end @@ -115,7 +137,10 @@ module LabelsHelper end def labels_filter_path + return group_labels_path(@group, :json) if @group + project = @target_project || @project + if project namespace_project_labels_path(project.namespace, project, :json) else @@ -124,11 +149,24 @@ module LabelsHelper end def label_subscription_status(label) - label.subscribed?(current_user) ? 'subscribed' : 'unsubscribed' + case label + when GroupLabel then 'Subscribing to group labels is currently not supported.' + when ProjectLabel then label.subscribed?(current_user) ? 'subscribed' : 'unsubscribed' + end end def label_subscription_toggle_button_text(label) - label.subscribed?(current_user) ? 'Unsubscribe' : 'Subscribe' + case label + when GroupLabel then 'Subscribing to group labels is currently not supported.' + when ProjectLabel then label.subscribed?(current_user) ? 'Unsubscribe' : 'Subscribe' + end + end + + def label_deletion_confirm_text(label) + case label + when GroupLabel then 'Remove this label? This will affect all projects within the group. Are you sure?' + when ProjectLabel then 'Remove this label? Are you sure?' + end end # Required for Banzai::Filter::LabelReferenceFilter diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index e75fe6c222b..e84c91b417d 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -19,7 +19,7 @@ module Ci validates_presence_of :status, unless: :importing? validate :valid_commit_sha, unless: :importing? - after_save :keep_around_commits, unless: :importing? + after_create :keep_around_commits, unless: :importing? delegate :stages, to: :statuses diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index c4b42ad82c7..17c3b526c97 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -145,8 +145,14 @@ module Issuable end def order_labels_priority(excluded_labels: []) - condition_field = "#{table_name}.id" - highest_priority = highest_label_priority(name, condition_field, excluded_labels: excluded_labels).to_sql + params = { + target_type: name, + target_column: "#{table_name}.id", + project_column: "#{table_name}.#{project_foreign_key}", + excluded_labels: excluded_labels + } + + highest_priority = highest_label_priority(params).to_sql select("#{table_name}.*, (#{highest_priority}) AS highest_priority"). group(arel_table[:id]). @@ -230,18 +236,6 @@ module Issuable labels.order('title ASC').pluck(:title) end - def remove_labels - labels.delete_all - end - - def add_labels_by_names(label_names) - label_names.each do |label_name| - label = project.labels.create_with(color: Label::DEFAULT_COLOR). - find_or_create_by(title: label_name.strip) - self.labels << label - end - end - # Convert this Issuable class name to a format usable by Ability definitions # # Examples: diff --git a/app/models/concerns/sortable.rb b/app/models/concerns/sortable.rb index 1ebecd86af9..12b23f00769 100644 --- a/app/models/concerns/sortable.rb +++ b/app/models/concerns/sortable.rb @@ -38,11 +38,13 @@ module Sortable private - def highest_label_priority(object_types, condition_field, excluded_labels: []) - query = Label.select(Label.arel_table[:priority].minimum). + def highest_label_priority(target_type:, target_column:, project_column:, excluded_labels: []) + query = Label.select(LabelPriority.arel_table[:priority].minimum). + left_join_priorities. joins(:label_links). - where(label_links: { target_type: object_types }). - where("label_links.target_id = #{condition_field}"). + where("label_priorities.project_id = #{project_column}"). + where(label_links: { target_type: target_type }). + where("label_links.target_id = #{target_column}"). reorder(nil) query.where.not(title: excluded_labels) if excluded_labels.present? diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 3d9902d496e..1f8c5fb3d85 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -34,7 +34,7 @@ class Deployment < ActiveRecord::Base end def manual_actions - deployable.try(:other_actions) + @manual_actions ||= deployable.try(:other_actions) end def includes_commit?(commit) @@ -84,6 +84,17 @@ class Deployment < ActiveRecord::Base take end + def stop_action + return nil unless on_stop.present? + return nil unless manual_actions + + @stop_action ||= manual_actions.find_by(name: on_stop) + end + + def stoppable? + stop_action.present? + end + def formatted_deployment_time created_at.to_time.in_time_zone.to_s(:medium) end diff --git a/app/models/environment.rb b/app/models/environment.rb index d970bc0a005..d575f1dc73a 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -19,6 +19,24 @@ class Environment < ActiveRecord::Base allow_nil: true, addressable_url: true + delegate :stop_action, to: :last_deployment, allow_nil: true + + scope :available, -> { with_state(:available) } + scope :stopped, -> { with_state(:stopped) } + + state_machine :state, initial: :available do + event :start do + transition stopped: :available + end + + event :stop do + transition available: :stopped + end + + state :available + state :stopped + end + def last_deployment deployments.last end @@ -66,4 +84,14 @@ class Environment < ActiveRecord::Base external_url.gsub(/\A.*?:\/\//, '') end + + def stoppable? + available? && stop_action.present? + end + + def stop!(current_user) + return unless stoppable? + + stop_action.play(current_user) + end end diff --git a/app/models/external_issue.rb b/app/models/external_issue.rb index b7894c99846..fd9a8c1b8b7 100644 --- a/app/models/external_issue.rb +++ b/app/models/external_issue.rb @@ -29,11 +29,6 @@ class ExternalIssue @project end - # Pattern used to extract `JIRA-123` issue references from text - def self.reference_pattern - @reference_pattern ||= %r{(?\b([A-Z][A-Z0-9_]+-)\d+)} - end - def to_reference(_from_project = nil) id end diff --git a/app/models/group.rb b/app/models/group.rb index a2f88cca828..00a595d2705 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -19,6 +19,7 @@ class Group < Namespace has_many :project_group_links, dependent: :destroy has_many :shared_projects, through: :project_group_links, source: :project has_many :notification_settings, dependent: :destroy, as: :source + has_many :labels, class_name: 'GroupLabel' validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? } validate :visibility_level_allowed_by_projects diff --git a/app/models/group_label.rb b/app/models/group_label.rb new file mode 100644 index 00000000000..a698b532d19 --- /dev/null +++ b/app/models/group_label.rb @@ -0,0 +1,11 @@ +class GroupLabel < Label + belongs_to :group + + validates :group, presence: true + + alias_attribute :subject, :group + + def to_reference(source_project = nil, target_project = nil, format: :id) + super(source_project, target_project, format: format) + end +end diff --git a/app/models/issue.rb b/app/models/issue.rb index 89794290520..ef92ac27b46 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -138,6 +138,10 @@ class Issue < ActiveRecord::Base reference.to_i > 0 && reference.to_i <= Gitlab::Database::MAX_INT_VALUE end + def self.project_foreign_key + 'project_id' + end + def self.sort(method, excluded_labels: []) case method.to_s when 'due_date_asc' then order_due_date_asc @@ -278,6 +282,14 @@ class Issue < ActiveRecord::Base def as_json(options = {}) super(options).tap do |json| json[:subscribed] = subscribed?(options[:user]) if options.has_key?(:user) + + if options.has_key?(:labels) + json[:labels] = labels.as_json( + project: project, + only: [:id, :title, :description, :color, :priority], + methods: [:text_color] + ) + end end end end diff --git a/app/models/label.rb b/app/models/label.rb index e8e12e2904e..149fd98ecb3 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -15,34 +15,49 @@ class Label < ActiveRecord::Base default_value_for :color, DEFAULT_COLOR - belongs_to :project - has_many :lists, dependent: :destroy + has_many :priorities, class_name: 'LabelPriority' has_many :label_links, dependent: :destroy has_many :issues, through: :label_links, source: :target, source_type: 'Issue' has_many :merge_requests, through: :label_links, source: :target, source_type: 'MergeRequest' validates :color, color: true, allow_blank: false - validates :project, presence: true, unless: Proc.new { |service| service.template? } # Don't allow ',' for label titles - validates :title, - presence: true, - format: { with: /\A[^,]+\z/ }, - uniqueness: { scope: :project_id } - - before_save :nullify_priority + validates :title, presence: true, format: { with: /\A[^,]+\z/ } + validates :title, uniqueness: { scope: [:group_id, :project_id] } default_scope { order(title: :asc) } - scope :templates, -> { where(template: true) } + scope :templates, -> { where(template: true) } + scope :with_title, ->(title) { where(title: title) } - def self.prioritized - where.not(priority: nil).reorder(:priority, :title) + def self.prioritized(project) + joins(:priorities) + .where(label_priorities: { project_id: project }) + .reorder('label_priorities.priority ASC, labels.title ASC') end - def self.unprioritized - where(priority: nil) + def self.unprioritized(project) + labels = Label.arel_table + priorities = LabelPriority.arel_table + + label_priorities = labels.join(priorities, Arel::Nodes::OuterJoin). + on(labels[:id].eq(priorities[:label_id]).and(priorities[:project_id].eq(project.id))). + join_sources + + joins(label_priorities).where(priorities[:priority].eq(nil)) + end + + def self.left_join_priorities + labels = Label.arel_table + priorities = LabelPriority.arel_table + + label_priorities = labels.join(priorities, Arel::Nodes::OuterJoin). + on(labels[:id].eq(priorities[:label_id])). + join_sources + + joins(label_priorities) end alias_attribute :name, :title @@ -77,40 +92,30 @@ class Label < ActiveRecord::Base nil end - ## - # Returns the String necessary to reference this Label in Markdown - # - # format - Symbol format to use (default: :id, optional: :name) - # - # Examples: - # - # Label.first.to_reference # => "~1" - # Label.first.to_reference(format: :name) # => "~\"bug\"" - # Label.first.to_reference(project) # => "gitlab-org/gitlab-ce~1" - # - # Returns a String - # - def to_reference(from_project = nil, format: :id) - format_reference = label_format_reference(format) - reference = "#{self.class.reference_prefix}#{format_reference}" - - if cross_project_reference?(from_project) - project.to_reference + reference - else - reference - end + def open_issues_count(user = nil, project = nil) + issues_count(user, project_id: project.try(:id) || project_id, state: 'opened') end - def open_issues_count(user = nil) - issues.visible_to_user(user).opened.count + def closed_issues_count(user = nil, project = nil) + issues_count(user, project_id: project.try(:id) || project_id, state: 'closed') end - def closed_issues_count(user = nil) - issues.visible_to_user(user).closed.count + def open_merge_requests_count(user = nil, project = nil) + merge_requests_count(user, project_id: project.try(:id) || project_id, state: 'opened') end - def open_merge_requests_count - merge_requests.opened.count + def prioritize!(project, value) + label_priority = priorities.find_or_initialize_by(project_id: project.id) + label_priority.priority = value + label_priority.save! + end + + def unprioritize!(project) + priorities.where(project: project).delete_all + end + + def priority(project) + priorities.find_by(project: project).try(:priority) end def template? @@ -118,15 +123,61 @@ class Label < ActiveRecord::Base end def text_color - LabelsHelper::text_color_for_bg(self.color) + LabelsHelper.text_color_for_bg(self.color) end def title=(value) write_attribute(:title, sanitize_title(value)) if value.present? end + ## + # Returns the String necessary to reference this Label in Markdown + # + # format - Symbol format to use (default: :id, optional: :name) + # + # Examples: + # + # Label.first.to_reference # => "~1" + # Label.first.to_reference(format: :name) # => "~\"bug\"" + # Label.first.to_reference(project1, project2) # => "gitlab-org/gitlab-ce~1" + # + # Returns a String + # + def to_reference(source_project = nil, target_project = nil, format: :id) + format_reference = label_format_reference(format) + reference = "#{self.class.reference_prefix}#{format_reference}" + + if cross_project_reference?(source_project, target_project) + source_project.to_reference + reference + else + reference + end + end + + def as_json(options = {}) + super(options).tap do |json| + json[:priority] = priority(options[:project]) if options.has_key?(:project) + end + end + private + def cross_project_reference?(source_project, target_project) + source_project && target_project && source_project != target_project + end + + def issues_count(user, params = {}) + IssuesFinder.new(user, params.reverse_merge(label_name: title, scope: 'all')) + .execute + .count + end + + def merge_requests_count(user, params = {}) + MergeRequestsFinder.new(user, params.reverse_merge(label_name: title, scope: 'all')) + .execute + .count + end + def label_format_reference(format = :id) raise StandardError, 'Unknown format' unless [:id, :name].include?(format) @@ -137,10 +188,6 @@ class Label < ActiveRecord::Base end end - def nullify_priority - self.priority = nil if priority.blank? - end - def sanitize_title(value) CGI.unescapeHTML(Sanitize.clean(value.to_s)) end diff --git a/app/models/label_priority.rb b/app/models/label_priority.rb new file mode 100644 index 00000000000..5b85e0b6533 --- /dev/null +++ b/app/models/label_priority.rb @@ -0,0 +1,8 @@ +class LabelPriority < ActiveRecord::Base + belongs_to :project + belongs_to :label + + validates :project, :label, :priority, presence: true + validates :label_id, uniqueness: { scope: :project_id } + validates :priority, numericality: { only_integer: true, greater_than_or_equal_to: 0 } +end diff --git a/app/models/list.rb b/app/models/list.rb index eb87decdbc8..065d75bd1dc 100644 --- a/app/models/list.rb +++ b/app/models/list.rb @@ -26,6 +26,17 @@ class List < ActiveRecord::Base label? ? label.name : list_type.humanize end + def as_json(options = {}) + super(options).tap do |json| + if options.has_key?(:label) + json[:label] = label.as_json( + project: board.project, + only: [:id, :title, :description, :color] + ) + end + end + end + private def can_be_destroyed diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 8c6905a442d..0cc0b3c2a0e 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -137,6 +137,10 @@ class MergeRequest < ActiveRecord::Base reference.to_i > 0 && reference.to_i <= Gitlab::Database::MAX_INT_VALUE end + def self.project_foreign_key + 'target_project_id' + end + # Returns all the merge requests from an ActiveRecord:Relation. # # This method uses a UNION as it usually operates on the result of @@ -787,21 +791,21 @@ class MergeRequest < ActiveRecord::Base def all_pipelines return unless source_project - @all_pipelines ||= begin - sha = if persisted? - all_commits_sha - else - diff_head_sha - end - - source_project.pipelines.order(id: :desc). - where(sha: sha, ref: source_branch) - end + @all_pipelines ||= source_project.pipelines + .where(sha: all_commits_sha, ref: source_branch) + .order(id: :desc) end # Note that this could also return SHA from now dangling commits + # def all_commits_sha - merge_request_diffs.flat_map(&:commits_sha).uniq + if persisted? + merge_request_diffs.flat_map(&:commits_sha).uniq + elsif compare_commits + compare_commits.to_a.reverse.map(&:id) + else + [diff_head_sha] + end end def merge_commit diff --git a/app/models/project.rb b/app/models/project.rb index aee74c3dba1..653c38322c5 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -107,7 +107,7 @@ class Project < ActiveRecord::Base # Merge requests from source project should be kept when source project was removed has_many :fork_merge_requests, foreign_key: 'source_project_id', class_name: MergeRequest has_many :issues, dependent: :destroy - has_many :labels, dependent: :destroy + has_many :labels, dependent: :destroy, class_name: 'ProjectLabel' has_many :services, dependent: :destroy has_many :events, dependent: :destroy has_many :milestones, dependent: :destroy @@ -388,6 +388,10 @@ class Project < ActiveRecord::Base Project.count end end + + def group_ids + joins(:namespace).where(namespaces: { type: 'Group' }).pluck(:namespace_id) + end end def lfs_enabled? @@ -664,6 +668,10 @@ class Project < ActiveRecord::Base end end + def issue_reference_pattern + issues_tracker.reference_pattern + end + def default_issues_tracker? !external_issue_tracker end @@ -729,10 +737,8 @@ class Project < ActiveRecord::Base def create_labels Label.templates.each do |label| - label = label.dup - label.template = nil - label.project_id = self.id - label.save + params = label.attributes.except('id', 'template', 'created_at', 'updated_at') + Labels::FindOrCreateService.new(owner, self, params).execute end end @@ -1293,7 +1299,7 @@ class Project < ActiveRecord::Base environment_ids.where(ref: ref) end - environments.where(id: environment_ids).select do |environment| + environments.available.where(id: environment_ids).select do |environment| environment.includes_commit?(commit) end end diff --git a/app/models/project_label.rb b/app/models/project_label.rb new file mode 100644 index 00000000000..33c2b617715 --- /dev/null +++ b/app/models/project_label.rb @@ -0,0 +1,34 @@ +class ProjectLabel < Label + MAX_NUMBER_OF_PRIORITIES = 1 + + belongs_to :project + + validates :project, presence: true + + validate :permitted_numbers_of_priorities + validate :title_must_not_exist_at_group_level + + delegate :group, to: :project, allow_nil: true + + alias_attribute :subject, :project + + def to_reference(target_project = nil, format: :id) + super(project, target_project, format: format) + end + + private + + def title_must_not_exist_at_group_level + return unless group.present? && title_changed? + + if group.labels.with_title(self.title).exists? + errors.add(:title, :label_already_exists_at_group_level, group: group.name) + end + end + + def permitted_numbers_of_priorities + if priorities && priorities.size > MAX_NUMBER_OF_PRIORITIES + errors.add(:priorities, 'Number of permitted priorities exceeded') + end + end +end diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index d1df6d0292f..b26ddd518d7 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -3,6 +3,12 @@ class IssueTrackerService < Service default_value_for :category, 'issue_tracker' + # Pattern used to extract links from comments + # Override this method on services that uses different patterns + def reference_pattern + @reference_pattern ||= %r{(\b[A-Z][A-Z0-9_]+-|#{Issue.reference_prefix})(?\d+)} + end + def default? default end diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 97bcbacf2b9..f81b66fd219 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -13,6 +13,11 @@ class JiraService < IssueTrackerService before_update :reset_password + # {PROJECT-KEY}-{NUMBER} Examples: JIRA-1, PROJECT-1 + def reference_pattern + @reference_pattern ||= %r{(?\b([A-Z][A-Z0-9_]+-)\d+)} + end + def reset_password # don't reset the password if a new one is provided if api_url_changed? && !password_touched? diff --git a/app/models/todo.rb b/app/models/todo.rb index 6ae9956ade5..11c072dd000 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -52,7 +52,13 @@ class Todo < ActiveRecord::Base # Todos with highest priority first then oldest todos # Need to order by created_at last because of differences on Mysql and Postgres when joining by type "Merge_request/Issue" def order_by_labels_priority - highest_priority = highest_label_priority(["Issue", "MergeRequest"], "todos.target_id").to_sql + params = { + target_type: ['Issue', 'MergeRequest'], + target_column: "todos.target_id", + project_column: "todos.project_id" + } + + highest_priority = highest_label_priority(params).to_sql select("#{table_name}.*, (#{highest_priority}) AS highest_priority"). order(Gitlab::Database.nulls_last_order('highest_priority', 'ASC')). diff --git a/app/policies/group_label_policy.rb b/app/policies/group_label_policy.rb new file mode 100644 index 00000000000..7b34aa182eb --- /dev/null +++ b/app/policies/group_label_policy.rb @@ -0,0 +1,5 @@ +class GroupLabelPolicy < BasePolicy + def rules + delegate! @subject.group + end +end diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 97ff6233968..b65fb68cd88 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -19,6 +19,7 @@ class GroupPolicy < BasePolicy if master can! :create_projects can! :admin_milestones + can! :admin_label end # Only group owner and administrators can admin group diff --git a/app/policies/project_label_policy.rb b/app/policies/project_label_policy.rb new file mode 100644 index 00000000000..b12b4c5166b --- /dev/null +++ b/app/policies/project_label_policy.rb @@ -0,0 +1,5 @@ +class ProjectLabelPolicy < BasePolicy + def rules + delegate! @subject.project + end +end diff --git a/app/services/boards/lists/create_service.rb b/app/services/boards/lists/create_service.rb index abc7aeece39..fe0d762ccd2 100644 --- a/app/services/boards/lists/create_service.rb +++ b/app/services/boards/lists/create_service.rb @@ -3,7 +3,7 @@ module Boards class CreateService < BaseService def execute(board) List.transaction do - label = project.labels.find(params[:label_id]) + label = available_labels.find(params[:label_id]) position = next_position(board) create_list(board, label, position) @@ -12,6 +12,10 @@ module Boards private + def available_labels + LabelsFinder.new(current_user, project_id: project.id).execute + end + def next_position(board) max_position = board.lists.movable.maximum(:position) max_position.nil? ? 0 : max_position.succ diff --git a/app/services/boards/lists/generate_service.rb b/app/services/boards/lists/generate_service.rb index d8048f1c67e..939f9bfd068 100644 --- a/app/services/boards/lists/generate_service.rb +++ b/app/services/boards/lists/generate_service.rb @@ -19,8 +19,7 @@ module Boards end def find_or_create_label(params) - project.labels.create_with(color: params[:color]) - .find_or_create_by(name: params[:name]) + ::Labels::FindOrCreateService.new(current_user, project, params).execute end def label_params diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb index ff9a8310a8c..8ae15ad32f4 100644 --- a/app/services/create_deployment_service.rb +++ b/app/services/create_deployment_service.rb @@ -6,7 +6,13 @@ class CreateDeploymentService < BaseService ActiveRecord::Base.transaction do @deployable = deployable - @environment = prepare_environment + + @environment = environment + @environment.external_url = expanded_url if expanded_url + @environment.fire_state_event(action) + + return unless @environment.save + return if @environment.stopped? deploy.tap do |deployment| deployment.update_merge_request_metrics! @@ -27,13 +33,12 @@ class CreateDeploymentService < BaseService tag: params[:tag], sha: params[:sha], user: current_user, - deployable: @deployable) + deployable: @deployable, + on_stop: options[:on_stop]) end - def prepare_environment - project.environments.find_or_create_by(name: expanded_name) do |environment| - environment.external_url = expanded_url - end + def environment + @environment ||= project.environments.find_or_create_by(name: expanded_name) end def expanded_name @@ -61,4 +66,8 @@ class CreateDeploymentService < BaseService def variables params[:variables] || [] end + + def action + options[:action] || 'start' + end end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 57d521f2fea..bb92cd80cc9 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -80,17 +80,18 @@ class IssuableBaseService < BaseService def filter_labels_in_param(key) return if params[key].to_a.empty? - params[key] = project.labels.where(id: params[key]).pluck(:id) + params[key] = available_labels.where(id: params[key]).pluck(:id) end def find_or_create_label_ids labels = params.delete(:labels) return unless labels - params[:label_ids] = labels.split(",").map do |label_name| - project.labels.create_with(color: Label::DEFAULT_COLOR) - .find_or_create_by(title: label_name.strip) - .id + params[:label_ids] = labels.split(',').map do |label_name| + service = Labels::FindOrCreateService.new(current_user, project, title: label_name.strip) + label = service.execute + + label.id end end @@ -111,6 +112,10 @@ class IssuableBaseService < BaseService new_label_ids end + def available_labels + LabelsFinder.new(current_user, project_id: @project.id).execute + end + def merge_slash_commands_into_params!(issuable) description, command_params = SlashCommands::InterpretService.new(project, current_user). diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index ab667456db7..a2a5f57d069 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -52,8 +52,12 @@ module Issues end def cloneable_label_ids - @new_project.labels - .where(title: @old_issue.labels.pluck(:title)).pluck(:id) + params = { + project_id: @new_project.id, + title: @old_issue.labels.pluck(:title) + } + + LabelsFinder.new(current_user, params).execute.pluck(:id) end def cloneable_milestone_id diff --git a/app/services/labels/find_or_create_service.rb b/app/services/labels/find_or_create_service.rb new file mode 100644 index 00000000000..74291312c4e --- /dev/null +++ b/app/services/labels/find_or_create_service.rb @@ -0,0 +1,33 @@ +module Labels + class FindOrCreateService + def initialize(current_user, project, params = {}) + @current_user = current_user + @group = project.group + @project = project + @params = params.dup + end + + def execute + find_or_create_label + end + + private + + attr_reader :current_user, :group, :project, :params + + def available_labels + @available_labels ||= LabelsFinder.new(current_user, project_id: project.id).execute + end + + def find_or_create_label + new_label = available_labels.find_by(title: title) + new_label ||= project.labels.create(params) + + new_label + end + + def title + params[:title] || params[:name] + end + end +end diff --git a/app/services/labels/transfer_service.rb b/app/services/labels/transfer_service.rb new file mode 100644 index 00000000000..514679ed29d --- /dev/null +++ b/app/services/labels/transfer_service.rb @@ -0,0 +1,78 @@ +# Labels::TransferService class +# +# User for recreate the missing group labels at project level +# +module Labels + class TransferService + def initialize(current_user, old_group, project) + @current_user = current_user + @old_group = old_group + @project = project + end + + def execute + return unless old_group.present? + + Label.transaction do + labels_to_transfer.find_each do |label| + new_label_id = find_or_create_label!(label) + + next if new_label_id == label.id + + update_label_links(group_labels_applied_to_issues, old_label_id: label.id, new_label_id: new_label_id) + update_label_links(group_labels_applied_to_merge_requests, old_label_id: label.id, new_label_id: new_label_id) + update_label_priorities(old_label_id: label.id, new_label_id: new_label_id) + end + end + end + + private + + attr_reader :current_user, :old_group, :project + + def labels_to_transfer + label_ids = [] + label_ids << group_labels_applied_to_issues.select(:id) + label_ids << group_labels_applied_to_merge_requests.select(:id) + + union = Gitlab::SQL::Union.new(label_ids) + + Label.where("labels.id IN (#{union.to_sql})").reorder(nil).uniq + end + + def group_labels_applied_to_issues + Label.joins(:issues). + where( + issues: { project_id: project.id }, + labels: { type: 'GroupLabel', group_id: old_group.id } + ) + end + + def group_labels_applied_to_merge_requests + Label.joins(:merge_requests). + where( + merge_requests: { target_project_id: project.id }, + labels: { type: 'GroupLabel', group_id: old_group.id } + ) + end + + def find_or_create_label!(label) + params = label.attributes.slice('title', 'description', 'color') + new_label = FindOrCreateService.new(current_user, project, params).execute + + new_label.id + end + + def update_label_links(labels, old_label_id:, new_label_id:) + LabelLink.joins(:label). + merge(labels). + where(label_id: old_label_id). + update_all(label_id: new_label_id) + end + + def update_label_priorities(old_label_id:, new_label_id:) + LabelPriority.where(project_id: project.id, label_id: old_label_id). + update_all(label_id: new_label_id) + end + end +end diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index b037780c431..ab9056a3250 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -11,14 +11,14 @@ module MergeRequests def execute(merge_request) @merge_request = merge_request - return error('Merge request is not mergeable') unless @merge_request.mergeable? + return log_merge_error('Merge request is not mergeable', true) unless @merge_request.mergeable? merge_request.in_locked_state do if commit after_merge success else - error('Can not merge changes') + log_merge_error('Can not merge changes', true) end end end @@ -46,8 +46,8 @@ module MergeRequests merge_request.update(merge_error: e.message) false rescue StandardError => e - merge_request.update(merge_error: "Something went wrong during merge") - Rails.logger.error(e.message) + merge_request.update(merge_error: "Something went wrong during merge: #{e.message}") + log_merge_error(e.message) false ensure merge_request.update(in_progress_merge_commit_sha: nil) @@ -65,5 +65,17 @@ module MergeRequests def branch_deletion_user @merge_request.force_remove_source_branch? ? @merge_request.author : current_user end + + def log_merge_error(message, http_error = false) + Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{message}") + + error(message) if http_error + end + + def merge_request_info + project = merge_request.project + + "#{project.to_reference}#{merge_request.to_reference}" + end end end diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index f578f8dbea2..015f2828921 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -13,7 +13,7 @@ module Projects end def labels - @project.labels.select([:title, :color]) + LabelsFinder.new(current_user, project_id: project.id).execute.select([:title, :color]) end def commands(noteable, type) diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index bc7f8bf433b..28470f59807 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -28,6 +28,7 @@ module Projects Project.transaction do old_path = project.path_with_namespace old_namespace = project.namespace + old_group = project.group new_path = File.join(new_namespace.try(:path) || '', project.path) if Project.where(path: project.path, namespace_id: new_namespace.try(:id)).present? @@ -57,6 +58,9 @@ module Projects # Move wiki repo also if present gitlab_shell.mv_repository(project.repository_storage_path, "#{old_path}.wiki", "#{new_path}.wiki") + # Move missing group labels to project + Labels::TransferService.new(current_user, old_group, project).execute + # clear project cached events project.reset_events_cache diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb index e4ae3dec8aa..5a81194a5f4 100644 --- a/app/services/slash_commands/interpret_service.rb +++ b/app/services/slash_commands/interpret_service.rb @@ -116,8 +116,10 @@ module SlashCommands desc 'Add label(s)' params '~label1 ~"label 2"' condition do + available_labels = LabelsFinder.new(current_user, project_id: project.id).execute + current_user.can?(:"admin_#{issuable.to_ability_name}", project) && - project.labels.any? + available_labels.any? end command :label do |labels_param| label_ids = find_label_ids(labels_param) @@ -248,7 +250,7 @@ module SlashCommands def find_label_ids(labels_param) label_ids_by_reference = extract_references(labels_param, :label).map(&:id) - labels_ids_by_name = @project.labels.where(name: labels_param.split).select(:id) + labels_ids_by_name = LabelsFinder.new(current_user, project_id: project.id, name: labels_param.split).execute.select(:id) label_ids_by_reference | labels_ids_by_name end diff --git a/app/views/groups/labels/destroy.js.haml b/app/views/groups/labels/destroy.js.haml new file mode 100644 index 00000000000..3dfbfc77c0d --- /dev/null +++ b/app/views/groups/labels/destroy.js.haml @@ -0,0 +1,2 @@ +- if @group.labels.empty? + $('.labels').load(document.URL + ' .nothing-here-block').hide().fadeIn(1000) diff --git a/app/views/groups/labels/edit.html.haml b/app/views/groups/labels/edit.html.haml new file mode 100644 index 00000000000..836981fc6fd --- /dev/null +++ b/app/views/groups/labels/edit.html.haml @@ -0,0 +1,7 @@ +- page_title 'Edit', @label.name, 'Labels' + +%h3.page-title + Edit Label +%hr + += render 'shared/labels/form', url: group_label_path(@group, @label), back_path: @previous_labels_path diff --git a/app/views/groups/labels/index.html.haml b/app/views/groups/labels/index.html.haml new file mode 100644 index 00000000000..70783a63409 --- /dev/null +++ b/app/views/groups/labels/index.html.haml @@ -0,0 +1,20 @@ +- page_title 'Labels' + +.top-area.adjust + .nav-text + Labels can be applied to issues and merge requests. Group labels are available for any project within the group. + + .nav-controls + - if can?(current_user, :admin_label, @group) + = link_to new_group_label_path(@group), class: "btn btn-new" do + New label + +.labels + .other-labels + - if @labels.present? + %ul.content-list.manage-labels-list.js-other-labels + = render partial: 'shared/label', collection: @labels, as: :label + = paginate @labels, theme: 'gitlab' + - else + .nothing-here-block + No labels created yet. diff --git a/app/views/groups/labels/new.html.haml b/app/views/groups/labels/new.html.haml new file mode 100644 index 00000000000..2be87460b1d --- /dev/null +++ b/app/views/groups/labels/new.html.haml @@ -0,0 +1,8 @@ +- page_title 'New Label' +- header_title group_title(@group, 'Labels', group_labels_path(@group)) + +%h3.page-title + New Label +%hr + += render 'shared/labels/form', url: group_labels_path, back_path: @previous_labels_path diff --git a/app/views/layouts/nav/_group.html.haml b/app/views/layouts/nav/_group.html.haml index 27ac1760166..f7edb47b666 100644 --- a/app/views/layouts/nav/_group.html.haml +++ b/app/views/layouts/nav/_group.html.haml @@ -13,6 +13,10 @@ = link_to activity_group_path(@group), title: 'Activity' do %span Activity + = nav_link(controller: [:group, :labels]) do + = link_to group_labels_path(@group), title: 'Labels' do + %span + Labels = nav_link(controller: [:group, :milestones]) do = link_to group_milestones_path(@group), title: 'Milestones' do %span diff --git a/app/views/projects/builds/_user.html.haml b/app/views/projects/builds/_user.html.haml index 2642de8021d..83f299da651 100644 --- a/app/views/projects/builds/_user.html.haml +++ b/app/views/projects/builds/_user.html.haml @@ -1,4 +1,7 @@ by %a{ href: user_path(@build.user) } - = image_tag avatar_icon(@build.user, 24), class: "avatar s24" - %strong= @build.user.to_reference + %span.hidden-xs + = image_tag avatar_icon(@build.user, 24), class: "avatar s24" + %strong{ data: { toggle: 'tooltip', placement: 'top', title: @build.user.to_reference } } + = @build.user.name + %strong.visible-xs-inline= @build.user.to_reference diff --git a/app/views/projects/cycle_analytics/show.html.haml b/app/views/projects/cycle_analytics/show.html.haml index 7f346df8797..b647882efa0 100644 --- a/app/views/projects/cycle_analytics/show.html.haml +++ b/app/views/projects/cycle_analytics/show.html.haml @@ -2,10 +2,10 @@ - page_title "Cycle Analytics" = render "projects/pipelines/head" -#cycle-analytics{class: container_class, "v-cloak" => "true", data: { request_path: project_cycle_analytics_path(@project)}} +#cycle-analytics{class: container_class, "v-cloak" => "true", data: { request_path: project_cycle_analytics_path(@project) }} .bordered-box.landing.content-block{"v-if" => "!isHelpDismissed"} - = icon('times', class: 'dismiss-icon', "@click": "dismissLanding()") + = icon('times', class: 'dismiss-icon', "@click" => "dismissLanding()") .row .col-sm-3.col-xs-12.svg-container = custom_icon('icon_cycle_analytics_splash') diff --git a/app/views/projects/deployments/_actions.haml b/app/views/projects/deployments/_actions.haml index 22c4a75d213..58a214bdbd1 100644 --- a/app/views/projects/deployments/_actions.haml +++ b/app/views/projects/deployments/_actions.haml @@ -1,28 +1,15 @@ -- if can?(current_user, :create_deployment, deployment) && deployment.deployable - .pull-right - - - external_url = deployment.environment.external_url - - if external_url - = link_to external_url, target: '_blank', class: 'btn external-url' do - = icon('external-link') - - - actions = deployment.manual_actions - - if actions.present? - .inline - .dropdown - %a.dropdown-new.btn.btn-default{type: 'button', 'data-toggle' => 'dropdown'} - = custom_icon('icon_play') - = icon('caret-down') - %ul.dropdown-menu.dropdown-menu-align-right - - actions.each do |action| - %li - = link_to [:play, @project.namespace.becomes(Namespace), @project, action], method: :post, rel: 'nofollow' do - = custom_icon('icon_play') - %span= action.name.humanize +- if can?(current_user, :create_deployment, deployment) + - actions = deployment.manual_actions + - if actions.present? + .inline + .dropdown + %a.dropdown-new.btn.btn-default{type: 'button', 'data-toggle' => 'dropdown'} + = custom_icon('icon_play') + = icon('caret-down') + %ul.dropdown-menu.dropdown-menu-align-right + - actions.each do |action| + %li + = link_to [:play, @project.namespace.becomes(Namespace), @project, action], method: :post, rel: 'nofollow' do + = custom_icon('icon_play') + %span= action.name.humanize - - if local_assigns.fetch(:allow_rollback, false) - = link_to [:retry, @project.namespace.becomes(Namespace), @project, deployment.deployable], method: :post, class: 'btn btn-build' do - - if deployment.last? - Re-deploy - - else - Rollback diff --git a/app/views/projects/deployments/_deployment.html.haml b/app/views/projects/deployments/_deployment.html.haml index ca0005abd0c..9238f232c7e 100644 --- a/app/views/projects/deployments/_deployment.html.haml +++ b/app/views/projects/deployments/_deployment.html.haml @@ -17,4 +17,6 @@ #{time_ago_with_tooltip(deployment.created_at)} %td.hidden-xs - = render 'projects/deployments/actions', deployment: deployment, allow_rollback: true + .pull-right + = render 'projects/deployments/actions', deployment: deployment + = render 'projects/deployments/rollback', deployment: deployment diff --git a/app/views/projects/deployments/_rollback.haml b/app/views/projects/deployments/_rollback.haml new file mode 100644 index 00000000000..5941e01c6f1 --- /dev/null +++ b/app/views/projects/deployments/_rollback.haml @@ -0,0 +1,6 @@ +- if can?(current_user, :create_deployment, deployment) && deployment.deployable + = link_to [:retry, @project.namespace.becomes(Namespace), @project, deployment.deployable], method: :post, class: 'btn btn-build' do + - if deployment.last? + Re-deploy + - else + Rollback diff --git a/app/views/projects/environments/_environment.html.haml b/app/views/projects/environments/_environment.html.haml index 251694e897c..b75d5df4150 100644 --- a/app/views/projects/environments/_environment.html.haml +++ b/app/views/projects/environments/_environment.html.haml @@ -28,4 +28,8 @@ #{time_ago_with_tooltip(last_deployment.created_at)} %td.hidden-xs - = render 'projects/deployments/actions', deployment: last_deployment + .pull-right + = render 'projects/environments/external_url', environment: environment + = render 'projects/deployments/actions', deployment: last_deployment + = render 'projects/environments/stop', environment: environment + = render 'projects/deployments/rollback', deployment: last_deployment diff --git a/app/views/projects/environments/_external_url.html.haml b/app/views/projects/environments/_external_url.html.haml new file mode 100644 index 00000000000..4c8fe1c271b --- /dev/null +++ b/app/views/projects/environments/_external_url.html.haml @@ -0,0 +1,3 @@ +- if environment.external_url && can?(current_user, :read_environment, environment) + = link_to environment.external_url, target: '_blank', class: 'btn external-url' do + = icon('external-link') diff --git a/app/views/projects/environments/_stop.html.haml b/app/views/projects/environments/_stop.html.haml new file mode 100644 index 00000000000..69848123c17 --- /dev/null +++ b/app/views/projects/environments/_stop.html.haml @@ -0,0 +1,5 @@ +- if can?(current_user, :create_deployment, environment) && environment.stoppable? + .inline + = link_to stop_namespace_project_environment_path(@project.namespace, @project, environment), method: :post, + class: 'btn stop-env-link', rel: 'nofollow', data: { confirm: 'Are you sure you want to stop this environment?' } do + = icon('stop', class: 'stop-env-icon') diff --git a/app/views/projects/environments/index.html.haml b/app/views/projects/environments/index.html.haml index 721ba156334..8f555afcf11 100644 --- a/app/views/projects/environments/index.html.haml +++ b/app/views/projects/environments/index.html.haml @@ -3,14 +3,27 @@ = render "projects/pipelines/head" %div{ class: container_class } - - if can?(current_user, :create_environment, @project) && !@environments.blank? - .top-area + .top-area + %ul.nav-links + %li{class: ('active' if @scope.nil?)} + = link_to project_environments_path(@project) do + Available + %span.badge.js-available-environments-count + = number_with_delimiter(@all_environments.available.count) + + %li{class: ('active' if @scope == 'stopped')} + = link_to project_environments_path(@project, scope: :stopped) do + Stopped + %span.badge.js-stopped-environments-count + = number_with_delimiter(@all_environments.stopped.count) + + - if can?(current_user, :create_environment, @project) && !@all_environments.blank? .nav-controls = link_to new_namespace_project_environment_path(@project.namespace, @project), class: 'btn btn-create' do New environment .environments-container - - if @environments.blank? + - if @all_environments.blank? .blank-state.blank-state-no-icon %h2.blank-state-title You don't have any environments right now. diff --git a/app/views/projects/environments/show.html.haml b/app/views/projects/environments/show.html.haml index 90c59223a35..bcac73d3698 100644 --- a/app/views/projects/environments/show.html.haml +++ b/app/views/projects/environments/show.html.haml @@ -3,14 +3,16 @@ = render "projects/pipelines/head" %div{ class: container_class } - .top-area + .top-area.adjust .col-md-9 %h3.page-title= @environment.name.capitalize .col-md-3 .nav-controls + = render 'projects/environments/external_url', environment: @environment - if can?(current_user, :update_environment, @environment) = link_to 'Edit', edit_namespace_project_environment_path(@project.namespace, @project, @environment), class: 'btn' - = link_to 'Destroy', namespace_project_environment_path(@project.namespace, @project, @environment), data: { confirm: 'Are you sure you want to delete this environment?' }, class: 'btn btn-danger', method: :delete + - if can?(current_user, :create_deployment, @environment) && @environment.stoppable? + = link_to 'Stop', stop_namespace_project_environment_path(@project.namespace, @project, @environment), data: { confirm: 'Are you sure you want to stop this environment?' }, class: 'btn btn-danger', method: :post .deployments-container - if @deployments.blank? diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml index 8b1a8a8a2d9..c80210d6ff4 100644 --- a/app/views/projects/issues/_issue.html.haml +++ b/app/views/projects/issues/_issue.html.haml @@ -50,7 +50,7 @@ - if issue.labels.any?   - issue.labels.each do |label| - = link_to_label(label, project: issue.project) + = link_to_label(label, subject: issue.project) - if issue.tasks?   %span.task-status diff --git a/app/views/projects/issues/edit.html.haml b/app/views/projects/issues/edit.html.haml index 3a6fbbc7fbc..1b7d878c38c 100644 --- a/app/views/projects/issues/edit.html.haml +++ b/app/views/projects/issues/edit.html.haml @@ -1,4 +1,4 @@ -- page_title "Edit", "#{@issue.to_reference} #{@issue.title}", "Issues" +- page_title "Edit", "#{@issue.title} (#{@issue.to_reference})", "Issues" %h3.page-title Edit Issue ##{@issue.iid} diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 09347ad5fff..6f3f238a436 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -1,4 +1,4 @@ -- page_title "#{@issue.to_reference} #{@issue.title}", "Issues" +- page_title "#{@issue.title} (#{@issue.to_reference})", "Issues" - page_description @issue.description - page_card_attributes @issue.card_attributes diff --git a/app/views/projects/labels/_label.html.haml b/app/views/projects/labels/_label.html.haml deleted file mode 100644 index 71f7f354d72..00000000000 --- a/app/views/projects/labels/_label.html.haml +++ /dev/null @@ -1,50 +0,0 @@ -- label_css_id = dom_id(label) -%li{id: label_css_id, data: { id: label.id } } - = render "shared/label_row", label: label - - .visible-xs.visible-sm-inline-block.visible-md-inline-block.dropdown - %button.btn.btn-default.label-options-toggle{ data: { toggle: "dropdown" } } - Options - = icon('caret-down') - .dropdown-menu.dropdown-menu-align-right - %ul - %li - = link_to_label(label, type: :merge_request) do - = pluralize label.open_merge_requests_count, 'merge request' - %li - = link_to_label(label) do - = pluralize label.open_issues_count(current_user), 'open issue' - - if current_user - %li.label-subscription{ data: { url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label) } } - %a.js-subscribe-button.label-subscribe-button.subscription-status{ role: "button", href: "#", data: { toggle: "tooltip", status: label_subscription_status(label) } } - %span= label_subscription_toggle_button_text(label) - - if can? current_user, :admin_label, @project - %li - = link_to "Edit", edit_namespace_project_label_path(@project.namespace, @project, label) - %li - = link_to "Delete", namespace_project_label_path(@project.namespace, @project, label), title: "Delete", method: :delete, remote: true, data: {confirm: "Remove this label? Are you sure?"} - - .pull-right.hidden-xs.hidden-sm.hidden-md - = link_to_label(label, type: :merge_request, css_class: 'btn btn-transparent btn-action') do - = pluralize label.open_merge_requests_count, 'merge request' - = link_to_label(label, css_class: 'btn btn-transparent btn-action') do - = pluralize label.open_issues_count(current_user), 'open issue' - - - if current_user - .label-subscription.inline{ data: { url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label) } } - %button.js-subscribe-button.label-subscribe-button.btn.btn-transparent.btn-action.subscription-status{ type: "button", title: label_subscription_toggle_button_text(label), data: { toggle: "tooltip", status: label_subscription_status(label) } } - %span.sr-only= label_subscription_toggle_button_text(label) - = icon('eye', class: 'label-subscribe-button-icon') - = icon('spinner spin', class: 'label-subscribe-button-loading') - - - if can? current_user, :admin_label, @project - = link_to edit_namespace_project_label_path(@project.namespace, @project, label), title: "Edit", class: 'btn btn-transparent btn-action', data: {toggle: "tooltip"} do - %span.sr-only Edit - = icon('pencil-square-o') - = link_to namespace_project_label_path(@project.namespace, @project, label), title: "Delete", class: 'btn btn-transparent btn-action remove-row', method: :delete, remote: true, data: {confirm: "Remove this label? Are you sure?", toggle: "tooltip"} do - %span.sr-only Delete - = icon('trash-o') - - - if current_user - :javascript - new Subscription('##{dom_id(label)} .label-subscription'); diff --git a/app/views/projects/labels/destroy.js.haml b/app/views/projects/labels/destroy.js.haml index d59563b122a..8d09e2bda11 100644 --- a/app/views/projects/labels/destroy.js.haml +++ b/app/views/projects/labels/destroy.js.haml @@ -1,2 +1,2 @@ -- if @project.labels.size == 0 +- if @labels.empty? $('.labels').load(document.URL + ' .nothing-here-block').hide().fadeIn(1000) diff --git a/app/views/projects/labels/edit.html.haml b/app/views/projects/labels/edit.html.haml index 52b187e7e58..a80a07b52e6 100644 --- a/app/views/projects/labels/edit.html.haml +++ b/app/views/projects/labels/edit.html.haml @@ -6,4 +6,4 @@ %h3.page-title Edit Label %hr - = render 'form' + = render 'shared/labels/form', url: namespace_project_label_path(@project.namespace.becomes(Namespace), @project, @label), back_path: namespace_project_labels_path(@project.namespace, @project) diff --git a/app/views/projects/labels/index.html.haml b/app/views/projects/labels/index.html.haml index db66a0edbd8..f135bf6f6b4 100644 --- a/app/views/projects/labels/index.html.haml +++ b/app/views/projects/labels/index.html.haml @@ -16,21 +16,22 @@ .labels - if can?(current_user, :admin_label, @project) -# Only show it in the first page - - hide = @project.labels.empty? || (params[:page].present? && params[:page] != '1') + - hide = @available_labels.empty? || (params[:page].present? && params[:page] != '1') .prioritized-labels{ class: ('hide' if hide) } %h5 Prioritized Labels %ul.content-list.manage-labels-list.js-prioritized-labels{ "data-url" => set_priorities_namespace_project_labels_path(@project.namespace, @project) } %p.empty-message{ class: ('hidden' unless @prioritized_labels.empty?) } No prioritized labels yet - if @prioritized_labels.present? - = render @prioritized_labels + = render partial: 'shared/label', collection: @prioritized_labels, as: :label + .other-labels - if can?(current_user, :admin_label, @project) %h5{ class: ('hide' if hide) } Other Labels - - if @labels.present? - %ul.content-list.manage-labels-list.js-other-labels - = render @labels + %ul.content-list.manage-labels-list.js-other-labels + - if @labels.present? + = render partial: 'shared/label', collection: @labels, as: :label = paginate @labels, theme: 'gitlab' - - else + - if @labels.blank? .nothing-here-block - if can?(current_user, :admin_label, @project) Create a label or #{link_to 'generate a default set of labels', generate_namespace_project_labels_path(@project.namespace, @project), method: :post}. diff --git a/app/views/projects/labels/new.html.haml b/app/views/projects/labels/new.html.haml index a1bb66cfb6c..f0d9be744d1 100644 --- a/app/views/projects/labels/new.html.haml +++ b/app/views/projects/labels/new.html.haml @@ -6,4 +6,4 @@ %h3.page-title New Label %hr - = render 'form' + = render 'shared/labels/form', url: namespace_project_labels_path(@project.namespace.becomes(Namespace), @project), back_path: namespace_project_labels_path(@project.namespace, @project) diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index 68fb7d5a414..12408068834 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -62,7 +62,7 @@ - if merge_request.labels.any?   - merge_request.labels.each do |label| - = link_to_label(label, project: merge_request.project, type: 'merge_request') + = link_to_label(label, subject: merge_request.project, type: :merge_request) - if merge_request.tasks?   %span.task-status diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml index da6927879a4..9c6f562f7db 100644 --- a/app/views/projects/merge_requests/_new_submit.html.haml +++ b/app/views/projects/merge_requests/_new_submit.html.haml @@ -29,7 +29,11 @@ = link_to url_for(params), data: {target: 'div#commits', action: 'new', toggle: 'tab'} do Commits %span.badge= @commits.size - - if @pipeline + - if @pipelines.any? + %li.builds-tab + = link_to url_for(params), data: {target: 'div#pipelines', action: 'pipelines', toggle: 'tab'} do + Pipelines + %span.badge= @pipelines.size %li.builds-tab = link_to url_for(params), data: {target: 'div#builds', action: 'builds', toggle: 'tab'} do Builds @@ -44,9 +48,11 @@ = render "projects/merge_requests/show/commits" #diffs.diffs.tab-pane - # This tab is always loaded via AJAX - - if @pipeline + - if @pipelines.any? #builds.builds.tab-pane = render "projects/merge_requests/show/builds" + #pipelines.pipelines.tab-pane + = render "projects/merge_requests/show/pipelines" .mr-loading-status = spinner @@ -59,5 +65,5 @@ :javascript var merge_request = new MergeRequest({ action: "#{(@show_changes_tab ? 'new/diffs' : 'new')}", - buildsLoaded: "#{@pipeline ? 'true' : 'false'}" + buildsLoaded: "#{@pipelines.any? ? 'true' : 'false'}" }); diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 662463bc72b..cd98aaf8d75 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -1,4 +1,4 @@ -- page_title "#{@merge_request.to_reference} #{@merge_request.title}", "Merge Requests" +- page_title "#{@merge_request.title} (#{@merge_request.to_reference})", "Merge Requests" - page_description @merge_request.description - page_card_attributes @merge_request.card_attributes - content_for :page_specific_javascripts do @@ -61,7 +61,7 @@ %li.pipelines-tab = link_to pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do Pipelines - %span.badge= @merge_request.all_pipelines.size + %span.badge= @pipelines.size %li.builds-tab = link_to builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#builds', action: 'builds', toggle: 'tab' } do Builds diff --git a/app/views/projects/merge_requests/edit.html.haml b/app/views/projects/merge_requests/edit.html.haml index 7c3ac6652ee..03159f123f3 100644 --- a/app/views/projects/merge_requests/edit.html.haml +++ b/app/views/projects/merge_requests/edit.html.haml @@ -1,4 +1,4 @@ -- page_title "Edit", "#{@merge_request.to_reference} #{@merge_request.title}", "Merge Requests" +- page_title "Edit", "#{@merge_request.title} (#{@merge_request.to_reference}", "Merge Requests" %h3.page-title Edit Merge Request #{@merge_request.to_reference} diff --git a/app/views/projects/pipelines_settings/show.html.haml b/app/views/projects/pipelines_settings/show.html.haml index 0740e9b56ab..bebf0ccd54d 100644 --- a/app/views/projects/pipelines_settings/show.html.haml +++ b/app/views/projects/pipelines_settings/show.html.haml @@ -64,8 +64,8 @@ .checkbox = f.label :public_builds do = f.check_box :public_builds - %strong Public pipelines - .help-block Allow everyone to access pipelines for Public and Internal projects + %strong Public builds + .help-block Allow everyone to access builds traces for Public and Internal projects .form-group.append-bottom-default = f.label :runners_token, "Runners token", class: 'label-light' diff --git a/app/views/shared/_label.html.haml b/app/views/shared/_label.html.haml new file mode 100644 index 00000000000..40c8d2af226 --- /dev/null +++ b/app/views/shared/_label.html.haml @@ -0,0 +1,53 @@ +- label_css_id = dom_id(label) +- open_issues_count = label.open_issues_count(current_user, @project) +- open_merge_requests_count = label.open_merge_requests_count(current_user, @project) + +%li{id: label_css_id, data: { id: label.id } } + = render "shared/label_row", label: label + + .visible-xs.visible-sm-inline-block.visible-md-inline-block.dropdown + %button.btn.btn-default.label-options-toggle{ data: { toggle: "dropdown" } } + Options + = icon('caret-down') + .dropdown-menu.dropdown-menu-align-right + %ul + %li + = link_to_label(label, subject: @project, type: :merge_request) do + = pluralize open_merge_requests_count, 'merge request' + %li + = link_to_label(label, subject: @project) do + = pluralize open_issues_count, 'open issue' + - if current_user + %li.label-subscription{ data: toggle_subscription_data(label) } + %a.js-subscribe-button.label-subscribe-button.subscription-status{ role: "button", href: "#", data: { toggle: "tooltip", status: label_subscription_status(label) } } + %span= label_subscription_toggle_button_text(label) + - if can?(current_user, :admin_label, label) + %li + = link_to 'Edit', edit_label_path(label) + %li + = link_to 'Delete', destroy_label_path(label), title: 'Delete', method: :delete, remote: true, data: {confirm: 'Remove this label? Are you sure?'} + + .pull-right.hidden-xs.hidden-sm.hidden-md + = link_to_label(label, subject: @project, type: :merge_request, css_class: 'btn btn-transparent btn-action') do + = pluralize open_merge_requests_count, 'merge request' + = link_to_label(label, subject: @project, css_class: 'btn btn-transparent btn-action') do + = pluralize open_issues_count, 'open issue' + + - if current_user + .label-subscription.inline{ data: toggle_subscription_data(label) } + %button.js-subscribe-button.label-subscribe-button.btn.btn-transparent.btn-action.subscription-status{ type: "button", title: label_subscription_toggle_button_text(label), data: { toggle: "tooltip", status: label_subscription_status(label) } } + %span.sr-only= label_subscription_toggle_button_text(label) + = icon('eye', class: 'label-subscribe-button-icon', disabled: label.is_a?(GroupLabel)) + = icon('spinner spin', class: 'label-subscribe-button-loading') + + - if can?(current_user, :admin_label, label) + = link_to edit_label_path(label), title: "Edit", class: 'btn btn-transparent btn-action', data: {toggle: "tooltip"} do + %span.sr-only Edit + = icon('pencil-square-o') + = link_to destroy_label_path(label), title: "Delete", class: 'btn btn-transparent btn-action remove-row', method: :delete, remote: true, data: {confirm: label_deletion_confirm_text(label), toggle: "tooltip"} do + %span.sr-only Delete + = icon('trash-o') + + - if current_user && label.is_a?(ProjectLabel) + :javascript + new Subscription('##{dom_id(label)} .label-subscription'); diff --git a/app/views/shared/_label_row.html.haml b/app/views/shared/_label_row.html.haml index 6f593e8dff9..d28f9421ecf 100644 --- a/app/views/shared/_label_row.html.haml +++ b/app/views/shared/_label_row.html.haml @@ -3,13 +3,16 @@ .draggable-handler = icon('bars') .js-toggle-priority.toggle-priority{ data: { url: remove_priority_namespace_project_label_path(@project.namespace, @project, label), - dom_id: dom_id(label) } } + dom_id: dom_id(label), type: label.type } } %button.add-priority.btn.has-tooltip{ title: 'Prioritize', :'data-placement' => 'top' } = icon('star-o') %button.remove-priority.btn.has-tooltip{ title: 'Remove priority', :'data-placement' => 'top' } = icon('star') %span.label-name - = link_to_label(label, tooltip: false) + = link_to_label(label, subject: @project, tooltip: false) + - if defined?(@project) && @project.group.present? + %span.label-type + = label.model_name.human.titleize - if label.description %span.label-description = markdown_field(label, :description) diff --git a/app/views/shared/_labels_row.html.haml b/app/views/shared/_labels_row.html.haml index e324d0e5203..21b37a7c9ae 100644 --- a/app/views/shared/_labels_row.html.haml +++ b/app/views/shared/_labels_row.html.haml @@ -1,5 +1,5 @@ - labels.each do |label| %span.label-row.btn-group{ role: "group", aria: { label: label.name }, style: "color: #{text_color_for_bg(label.color)}" } - = link_to_label(label, css_class: 'btn btn-transparent') + = link_to_label(label, subject: @project, css_class: 'btn btn-transparent') %button.btn.btn-transparent.label-remove.js-label-filter-remove{ type: "button", style: "background-color: #{label.color};", data: { label: label.title } } = icon("times") diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index 31620297be0..ed93857e6d4 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -29,8 +29,9 @@ .filter-item.inline.labels-filter = render "shared/issuable/label_dropdown", selected: finder.labels.select(:title).uniq, use_id: false, selected_toggle: params[:label_name], data_options: { field_name: "label_name[]" } - .filter-item.inline.reset-filters - %a{href: page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name, :search])} Reset filters + - if issuable_filters_present + .filter-item.inline.reset-filters + %a{href: page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name, :search])} Reset filters .pull-right - if boards_page @@ -77,11 +78,10 @@ = hidden_field_tag :state_event, params[:state_event] .filter-item.inline = button_tag "Update #{type.to_s.humanize(capitalize: false)}", class: "btn update_selected_issues btn-save" - - - if !@labels.nil? - .row-content-block.second-block.filtered-labels{ class: ("hidden" if !@labels.any?) } - - if @labels.any? - = render "shared/labels_row", labels: @labels + - has_labels = @labels && @labels.any? + .row-content-block.second-block.filtered-labels{ class: ("hidden" unless has_labels) } + - if has_labels + = render 'shared/labels_row', labels: @labels :javascript new UsersSelect(); diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index a7944a60130..d410755cad1 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -88,19 +88,19 @@ - if issuable.assignee_id = f.hidden_field :assignee_id = dropdown_tag(user_dropdown_label(issuable.assignee_id, "Assignee"), options: { toggle_class: "js-dropdown-keep-input js-user-search js-issuable-form-dropdown js-assignee-search", title: "Select assignee", filter: true, dropdown_class: "dropdown-menu-user dropdown-menu-selectable dropdown-menu-assignee js-filter-submit", - placeholder: "Search assignee", data: { first_user: current_user.try(:username), null_user: true, current_user: true, project_id: project.try(:id), selected: issuable.assignee_id, field_name: "#{issuable.class.model_name.param_key}[assignee_id]", default_label: "Assignee", show_menu_above: true } }) + placeholder: "Search assignee", data: { first_user: current_user.try(:username), null_user: true, current_user: true, project_id: project.try(:id), selected: issuable.assignee_id, field_name: "#{issuable.class.model_name.param_key}[assignee_id]", default_label: "Assignee"} }) .form-group.issue-milestone = f.label :milestone_id, "Milestone", class: "control-label #{"col-lg-4" if has_due_date}" .col-sm-10{ class: ("col-lg-8" if has_due_date) } .issuable-form-select-holder - = render "shared/issuable/milestone_dropdown", selected: issuable.milestone, name: "#{issuable.class.model_name.param_key}[milestone_id]", show_any: false, show_menu_above: true, show_upcoming: false, extra_class: "js-issuable-form-dropdown js-dropdown-keep-input", dropdown_title: "Select milestone" + = render "shared/issuable/milestone_dropdown", selected: issuable.milestone, name: "#{issuable.class.model_name.param_key}[milestone_id]", show_any: false, show_upcoming: false, extra_class: "js-issuable-form-dropdown js-dropdown-keep-input", dropdown_title: "Select milestone" .form-group - - has_labels = issuable.project.labels.any? + - has_labels = @labels && @labels.any? = f.label :label_ids, "Labels", class: "control-label #{"col-lg-4" if has_due_date}" = f.hidden_field :label_ids, multiple: true, value: '' .col-sm-10{ class: "#{"col-lg-8" if has_due_date} #{'issuable-form-padding-top' if !has_labels}" } .issuable-form-select-holder - = render "shared/issuable/label_dropdown", classes: ["js-issuable-form-dropdown"], selected: issuable.labels, data_options: { field_name: "#{issuable.class.model_name.param_key}[label_ids][]", show_any: false, show_menu_above: 'true' }, dropdown_title: "Select label" + = render "shared/issuable/label_dropdown", classes: ["js-issuable-form-dropdown"], selected: issuable.labels, data_options: { field_name: "#{issuable.class.model_name.param_key}[label_ids][]", show_any: false}, dropdown_title: "Select label" - if has_due_date .col-lg-6 .form-group diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index ba9f0c27661..7363ead09ff 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -107,7 +107,7 @@ = dropdown_content do .js-due-date-calendar - - if issuable.project.labels.any? + - if @labels && @labels.any? - selected_labels = issuable.labels .block.labels .sidebar-collapsed-icon.js-sidebar-labels-tooltip{ title: issuable_labels_tooltip(issuable.labels_array), data: { placement: "left", container: "body" } } diff --git a/app/views/projects/labels/_form.html.haml b/app/views/shared/labels/_form.html.haml similarity index 79% rename from app/views/projects/labels/_form.html.haml rename to app/views/shared/labels/_form.html.haml index 6ab6ae50389..647e05e5ff7 100644 --- a/app/views/projects/labels/_form.html.haml +++ b/app/views/shared/labels/_form.html.haml @@ -1,4 +1,4 @@ -= form_for [@project.namespace.becomes(Namespace), @project, @label], html: { class: 'form-horizontal label-form js-quick-submit js-requires-input' } do |f| += form_for @label, as: :label, url: url, html: { class: 'form-horizontal label-form js-quick-submit js-requires-input' } do |f| = form_errors(@label) .form-group @@ -30,4 +30,4 @@ = f.submit 'Save changes', class: 'btn btn-save js-save-button' - else = f.submit 'Create Label', class: 'btn btn-create js-save-button' - = link_to "Cancel", namespace_project_labels_path(@project.namespace, @project), class: 'btn btn-cancel' + = link_to 'Cancel', back_path, class: 'btn btn-cancel' diff --git a/config/locales/en.yml b/config/locales/en.yml index cedb5e207bd..12a59be79f0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -5,6 +5,7 @@ en: hello: "Hello world" errors: messages: + label_already_exists_at_group_level: "already exists at group level for %{group}. Please choose another one." wrong_size: "is the wrong size (should be %{file_size})" size_too_small: "is too small (should be at least %{file_size})" size_too_big: "is too big (should be at most %{file_size})" diff --git a/config/routes/group.rb b/config/routes/group.rb index 06b464d79c8..4838c9d91c6 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -28,5 +28,7 @@ resources :groups, constraints: { id: /[a-zA-Z.0-9_\-]+(? 1').each do |label| + label_title = quote_string(label['title']) + duplicated_ids = select_all("SELECT id FROM labels WHERE title = '#{label_title}' ORDER BY id ASC").map{ |label| label['id'] } + label_id = duplicated_ids.first + duplicated_ids.delete(label_id) + + execute("UPDATE label_links SET label_id = #{label_id} WHERE label_id IN(#{duplicated_ids.join(",")})") + execute("DELETE FROM labels WHERE id IN(#{duplicated_ids.join(",")})") + end + + remove_index :labels, column: :project_id if index_exists?(:labels, :project_id) + remove_index :labels, column: :title if index_exists?(:labels, :title) + + add_concurrent_index :labels, [:group_id, :project_id, :title], unique: true + end + + def down + remove_index :labels, column: [:group_id, :project_id, :title] if index_exists?(:labels, [:group_id, :project_id, :title], unique: true) + + add_concurrent_index :labels, :project_id + add_concurrent_index :labels, :title + end +end diff --git a/db/migrate/20161018024215_migrate_labels_priority.rb b/db/migrate/20161018024215_migrate_labels_priority.rb new file mode 100644 index 00000000000..22bec2382f4 --- /dev/null +++ b/db/migrate/20161018024215_migrate_labels_priority.rb @@ -0,0 +1,36 @@ +class MigrateLabelsPriority < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = true + DOWNTIME_REASON = 'Prioritized labels will not work as expected until this migration is complete.' + + disable_ddl_transaction! + + def up + execute <<-EOF.strip_heredoc + INSERT INTO label_priorities (project_id, label_id, priority, created_at, updated_at) + SELECT labels.project_id, labels.id, labels.priority, NOW(), NOW() + FROM labels + WHERE labels.project_id IS NOT NULL + AND labels.priority IS NOT NULL; + EOF + end + + def down + if Gitlab::Database.mysql? + execute <<-EOF.strip_heredoc + UPDATE labels + INNER JOIN label_priorities ON labels.id = label_priorities.label_id AND labels.project_id = label_priorities.project_id + SET labels.priority = label_priorities.priority; + EOF + else + execute <<-EOF.strip_heredoc + UPDATE labels + SET priority = label_priorities.priority + FROM label_priorities + WHERE labels.id = label_priorities.label_id + AND labels.project_id = label_priorities.project_id; + EOF + end + end +end diff --git a/db/migrate/20161018024550_remove_priority_from_labels.rb b/db/migrate/20161018024550_remove_priority_from_labels.rb new file mode 100644 index 00000000000..b7416cca664 --- /dev/null +++ b/db/migrate/20161018024550_remove_priority_from_labels.rb @@ -0,0 +1,17 @@ +class RemovePriorityFromLabels < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = true + DOWNTIME_REASON = 'This migration removes an existing column' + + disable_ddl_transaction! + + def up + remove_column :labels, :priority, :integer, index: true + end + + def down + add_column :labels, :priority, :integer + add_concurrent_index :labels, :priority + end +end diff --git a/db/schema.rb b/db/schema.rb index 51ac0fbaeb5..65f55aa109b 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: 20161012180455) do +ActiveRecord::Schema.define(version: 20161018024550) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -380,6 +380,7 @@ ActiveRecord::Schema.define(version: 20161012180455) do t.string "deployable_type" t.datetime "created_at" t.datetime "updated_at" + t.string "on_stop" end add_index "deployments", ["project_id", "environment_id", "iid"], name: "index_deployments_on_project_id_and_environment_id_and_iid", using: :btree @@ -404,6 +405,7 @@ ActiveRecord::Schema.define(version: 20161012180455) do t.datetime "updated_at" t.string "external_url" t.string "environment_type" + t.string "state", default: "available", null: false end add_index "environments", ["project_id", "name"], name: "index_environments_on_project_id_and_name", using: :btree @@ -517,6 +519,17 @@ ActiveRecord::Schema.define(version: 20161012180455) do add_index "label_links", ["label_id"], name: "index_label_links_on_label_id", using: :btree add_index "label_links", ["target_id", "target_type"], name: "index_label_links_on_target_id_and_target_type", using: :btree + create_table "label_priorities", force: :cascade do |t| + t.integer "project_id", null: false + t.integer "label_id", null: false + t.integer "priority", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "label_priorities", ["priority"], name: "index_label_priorities_on_priority", using: :btree + add_index "label_priorities", ["project_id", "label_id"], name: "index_label_priorities_on_project_id_and_label_id", unique: true, using: :btree + create_table "labels", force: :cascade do |t| t.string "title" t.string "color" @@ -525,13 +538,13 @@ ActiveRecord::Schema.define(version: 20161012180455) do t.datetime "updated_at" t.boolean "template", default: false t.string "description" - t.integer "priority" t.text "description_html" + t.string "type" + t.integer "group_id" end - add_index "labels", ["priority"], name: "index_labels_on_priority", using: :btree - add_index "labels", ["project_id"], name: "index_labels_on_project_id", using: :btree - add_index "labels", ["title"], name: "index_labels_on_title", using: :btree + add_index "labels", ["group_id", "project_id", "title"], name: "index_labels_on_group_id_and_project_id_and_title", unique: true, using: :btree + add_index "labels", ["group_id"], name: "index_labels_on_group_id", using: :btree create_table "lfs_objects", force: :cascade do |t| t.string "oid", null: false @@ -1211,6 +1224,9 @@ ActiveRecord::Schema.define(version: 20161012180455) do add_foreign_key "boards", "projects" add_foreign_key "issue_metrics", "issues", on_delete: :cascade + add_foreign_key "label_priorities", "labels", on_delete: :cascade + add_foreign_key "label_priorities", "projects", on_delete: :cascade + add_foreign_key "labels", "namespaces", column: "group_id", on_delete: :cascade add_foreign_key "lists", "boards" add_foreign_key "lists", "labels" add_foreign_key "merge_request_metrics", "merge_requests", on_delete: :cascade diff --git a/doc/api/system_hooks.md b/doc/api/system_hooks.md index 1802fae14fe..073e99b7147 100644 --- a/doc/api/system_hooks.md +++ b/doc/api/system_hooks.md @@ -98,11 +98,8 @@ Example response: ## Delete system hook -Deletes a system hook. This is an idempotent API function and returns `200 OK` -even if the hook is not available. - -If the hook is deleted, a JSON object is returned. An error is raised if the -hook is not found. +Deletes a system hook. It returns `200 OK` if the hooks is deleted and +`404 Not Found` if the hook is not found. --- diff --git a/doc/user/project/settings/import_export.md b/doc/user/project/settings/import_export.md index 65ed9fae4ec..dfc762fe1d3 100644 --- a/doc/user/project/settings/import_export.md +++ b/doc/user/project/settings/import_export.md @@ -22,7 +22,8 @@ with all their related data and be moved into a new GitLab instance. | GitLab version | Import/Export version | | -------- | -------- | -| 8.12.0 to current | 0.1.4 | +| 8.13.0 to current | 0.1.5 | +| 8.12.0 | 0.1.4 | | 8.10.3 | 0.1.3 | | 8.10.0 | 0.1.2 | | 8.9.5 | 0.1.1 | diff --git a/features/steps/project/issues/labels.rb b/features/steps/project/issues/labels.rb index 2937d5d7ca8..f74a9b5df47 100644 --- a/features/steps/project/issues/labels.rb +++ b/features/steps/project/issues/labels.rb @@ -8,7 +8,7 @@ class Spinach::Features::ProjectIssuesLabels < Spinach::FeatureSteps end step 'I remove label \'bug\'' do - page.within "#label_#{bug_label.id}" do + page.within "#project_label_#{bug_label.id}" do first(:link, 'Delete').click end end diff --git a/lib/api/boards.rb b/lib/api/boards.rb index b14dd4f6e83..4ac491edc1b 100644 --- a/lib/api/boards.rb +++ b/lib/api/boards.rb @@ -65,8 +65,8 @@ module API requires :label_id, type: Integer, desc: 'The ID of an existing label' end post '/lists' do - unless user_project.labels.exists?(params[:label_id]) - render_api_error!({ error: "Label not found!" }, 400) + unless available_labels.exists?(params[:label_id]) + render_api_error!({ error: 'Label not found!' }, 400) end authorize!(:admin_list, user_project) diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index dfbdd597d29..f282a3b9cd6 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -6,17 +6,17 @@ module API resource :projects do before { authenticate! } - # Get a commit's statuses - # - # Parameters: - # id (required) - The ID of a project - # sha (required) - The commit hash - # ref (optional) - The ref - # stage (optional) - The stage - # name (optional) - The name - # all (optional) - Show all statuses, default: false - # Examples: - # GET /projects/:id/repository/commits/:sha/statuses + desc "Get a commit's statuses" do + success Entities::CommitStatus + end + params do + requires :id, type: String, desc: 'The ID of a project' + requires :sha, type: String, desc: 'The commit hash' + optional :ref, type: String, desc: 'The ref' + optional :stage, type: String, desc: 'The stage' + optional :name, type: String, desc: 'The name' + optional :all, type: String, desc: 'Show all statuses, default: false' + end get ':id/repository/commits/:sha/statuses' do authorize!(:read_commit_status, user_project) @@ -31,22 +31,23 @@ module API present paginate(statuses), with: Entities::CommitStatus end - # Post status to commit - # - # Parameters: - # id (required) - The ID of a project - # sha (required) - The commit hash - # ref (optional) - The ref - # state (required) - The state of the status. Can be: pending, running, success, failed or canceled - # target_url (optional) - The target URL to associate with this status - # description (optional) - A short description of the status - # name or context (optional) - A string label to differentiate this status from the status of other systems. Default: "default" - # Examples: - # POST /projects/:id/statuses/:sha + desc 'Post status to a commit' do + success Entities::CommitStatus + end + params do + requires :id, type: String, desc: 'The ID of a project' + requires :sha, type: String, desc: 'The commit hash' + requires :state, type: String, desc: 'The state of the status', + values: ['pending', 'running', 'success', 'failed', 'canceled'] + optional :ref, type: String, desc: 'The ref' + optional :target_url, type: String, desc: 'The target URL to associate with this status' + optional :description, type: String, desc: 'A short description of the status' + optional :name, type: String, desc: 'A string label to differentiate this status from the status of other systems. Default: "default"' + optional :context, type: String, desc: 'A string label to differentiate this status from the status of other systems. Default: "default"' + end post ':id/statuses/:sha' do authorize! :create_commit_status, user_project - required_attributes! [:state] - attrs = attributes_for_keys [:target_url, :description] + commit = @project.commit(params[:sha]) not_found! 'Commit' unless commit @@ -68,7 +69,7 @@ module API status = GenericCommitStatus.running_or_pending.find_or_initialize_by( project: @project, pipeline: pipeline, user: current_user, name: name, ref: ref) - status.attributes = attrs + status.attributes = declared(params).slice(:target_url, :description) begin case params[:state].to_s diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 67473f300c9..45120898b76 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -71,6 +71,10 @@ module API @project ||= find_project(params[:id]) end + def available_labels + @available_labels ||= LabelsFinder.new(current_user, project_id: user_project.id).execute + end + def find_project(id) project = Project.find_with_namespace(id) || Project.find_by(id: id) @@ -118,7 +122,7 @@ module API end def find_project_label(id) - label = user_project.labels.find_by_id(id) || user_project.labels.find_by_title(id) + label = available_labels.find_by_id(id) || available_labels.find_by_title(id) label || not_found!('Label') end @@ -197,16 +201,11 @@ module API def validate_label_params(params) errors = {} - if params[:labels].present? - params[:labels].split(',').each do |label_name| - label = user_project.labels.create_with( - color: Label::DEFAULT_COLOR).find_or_initialize_by( - title: label_name.strip) + params[:labels].to_s.split(',').each do |label_name| + label = available_labels.find_or_initialize_by(title: label_name.strip) + next if label.valid? - if label.invalid? - errors[label.title] = label.errors - end - end + errors[label.title] = label.errors end errors diff --git a/lib/api/labels.rb b/lib/api/labels.rb index c806829d69e..642e6345b9e 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -11,7 +11,7 @@ module API # Example Request: # GET /projects/:id/labels get ':id/labels' do - present user_project.labels, with: Entities::Label, current_user: current_user + present available_labels, with: Entities::Label, current_user: current_user end # Creates a new label diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 2b685621da9..bf8504e1101 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -86,14 +86,11 @@ module API render_api_error!({ labels: errors }, 400) end + attrs[:labels] = params[:labels] if params[:labels] + merge_request = ::MergeRequests::CreateService.new(user_project, current_user, attrs).execute if merge_request.valid? - # Find or create labels and attach to issue - if params[:labels].present? - merge_request.add_labels_by_names(params[:labels].split(",")) - end - present merge_request, with: Entities::MergeRequest, current_user: current_user else handle_merge_request_errors! merge_request.errors @@ -195,15 +192,11 @@ module API render_api_error!({ labels: errors }, 400) end + attrs[:labels] = params[:labels] if params[:labels] + merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, attrs).execute(merge_request) if merge_request.valid? - # Find or create labels and attach to issue - unless params[:labels].nil? - merge_request.remove_labels - merge_request.add_labels_by_names(params[:labels].split(",")) - end - present merge_request, with: Entities::MergeRequest, current_user: current_user else handle_merge_request_errors! merge_request.errors diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb index 2e76b91051f..794e34874f4 100644 --- a/lib/api/system_hooks.rb +++ b/lib/api/system_hooks.rb @@ -56,12 +56,10 @@ module API requires :id, type: Integer, desc: 'The ID of the system hook' end delete ":id" do - begin - hook = SystemHook.find(params[:id]) - present hook.destroy, with: Entities::Hook - rescue - # SystemHook raises an Error if no hook with id found - end + hook = SystemHook.find_by(id: params[:id]) + not_found!('System hook') unless hook + + present hook.destroy, with: Entities::Hook end end end diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index affe34394c2..cb213a76a05 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -208,8 +208,12 @@ module Banzai @references_per_project ||= begin refs = Hash.new { |hash, key| hash[key] = Set.new } - regex = Regexp.union(object_class.reference_pattern, - object_class.link_reference_pattern) + regex = + if uses_reference_pattern? + Regexp.union(object_class.reference_pattern, object_class.link_reference_pattern) + else + object_class.link_reference_pattern + end nodes.each do |node| node.to_html.scan(regex) do @@ -295,6 +299,14 @@ module Banzai value end end + + # There might be special cases like filters + # that should ignore reference pattern + # eg: IssueReferenceFilter when using a external issues tracker + # In those cases this method should be overridden on the filter subclass + def uses_reference_pattern? + true + end end end end diff --git a/lib/banzai/filter/external_issue_reference_filter.rb b/lib/banzai/filter/external_issue_reference_filter.rb index eaa702952cc..0d20be557a0 100644 --- a/lib/banzai/filter/external_issue_reference_filter.rb +++ b/lib/banzai/filter/external_issue_reference_filter.rb @@ -8,7 +8,7 @@ module Banzai # Public: Find `JIRA-123` issue references in text # - # ExternalIssueReferenceFilter.references_in(text) do |match, issue| + # ExternalIssueReferenceFilter.references_in(text, pattern) do |match, issue| # "##{issue}" # end # @@ -17,8 +17,8 @@ module Banzai # Yields the String match and the String issue reference. # # Returns a String replaced with the return of the block. - def self.references_in(text) - text.gsub(ExternalIssue.reference_pattern) do |match| + def self.references_in(text, pattern) + text.gsub(pattern) do |match| yield match, $~[:issue] end end @@ -27,7 +27,7 @@ module Banzai # Early return if the project isn't using an external tracker return doc if project.nil? || default_issues_tracker? - ref_pattern = ExternalIssue.reference_pattern + ref_pattern = issue_reference_pattern ref_start_pattern = /\A#{ref_pattern}\z/ each_node do |node| @@ -60,7 +60,7 @@ module Banzai def issue_link_filter(text, link_text: nil) project = context[:project] - self.class.references_in(text) do |match, id| + self.class.references_in(text, issue_reference_pattern) do |match, id| ExternalIssue.new(id, project) url = url_for_issue(id, project, only_path: context[:only_path]) @@ -82,18 +82,21 @@ module Banzai end def default_issues_tracker? - if RequestStore.active? - default_issues_tracker_cache[project.id] ||= - project.default_issues_tracker? - else - project.default_issues_tracker? - end + external_issues_cached(:default_issues_tracker?) + end + + def issue_reference_pattern + external_issues_cached(:issue_reference_pattern) end private - def default_issues_tracker_cache - RequestStore[:banzai_default_issues_tracker_cache] ||= {} + def external_issues_cached(attribute) + return project.public_send(attribute) unless RequestStore.active? + + cached_attributes = RequestStore[:banzai_external_issues_tracker_attributes] ||= Hash.new { |h, k| h[k] = {} } + cached_attributes[project.id][attribute] = project.public_send(attribute) if cached_attributes[project.id][attribute].nil? + cached_attributes[project.id][attribute] end end end diff --git a/lib/banzai/filter/html_entity_filter.rb b/lib/banzai/filter/html_entity_filter.rb index e008fd428b0..f3bd587c28b 100644 --- a/lib/banzai/filter/html_entity_filter.rb +++ b/lib/banzai/filter/html_entity_filter.rb @@ -5,7 +5,7 @@ module Banzai # Text filter that escapes these HTML entities: & " < > class HtmlEntityFilter < HTML::Pipeline::TextFilter def call - ERB::Util.html_escape(text) + ERB::Util.html_escape_once(text) end end end diff --git a/lib/banzai/filter/issue_reference_filter.rb b/lib/banzai/filter/issue_reference_filter.rb index 54c5f9a71a4..4d1bc687696 100644 --- a/lib/banzai/filter/issue_reference_filter.rb +++ b/lib/banzai/filter/issue_reference_filter.rb @@ -4,6 +4,10 @@ module Banzai # issues that do not exist are ignored. # # This filter supports cross-project references. + # + # When external issues tracker like Jira is activated we should not + # use issue reference pattern, but we should still be able + # to reference issues from other GitLab projects. class IssueReferenceFilter < AbstractReferenceFilter self.reference_type = :issue @@ -11,6 +15,10 @@ module Banzai Issue end + def uses_reference_pattern? + context[:project].default_issues_tracker? + end + def find_object(project, iid) issues_per_project[project][iid] end diff --git a/lib/banzai/filter/label_reference_filter.rb b/lib/banzai/filter/label_reference_filter.rb index 8f262ef3d8d..c24831e68ee 100644 --- a/lib/banzai/filter/label_reference_filter.rb +++ b/lib/banzai/filter/label_reference_filter.rb @@ -9,7 +9,7 @@ module Banzai end def find_object(project, id) - project.labels.find(id) + find_labels(project).find(id) end def self.references_in(text, pattern = Label.reference_pattern) @@ -35,7 +35,11 @@ module Banzai return unless project label_params = label_params(label_id, label_name) - project.labels.find_by(label_params) + find_labels(project).find_by(label_params) + end + + def find_labels(project) + LabelsFinder.new(nil, project_id: project.id).execute(authorized_only: false) end # Parameters to pass to `Label.find_by` based on the given arguments @@ -60,13 +64,50 @@ module Banzai end def object_link_text(object, matches) - if context[:project] == object.project - LabelsHelper.render_colored_label(object) + if same_group?(object) && namespace_match?(matches) + render_same_project_label(object) + elsif same_project?(object) + render_same_project_label(object) else - LabelsHelper.render_colored_cross_project_label(object) + render_cross_project_label(object, matches) end end + def same_group?(object) + object.is_a?(GroupLabel) && object.group == project.group + end + + def namespace_match?(matches) + matches[:project].blank? || matches[:project] == project.path_with_namespace + end + + def same_project?(object) + object.is_a?(ProjectLabel) && object.project == project + end + + def user + context[:current_user] || context[:author] + end + + def project + context[:project] + end + + def render_same_project_label(object) + LabelsHelper.render_colored_label(object) + end + + def render_cross_project_label(object, matches) + source_project = + if matches[:project] + Project.find_with_namespace(matches[:project]) + else + object.project + end + + LabelsHelper.render_colored_cross_project_label(object, source_project) + end + def unescape_html_entities(text) CGI.unescapeHTML(text.to_s) end diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index 2fd1fced65c..3e33c9399e2 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -109,6 +109,7 @@ module Ci validate_job_stage!(name, job) validate_job_dependencies!(name, job) + validate_job_environment!(name, job) end end @@ -150,6 +151,35 @@ module Ci end end + def validate_job_environment!(name, job) + return unless job[:environment] + return unless job[:environment].is_a?(Hash) + + environment = job[:environment] + validate_on_stop_job!(name, environment, environment[:on_stop]) + end + + def validate_on_stop_job!(name, environment, on_stop) + return unless on_stop + + on_stop_job = @jobs[on_stop.to_sym] + unless on_stop_job + raise ValidationError, "#{name} job: on_stop job #{on_stop} is not defined" + end + + unless on_stop_job[:environment] + raise ValidationError, "#{name} job: on_stop job #{on_stop} does not have environment defined" + end + + unless on_stop_job[:environment][:name] == environment[:name] + raise ValidationError, "#{name} job: on_stop job #{on_stop} have different environment name" + end + + unless on_stop_job[:environment][:action] == 'stop' + raise ValidationError, "#{name} job: on_stop job #{on_stop} needs to have action stop defined" + end + end + def process?(only_params, except_params, ref, tag, trigger_request) if only_params.present? return false unless matching?(only_params, ref, tag, trigger_request) diff --git a/lib/gitlab/ci/config/node/environment.rb b/lib/gitlab/ci/config/node/environment.rb index d388ab6b879..9a95ef43628 100644 --- a/lib/gitlab/ci/config/node/environment.rb +++ b/lib/gitlab/ci/config/node/environment.rb @@ -8,7 +8,7 @@ module Gitlab class Environment < Entry include Validatable - ALLOWED_KEYS = %i[name url] + ALLOWED_KEYS = %i[name url action on_stop] validations do validate do @@ -35,6 +35,12 @@ module Gitlab length: { maximum: 255 }, addressable_url: true, allow_nil: true + + validates :action, + inclusion: { in: %w[start stop], message: 'should be start or stop' }, + allow_nil: true + + validates :on_stop, type: String, allow_nil: true end end @@ -54,9 +60,17 @@ module Gitlab value[:url] end + def action + value[:action] || 'start' + end + + def on_stop + value[:on_stop] + end + def value case @config - when String then { name: @config } + when String then { name: @config, action: 'start' } when Hash then @config else {} end diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index 501d5a95547..65ee85ca5a9 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -74,8 +74,8 @@ module Gitlab end def create_label(name) - color = nice_label_color(name) - Label.create!(project_id: project.id, title: name, color: color) + params = { title: name, color: nice_label_color(name) } + ::Labels::FindOrCreateService.new(project.owner, project, params).execute end def user_info(person_id) @@ -122,25 +122,21 @@ module Gitlab author_id = user_info(bug['ixPersonOpenedBy'])[:gitlab_id] || project.creator_id issue = Issue.create!( - project_id: project.id, - title: bug['sTitle'], - description: body, - author_id: author_id, - assignee_id: assignee_id, - state: bug['fOpen'] == 'true' ? 'opened' : 'closed' + iid: bug['ixBug'], + project_id: project.id, + title: bug['sTitle'], + description: body, + author_id: author_id, + assignee_id: assignee_id, + state: bug['fOpen'] == 'true' ? 'opened' : 'closed', + created_at: date, + updated_at: DateTime.parse(bug['dtLastUpdated']) ) - issue.add_labels_by_names(labels) - if issue.iid != bug['ixBug'] - issue.update_attribute(:iid, bug['ixBug']) - end + issue_labels = ::LabelsFinder.new(project.owner, project_id: project.id, title: labels).execute + issue.update_attribute(:label_ids, issue_labels.pluck(:id)) import_issue_comments(issue, comments) - - issue.update_attribute(:created_at, date) - - last_update = DateTime.parse(bug['dtLastUpdated']) - issue.update_attribute(:updated_at, last_update) end end diff --git a/lib/gitlab/gfm/reference_rewriter.rb b/lib/gitlab/gfm/reference_rewriter.rb index 78d7a4f27cf..a7c596dced0 100644 --- a/lib/gitlab/gfm/reference_rewriter.rb +++ b/lib/gitlab/gfm/reference_rewriter.rb @@ -58,7 +58,7 @@ module Gitlab referable = find_referable(reference) return reference unless referable - cross_reference = referable.to_reference(target_project) + cross_reference = build_cross_reference(referable, target_project) return reference if reference == cross_reference new_text = before + cross_reference + after @@ -72,6 +72,14 @@ module Gitlab extractor.all.first end + def build_cross_reference(referable, target_project) + if referable.respond_to?(:project) + referable.to_reference(target_project) + else + referable.to_reference(@source_project, target_project) + end + end + def substitution_valid?(substituted) @original_html == markdown(substituted) end diff --git a/lib/gitlab/github_import/label_formatter.rb b/lib/gitlab/github_import/label_formatter.rb index 2cad7fca88e..942dfb3312b 100644 --- a/lib/gitlab/github_import/label_formatter.rb +++ b/lib/gitlab/github_import/label_formatter.rb @@ -14,9 +14,13 @@ module Gitlab end def create! - project.labels.find_or_create_by!(title: title) do |label| - label.color = color - end + params = attributes.except(:project) + service = ::Labels::FindOrCreateService.new(project.owner, project, params) + label = service.execute + + raise ActiveRecord::RecordInvalid.new(label) unless label.persisted? + + label end private diff --git a/lib/gitlab/google_code_import/importer.rb b/lib/gitlab/google_code_import/importer.rb index 62da327931f..6a68e786b4f 100644 --- a/lib/gitlab/google_code_import/importer.rb +++ b/lib/gitlab/google_code_import/importer.rb @@ -92,19 +92,17 @@ module Gitlab end issue = Issue.create!( - project_id: project.id, - title: raw_issue["title"], - description: body, - author_id: project.creator_id, - assignee_id: assignee_id, - state: raw_issue["state"] == "closed" ? "closed" : "opened" + iid: raw_issue['id'], + project_id: project.id, + title: raw_issue['title'], + description: body, + author_id: project.creator_id, + assignee_id: assignee_id, + state: raw_issue['state'] == 'closed' ? 'closed' : 'opened' ) - issue.add_labels_by_names(labels) - - if issue.iid != raw_issue["id"] - issue.update_attribute(:iid, raw_issue["id"]) - end + issue_labels = ::LabelsFinder.new(project.owner, project_id: project.id, title: labels).execute + issue.update_attribute(:label_ids, issue_labels.pluck(:id)) import_issue_comments(issue, comments) end @@ -236,8 +234,8 @@ module Gitlab end def create_label(name) - color = nice_label_color(name) - Label.create!(project_id: project.id, name: name, color: color) + params = { name: name, color: nice_label_color(name) } + ::Labels::FindOrCreateService.new(project.owner, project, params).execute end def format_content(raw_content) diff --git a/lib/gitlab/import_export.rb b/lib/gitlab/import_export.rb index 181e288a014..eb667a85b78 100644 --- a/lib/gitlab/import_export.rb +++ b/lib/gitlab/import_export.rb @@ -3,7 +3,7 @@ module Gitlab extend self # For every version update, the version history in import_export.md has to be kept up to date. - VERSION = '0.1.4' + VERSION = '0.1.5' FILENAME_LIMIT = 50 def export_path(relative_path:) diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb index b9e4042220a..f755a404693 100644 --- a/lib/gitlab/import_export/attribute_cleaner.rb +++ b/lib/gitlab/import_export/attribute_cleaner.rb @@ -1,7 +1,7 @@ module Gitlab module ImportExport class AttributeCleaner - ALLOWED_REFERENCES = RelationFactory::PROJECT_REFERENCES + RelationFactory::USER_REFERENCES + ALLOWED_REFERENCES = RelationFactory::PROJECT_REFERENCES + RelationFactory::USER_REFERENCES + ['group_id'] def self.clean!(relation_hash:) relation_hash.reject! do |key, _value| diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index bb9d1080330..e6ecd118609 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -1,6 +1,7 @@ # Model relationships to be included in the project import/export project_tree: - - :labels + - labels: + :priorities - milestones: - :events - issues: @@ -9,7 +10,8 @@ project_tree: - :author - :events - label_links: - - :label + - label: + :priorities - milestone: - :events - snippets: @@ -26,7 +28,8 @@ project_tree: - :merge_request_diff - :events - label_links: - - :label + - label: + :priorities - milestone: - :events - pipelines: @@ -71,6 +74,10 @@ excluded_attributes: - :awardable_id methods: + labels: + - :type + label: + - :type statuses: - :type services: diff --git a/lib/gitlab/import_export/json_hash_builder.rb b/lib/gitlab/import_export/json_hash_builder.rb index 0cc10f40087..48c09dafcb6 100644 --- a/lib/gitlab/import_export/json_hash_builder.rb +++ b/lib/gitlab/import_export/json_hash_builder.rb @@ -65,11 +65,17 @@ module Gitlab # +value+ existing model to be included in the hash # +parsed_hash+ the original hash def parse_hash(value) + return nil if already_contains_methods?(value) + @attributes_finder.parse(value) do |hash| { include: hash_or_merge(value, hash) } end end + def already_contains_methods?(value) + value.is_a?(Hash) && value.values.detect { |val| val[:methods]} + end + # Adds new model configuration to an existing hash with key +current_key+ # It may include exceptions or other attribute detail configuration, parsed by +@attributes_finder+ # diff --git a/lib/gitlab/import_export/project_tree_restorer.rb b/lib/gitlab/import_export/project_tree_restorer.rb index 5a109f24f9f..7cdba880a93 100644 --- a/lib/gitlab/import_export/project_tree_restorer.rb +++ b/lib/gitlab/import_export/project_tree_restorer.rb @@ -110,7 +110,7 @@ module Gitlab def create_relation(relation, relation_hash_list) relation_array = [relation_hash_list].flatten.map do |relation_hash| Gitlab::ImportExport::RelationFactory.create(relation_sym: relation.to_sym, - relation_hash: relation_hash, + relation_hash: parsed_relation_hash(relation_hash), members_mapper: members_mapper, user: @user, project_id: restored_project.id) @@ -118,6 +118,10 @@ module Gitlab relation_hash_list.is_a?(Array) ? relation_array : relation_array.first end + + def parsed_relation_hash(relation_hash) + relation_hash.merge!('group_id' => restored_project.group.try(:id), 'project_id' => restored_project.id) + end end end end diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 9300f789e1b..dc630e76411 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -9,7 +9,10 @@ module Gitlab builds: 'Ci::Build', hooks: 'ProjectHook', merge_access_levels: 'ProtectedBranch::MergeAccessLevel', - push_access_levels: 'ProtectedBranch::PushAccessLevel' }.freeze + push_access_levels: 'ProtectedBranch::PushAccessLevel', + labels: :project_labels, + priorities: :label_priorities, + label: :project_label }.freeze USER_REFERENCES = %w[author_id assignee_id updated_by_id user_id].freeze @@ -19,9 +22,7 @@ module Gitlab IMPORTED_OBJECT_MAX_RETRIES = 5.freeze - EXISTING_OBJECT_CHECK = %i[milestone milestones label labels].freeze - - FINDER_ATTRIBUTES = %w[title project_id].freeze + EXISTING_OBJECT_CHECK = %i[milestone milestones label labels project_label project_labels project_label group_label].freeze def self.create(*args) new(*args).create @@ -56,6 +57,8 @@ module Gitlab update_user_references update_project_references + + handle_group_label if group_label? reset_ci_tokens if @relation_name == 'Ci::Trigger' @relation_hash['data'].deep_symbolize_keys! if @relation_name == :events && @relation_hash['data'] set_st_diffs if @relation_name == :merge_request_diff @@ -123,6 +126,20 @@ module Gitlab @relation_hash['target_project_id'] && @relation_hash['target_project_id'] == @relation_hash['source_project_id'] end + def group_label? + @relation_hash['type'] == 'GroupLabel' + end + + def handle_group_label + # If there's no group, move the label to a project label + if @relation_hash['group_id'] + @relation_hash['project_id'] = nil + @relation_name = :group_label + else + @relation_hash['type'] = 'ProjectLabel' + end + end + def reset_ci_tokens return unless Gitlab::ImportExport.reset_tokens? @@ -171,11 +188,9 @@ module Gitlab # Otherwise always create the record, skipping the extra SELECT clause. @existing_or_new_object ||= begin if EXISTING_OBJECT_CHECK.include?(@relation_name) - events = parsed_relation_hash.delete('events') + attribute_hash = attribute_hash_for(['events', 'priorities']) - unless events.blank? - existing_object.assign_attributes(events: events) - end + existing_object.assign_attributes(attribute_hash) if attribute_hash.any? existing_object else @@ -184,14 +199,22 @@ module Gitlab end end + def attribute_hash_for(attributes) + attributes.inject({}) do |hash, value| + hash[value] = parsed_relation_hash.delete(value) if parsed_relation_hash[value] + hash + end + end + def existing_object @existing_object ||= begin - finder_hash = parsed_relation_hash.slice(*FINDER_ATTRIBUTES) + finder_attributes = @relation_name == :group_label ? %w[title group_id] : %w[title project_id] + finder_hash = parsed_relation_hash.slice(*finder_attributes) existing_object = relation_class.find_or_create_by(finder_hash) # Done in two steps, as MySQL behaves differently than PostgreSQL using # the +find_or_create_by+ method and does not return the ID the second time. - existing_object.update(parsed_relation_hash) + existing_object.update!(parsed_relation_hash) existing_object end end diff --git a/lib/gitlab/issues_labels.rb b/lib/gitlab/issues_labels.rb index 1bec6088292..01a2c19ab23 100644 --- a/lib/gitlab/issues_labels.rb +++ b/lib/gitlab/issues_labels.rb @@ -18,8 +18,8 @@ module Gitlab { title: "enhancement", color: green } ] - labels.each do |label| - project.labels.create(label) + labels.each do |params| + ::Labels::FindOrCreateService.new(project.owner, project).execute(params) end end end diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index a0870891cf4..ad15b3f8f40 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -2,16 +2,10 @@ require 'spec_helper' describe Groups::GroupMembersController do let(:user) { create(:user) } + let(:group) { create(:group, :public) } - describe '#index' do - let(:group) { create(:group) } - - before do - group.add_owner(user) - stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) - end - - it 'renders index with group members' do + describe 'GET index' do + it 'renders index with 200 status code' do get :index, group_id: group expect(response).to have_http_status(200) @@ -19,74 +13,56 @@ describe Groups::GroupMembersController do end end - describe '#destroy' do - let(:group) { create(:group, :public) } + describe 'DELETE destroy' do + let(:member) { create(:group_member, :developer, group: group) } + + before { sign_in(user) } context 'when member is not found' do it 'returns 403' do - delete :destroy, group_id: group, - id: 42 + delete :destroy, group_id: group, id: 42 expect(response).to have_http_status(403) end end context 'when member is found' do - let(:user) { create(:user) } - let(:group_user) { create(:user) } - let(:member) do - group.add_developer(group_user) - group.members.find_by(user_id: group_user) - end - context 'when user does not have enough rights' do - before do - group.add_developer(user) - sign_in(user) - end + before { group.add_developer(user) } it 'returns 403' do - delete :destroy, group_id: group, - id: member + delete :destroy, group_id: group, id: member expect(response).to have_http_status(403) - expect(group.users).to include group_user + expect(group.members).to include member end end context 'when user has enough rights' do - before do - group.add_owner(user) - sign_in(user) - end + before { group.add_owner(user) } it '[HTML] removes user from members' do - delete :destroy, group_id: group, - id: member + delete :destroy, group_id: group, id: member expect(response).to set_flash.to 'User was successfully removed from group.' expect(response).to redirect_to(group_group_members_path(group)) - expect(group.users).not_to include group_user + expect(group.members).not_to include member end it '[JS] removes user from members' do - xhr :delete, :destroy, group_id: group, - id: member + xhr :delete, :destroy, group_id: group, id: member expect(response).to be_success - expect(group.users).not_to include group_user + expect(group.members).not_to include member end end end end - describe '#leave' do - let(:group) { create(:group, :public) } - let(:user) { create(:user) } + describe 'DELETE leave' do + before { sign_in(user) } context 'when member is not found' do - before { sign_in(user) } - it 'returns 404' do delete :leave, group_id: group @@ -96,10 +72,7 @@ describe Groups::GroupMembersController do context 'when member is found' do context 'and is not an owner' do - before do - group.add_developer(user) - sign_in(user) - end + before { group.add_developer(user) } it 'removes user from members' do delete :leave, group_id: group @@ -111,10 +84,7 @@ describe Groups::GroupMembersController do end context 'and is an owner' do - before do - group.add_owner(user) - sign_in(user) - end + before { group.add_owner(user) } it 'cannot removes himself from the group' do delete :leave, group_id: group @@ -124,10 +94,7 @@ describe Groups::GroupMembersController do end context 'and is a requester' do - before do - group.request_access(user) - sign_in(user) - end + before { group.request_access(user) } it 'removes user from members' do delete :leave, group_id: group @@ -141,13 +108,8 @@ describe Groups::GroupMembersController do end end - describe '#request_access' do - let(:group) { create(:group, :public) } - let(:user) { create(:user) } - - before do - sign_in(user) - end + describe 'POST request_access' do + before { sign_in(user) } it 'creates a new GroupMember that is not a team member' do post :request_access, group_id: group @@ -159,53 +121,39 @@ describe Groups::GroupMembersController do end end - describe '#approve_access_request' do - let(:group) { create(:group, :public) } + describe 'POST approve_access_request' do + let(:member) { create(:group_member, :access_request, group: group) } + + before { sign_in(user) } context 'when member is not found' do it 'returns 403' do - post :approve_access_request, group_id: group, - id: 42 + post :approve_access_request, group_id: group, id: 42 expect(response).to have_http_status(403) end end context 'when member is found' do - let(:user) { create(:user) } - let(:group_requester) { create(:user) } - let(:member) do - group.request_access(group_requester) - group.requesters.find_by(user_id: group_requester) - end - context 'when user does not have enough rights' do - before do - group.add_developer(user) - sign_in(user) - end + before { group.add_developer(user) } it 'returns 403' do - post :approve_access_request, group_id: group, - id: member + post :approve_access_request, group_id: group, id: member expect(response).to have_http_status(403) - expect(group.users).not_to include group_requester + expect(group.members).not_to include member end end context 'when user has enough rights' do - before do - group.add_owner(user) - sign_in(user) - end + before { group.add_owner(user) } it 'adds user to members' do - post :approve_access_request, group_id: group, - id: member + post :approve_access_request, group_id: group, id: member expect(response).to redirect_to(group_group_members_path(group)) - expect(group.users).to include group_requester + expect(group.members).to include member end end end diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb index 3492b6ffbbb..622ab154493 100644 --- a/spec/controllers/projects/labels_controller_spec.rb +++ b/spec/controllers/projects/labels_controller_spec.rb @@ -1,52 +1,73 @@ require 'spec_helper' describe Projects::LabelsController do - let(:project) { create(:project) } + let(:group) { create(:group) } + let(:project) { create(:project, namespace: group) } let(:user) { create(:user) } before do project.team << [user, :master] + sign_in(user) end describe 'GET #index' do - def create_label(attributes) - create(:label, attributes.merge(project: project)) - end + let!(:label_1) { create(:label, project: project, priority: 1, title: 'Label 1') } + let!(:label_2) { create(:label, project: project, priority: 3, title: 'Label 2') } + let!(:label_3) { create(:label, project: project, priority: 1, title: 'Label 3') } + let!(:label_4) { create(:label, project: project, title: 'Label 4') } + let!(:label_5) { create(:label, project: project, title: 'Label 5') } + + let!(:group_label_1) { create(:group_label, group: group, title: 'Group Label 1') } + let!(:group_label_2) { create(:group_label, group: group, title: 'Group Label 2') } + let!(:group_label_3) { create(:group_label, group: group, title: 'Group Label 3') } + let!(:group_label_4) { create(:group_label, group: group, title: 'Group Label 4') } before do - 15.times { |i| create_label(priority: (i % 3) + 1, title: "label #{15 - i}") } - 5.times { |i| create_label(title: "label #{100 - i}") } - - get :index, namespace_id: project.namespace.to_param, project_id: project.to_param + create(:label_priority, project: project, label: group_label_1, priority: 3) + create(:label_priority, project: project, label: group_label_2, priority: 1) end context '@prioritized_labels' do - let(:prioritized_labels) { assigns(:prioritized_labels) } + before do + list_labels + end - it 'contains only prioritized labels' do - expect(prioritized_labels).to all(have_attributes(priority: a_value > 0)) + it 'does not include labels without priority' do + list_labels + + expect(assigns(:prioritized_labels)).not_to include(group_label_3, group_label_4, label_4, label_5) end it 'is sorted by priority, then label title' do - priorities_and_titles = prioritized_labels.pluck(:priority, :title) - - expect(priorities_and_titles.sort).to eq(priorities_and_titles) + expect(assigns(:prioritized_labels)).to eq [group_label_2, label_1, label_3, group_label_1, label_2] end end context '@labels' do - let(:labels) { assigns(:labels) } - - it 'contains only unprioritized labels' do - expect(labels).to all(have_attributes(priority: nil)) - end - it 'is sorted by label title' do - titles = labels.pluck(:title) + list_labels - expect(titles.sort).to eq(titles) + expect(assigns(:labels)).to eq [group_label_3, group_label_4, label_4, label_5] end + + it 'does not include labels with priority' do + list_labels + + expect(assigns(:labels)).not_to include(group_label_2, label_1, label_3, group_label_1, label_2) + end + + it 'does not include group labels when project does not belong to a group' do + project.update(namespace: create(:namespace)) + + list_labels + + expect(assigns(:labels)).not_to include(group_label_3, group_label_4) + end + end + + def list_labels + get :index, namespace_id: project.namespace.to_param, project_id: project.to_param end end end diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 074f85157de..8519ebc1d5f 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -1,10 +1,192 @@ require('spec_helper') describe Projects::ProjectMembersController do - describe '#apply_import' do - let(:project) { create(:project) } + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + + describe 'GET index' do + it 'renders index with 200 status code' do + get :index, namespace_id: project.namespace, project_id: project + + expect(response).to have_http_status(200) + expect(response).to render_template(:index) + end + end + + describe 'DELETE destroy' do + let(:member) { create(:project_member, :developer, project: project) } + + before { sign_in(user) } + + context 'when member is not found' do + it 'returns 404' do + delete :destroy, namespace_id: project.namespace, + project_id: project, + id: 42 + + expect(response).to have_http_status(404) + end + end + + context 'when member is found' do + context 'when user does not have enough rights' do + before { project.team << [user, :developer] } + + it 'returns 404' do + delete :destroy, namespace_id: project.namespace, + project_id: project, + id: member + + expect(response).to have_http_status(404) + expect(project.members).to include member + end + end + + context 'when user has enough rights' do + before { project.team << [user, :master] } + + it '[HTML] removes user from members' do + delete :destroy, namespace_id: project.namespace, + project_id: project, + id: member + + expect(response).to redirect_to( + namespace_project_project_members_path(project.namespace, project) + ) + expect(project.members).not_to include member + end + + it '[JS] removes user from members' do + xhr :delete, :destroy, namespace_id: project.namespace, + project_id: project, + id: member + + expect(response).to be_success + expect(project.members).not_to include member + end + end + end + end + + describe 'DELETE leave' do + before { sign_in(user) } + + context 'when member is not found' do + it 'returns 404' do + delete :leave, namespace_id: project.namespace, + project_id: project + + expect(response).to have_http_status(404) + end + end + + context 'when member is found' do + context 'and is not an owner' do + before { project.team << [user, :developer] } + + it 'removes user from members' do + delete :leave, namespace_id: project.namespace, + project_id: project + + expect(response).to set_flash.to "You left the \"#{project.human_name}\" project." + expect(response).to redirect_to(dashboard_projects_path) + expect(project.users).not_to include user + end + end + + context 'and is an owner' do + let(:project) { create(:project, namespace: user.namespace) } + + before { project.team << [user, :master] } + + it 'cannot remove himself from the project' do + delete :leave, namespace_id: project.namespace, + project_id: project + + expect(response).to have_http_status(403) + end + end + + context 'and is a requester' do + before { project.request_access(user) } + + it 'removes user from members' do + delete :leave, namespace_id: project.namespace, + project_id: project + + expect(response).to set_flash.to 'Your access request to the project has been withdrawn.' + expect(response).to redirect_to(namespace_project_path(project.namespace, project)) + expect(project.requesters).to be_empty + expect(project.users).not_to include user + end + end + end + end + + describe 'POST request_access' do + before { sign_in(user) } + + it 'creates a new ProjectMember that is not a team member' do + post :request_access, namespace_id: project.namespace, + project_id: project + + expect(response).to set_flash.to 'Your request for access has been queued for review.' + expect(response).to redirect_to( + namespace_project_path(project.namespace, project) + ) + expect(project.requesters.exists?(user_id: user)).to be_truthy + expect(project.users).not_to include user + end + end + + describe 'POST approve' do + let(:member) { create(:project_member, :access_request, project: project) } + + before { sign_in(user) } + + context 'when member is not found' do + it 'returns 404' do + post :approve_access_request, namespace_id: project.namespace, + project_id: project, + id: 42 + + expect(response).to have_http_status(404) + end + end + + context 'when member is found' do + context 'when user does not have enough rights' do + before { project.team << [user, :developer] } + + it 'returns 404' do + post :approve_access_request, namespace_id: project.namespace, + project_id: project, + id: member + + expect(response).to have_http_status(404) + expect(project.members).not_to include member + end + end + + context 'when user has enough rights' do + before { project.team << [user, :master] } + + it 'adds user to members' do + post :approve_access_request, namespace_id: project.namespace, + project_id: project, + id: member + + expect(response).to redirect_to( + namespace_project_project_members_path(project.namespace, project) + ) + expect(project.members).to include member + end + end + end + end + + describe 'POST apply_import' do let(:another_project) { create(:project, :private) } - let(:user) { create(:user) } let(:member) { create(:user) } before do @@ -46,229 +228,4 @@ describe Projects::ProjectMembersController do end end end - - describe '#index' do - context 'when user is member' do - before do - project = create(:project, :private) - member = create(:user) - project.team << [member, :guest] - sign_in(member) - - get :index, namespace_id: project.namespace, project_id: project - end - - it { expect(response).to have_http_status(200) } - end - end - - describe '#destroy' do - let(:project) { create(:project, :public) } - - context 'when member is not found' do - it 'returns 404' do - delete :destroy, namespace_id: project.namespace, - project_id: project, - id: 42 - - expect(response).to have_http_status(404) - end - end - - context 'when member is found' do - let(:user) { create(:user) } - let(:team_user) { create(:user) } - let(:member) do - project.team << [team_user, :developer] - project.members.find_by(user_id: team_user.id) - end - - context 'when user does not have enough rights' do - before do - project.team << [user, :developer] - sign_in(user) - end - - it 'returns 404' do - delete :destroy, namespace_id: project.namespace, - project_id: project, - id: member - - expect(response).to have_http_status(404) - expect(project.users).to include team_user - end - end - - context 'when user has enough rights' do - before do - project.team << [user, :master] - sign_in(user) - end - - it '[HTML] removes user from members' do - delete :destroy, namespace_id: project.namespace, - project_id: project, - id: member - - expect(response).to redirect_to( - namespace_project_project_members_path(project.namespace, project) - ) - expect(project.users).not_to include team_user - end - - it '[JS] removes user from members' do - xhr :delete, :destroy, namespace_id: project.namespace, - project_id: project, - id: member - - expect(response).to be_success - expect(project.users).not_to include team_user - end - end - end - end - - describe '#leave' do - let(:project) { create(:project, :public) } - let(:user) { create(:user) } - - context 'when member is not found' do - before { sign_in(user) } - - it 'returns 404' do - delete :leave, namespace_id: project.namespace, - project_id: project - - expect(response).to have_http_status(404) - end - end - - context 'when member is found' do - context 'and is not an owner' do - before do - project.team << [user, :developer] - sign_in(user) - end - - it 'removes user from members' do - delete :leave, namespace_id: project.namespace, - project_id: project - - expect(response).to set_flash.to "You left the \"#{project.human_name}\" project." - expect(response).to redirect_to(dashboard_projects_path) - expect(project.users).not_to include user - end - end - - context 'and is an owner' do - before do - project.update(namespace_id: user.namespace_id) - project.team << [user, :master, user] - sign_in(user) - end - - it 'cannot remove himself from the project' do - delete :leave, namespace_id: project.namespace, - project_id: project - - expect(response).to have_http_status(403) - end - end - - context 'and is a requester' do - before do - project.request_access(user) - sign_in(user) - end - - it 'removes user from members' do - delete :leave, namespace_id: project.namespace, - project_id: project - - expect(response).to set_flash.to 'Your access request to the project has been withdrawn.' - expect(response).to redirect_to(namespace_project_path(project.namespace, project)) - expect(project.requesters).to be_empty - expect(project.users).not_to include user - end - end - end - end - - describe '#request_access' do - let(:project) { create(:project, :public) } - let(:user) { create(:user) } - - before do - sign_in(user) - end - - it 'creates a new ProjectMember that is not a team member' do - post :request_access, namespace_id: project.namespace, - project_id: project - - expect(response).to set_flash.to 'Your request for access has been queued for review.' - expect(response).to redirect_to( - namespace_project_path(project.namespace, project) - ) - expect(project.requesters.exists?(user_id: user)).to be_truthy - expect(project.users).not_to include user - end - end - - describe '#approve' do - let(:project) { create(:project, :public) } - - context 'when member is not found' do - it 'returns 404' do - post :approve_access_request, namespace_id: project.namespace, - project_id: project, - id: 42 - - expect(response).to have_http_status(404) - end - end - - context 'when member is found' do - let(:user) { create(:user) } - let(:team_requester) { create(:user) } - let(:member) do - project.request_access(team_requester) - project.requesters.find_by(user_id: team_requester.id) - end - - context 'when user does not have enough rights' do - before do - project.team << [user, :developer] - sign_in(user) - end - - it 'returns 404' do - post :approve_access_request, namespace_id: project.namespace, - project_id: project, - id: member - - expect(response).to have_http_status(404) - expect(project.users).not_to include team_requester - end - end - - context 'when user has enough rights' do - before do - project.team << [user, :master] - sign_in(user) - end - - it 'adds user to members' do - post :approve_access_request, namespace_id: project.namespace, - project_id: project, - id: member - - expect(response).to redirect_to( - namespace_project_project_members_path(project.namespace, project) - ) - expect(project.users).to include team_requester - end - end - end - end end diff --git a/spec/factories/group_members.rb b/spec/factories/group_members.rb index 795df5dfda9..080b2e75ea1 100644 --- a/spec/factories/group_members.rb +++ b/spec/factories/group_members.rb @@ -9,5 +9,6 @@ FactoryGirl.define do trait(:developer) { access_level GroupMember::DEVELOPER } trait(:master) { access_level GroupMember::MASTER } trait(:owner) { access_level GroupMember::OWNER } + trait(:access_request) { requested_at Time.now } end end diff --git a/spec/factories/label_priorities.rb b/spec/factories/label_priorities.rb new file mode 100644 index 00000000000..f25939d2d3e --- /dev/null +++ b/spec/factories/label_priorities.rb @@ -0,0 +1,7 @@ +FactoryGirl.define do + factory :label_priority do + project factory: :empty_project + label + sequence(:priority) + end +end diff --git a/spec/factories/labels.rb b/spec/factories/labels.rb index eb489099854..3e8822faf97 100644 --- a/spec/factories/labels.rb +++ b/spec/factories/labels.rb @@ -1,7 +1,23 @@ FactoryGirl.define do - factory :label do + factory :label, class: ProjectLabel do sequence(:title) { |n| "label#{n}" } color "#990000" project + + transient do + priority nil + end + + after(:create) do |label, evaluator| + if evaluator.priority + label.priorities.create(project: label.project, priority: evaluator.priority) + end + end + end + + factory :group_label, class: GroupLabel do + sequence(:title) { |n| "label#{n}" } + color "#990000" + group end end diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index c6a08d78b78..f780e01253c 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -68,5 +68,15 @@ FactoryGirl.define do factory :closed_merge_request, traits: [:closed] factory :reopened_merge_request, traits: [:reopened] factory :merge_request_with_diffs, traits: [:with_diffs] + + factory :labeled_merge_request do + transient do + labels [] + end + + after(:create) do |merge_request, evaluator| + merge_request.update_attributes(labels: evaluator.labels) + end + end end end diff --git a/spec/factories/project_members.rb b/spec/factories/project_members.rb index 1ddb305a8af..c21927640d1 100644 --- a/spec/factories/project_members.rb +++ b/spec/factories/project_members.rb @@ -8,5 +8,6 @@ FactoryGirl.define do trait(:reporter) { access_level ProjectMember::REPORTER } trait(:developer) { access_level ProjectMember::DEVELOPER } trait(:master) { access_level ProjectMember::MASTER } + trait(:access_request) { requested_at Time.now } end end diff --git a/spec/features/environments_spec.rb b/spec/features/environments_spec.rb index 68ea4eeae31..b565586ee14 100644 --- a/spec/features/environments_spec.rb +++ b/spec/features/environments_spec.rb @@ -19,10 +19,22 @@ feature 'Environments', feature: true do visit namespace_project_environments_path(project.namespace, project) end + context 'shows two tabs' do + scenario 'shows "Available" and "Stopped" tab with links' do + expect(page).to have_link('Available') + expect(page).to have_link('Stopped') + end + end + context 'without environments' do scenario 'does show no environments' do expect(page).to have_content('You don\'t have any environments right now.') end + + scenario 'does show 0 as counter for environments in both tabs' do + expect(page.find('.js-available-environments-count').text).to eq('0') + expect(page.find('.js-stopped-environments-count').text).to eq('0') + end end context 'with environments' do @@ -32,6 +44,11 @@ feature 'Environments', feature: true do expect(page).to have_link(environment.name) end + scenario 'does show number of available and stopped environments' do + expect(page.find('.js-available-environments-count').text).to eq('1') + expect(page.find('.js-stopped-environments-count').text).to eq('0') + end + context 'without deployments' do scenario 'does show no deployments' do expect(page).to have_content('No deployments yet') @@ -44,7 +61,7 @@ feature 'Environments', feature: true do scenario 'does show deployment SHA' do expect(page).to have_link(deployment.short_sha) end - + scenario 'does show deployment internal id' do expect(page).to have_content(deployment.iid) end @@ -65,20 +82,51 @@ feature 'Environments', feature: true do expect(page).to have_content(manual.name) expect(manual.reload).to be_pending end - + scenario 'does show build name and id' do expect(page).to have_link("#{build.name} (##{build.id})") end - + + scenario 'does not show stop button' do + expect(page).not_to have_selector('.stop-env-link') + end + + scenario 'does not show external link button' do + expect(page).not_to have_css('external-url') + end + context 'with external_url' do given(:environment) { create(:environment, project: project, external_url: 'https://git.gitlab.com') } given(:build) { create(:ci_build, pipeline: pipeline) } given(:deployment) { create(:deployment, environment: environment, deployable: build) } - + scenario 'does show an external link button' do expect(page).to have_link(nil, href: environment.external_url) end end + + context 'with stop action' do + given(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') } + given(:deployment) { create(:deployment, environment: environment, deployable: build, on_stop: 'close_app') } + + scenario 'does show stop button' do + expect(page).to have_selector('.stop-env-link') + end + + scenario 'starts build when stop button clicked' do + first('.stop-env-link').click + + expect(page).to have_content('close_app') + end + + context 'for reporter' do + let(:role) { :reporter } + + scenario 'does not show stop button' do + expect(page).not_to have_selector('.stop-env-link') + end + end + end end end end @@ -127,6 +175,10 @@ feature 'Environments', feature: true do expect(page).to have_link('Re-deploy') end + scenario 'does not show stop button' do + expect(page).not_to have_link('Stop') + end + context 'with manual action' do given(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'deploy to production') } @@ -140,16 +192,39 @@ feature 'Environments', feature: true do expect(page).to have_content(manual.name) expect(manual.reload).to be_pending end - + context 'with external_url' do given(:environment) { create(:environment, project: project, external_url: 'https://git.gitlab.com') } given(:build) { create(:ci_build, pipeline: pipeline) } given(:deployment) { create(:deployment, environment: environment, deployable: build) } - + scenario 'does show an external link button' do expect(page).to have_link(nil, href: environment.external_url) end end + + context 'with stop action' do + given(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') } + given(:deployment) { create(:deployment, environment: environment, deployable: build, on_stop: 'close_app') } + + scenario 'does show stop button' do + expect(page).to have_link('Stop') + end + + scenario 'does allow to stop environment' do + click_link('Stop') + + expect(page).to have_content('close_app') + end + + context 'for reporter' do + let(:role) { :reporter } + + scenario 'does not show stop button' do + expect(page).not_to have_link('Stop') + end + end + end end end end @@ -196,29 +271,4 @@ feature 'Environments', feature: true do end end end - - describe 'when deleting existing environment' do - given(:environment) { create(:environment, project: project) } - - before do - visit namespace_project_environment_path(project.namespace, project, environment) - end - - context 'when logged as master' do - given(:role) { :master } - - scenario 'does delete environment' do - click_link 'Destroy' - expect(page).not_to have_link(environment.name) - end - end - - context 'when logged as developer' do - given(:role) { :developer } - - scenario 'does not have a Destroy link' do - expect(page).not_to have_link('Destroy') - end - end - end end diff --git a/spec/features/issues/award_emoji_spec.rb b/spec/features/issues/award_emoji_spec.rb index 79cc50bc18e..ef00f209998 100644 --- a/spec/features/issues/award_emoji_spec.rb +++ b/spec/features/issues/award_emoji_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' describe 'Awards Emoji', feature: true do + include WaitForAjax + let!(:project) { create(:project) } let!(:user) { create(:user) } @@ -16,20 +18,22 @@ describe 'Awards Emoji', feature: true do project: project) end + let!(:note) { create(:note_on_issue, noteable: issue, project: issue.project, note: "Hello world") } + before do visit namespace_project_issue_path(project.namespace, project, issue) end it 'increments the thumbsdown emoji', js: true do find('[data-emoji="thumbsdown"]').click - sleep 2 + wait_for_ajax expect(thumbsdown_emoji).to have_text("1") end context 'click the thumbsup emoji' do it 'increments the thumbsup emoji', js: true do find('[data-emoji="thumbsup"]').click - sleep 2 + wait_for_ajax expect(thumbsup_emoji).to have_text("1") end @@ -41,7 +45,7 @@ describe 'Awards Emoji', feature: true do context 'click the thumbsdown emoji' do it 'increments the thumbsdown emoji', js: true do find('[data-emoji="thumbsdown"]').click - sleep 2 + wait_for_ajax expect(thumbsdown_emoji).to have_text("1") end @@ -49,13 +53,45 @@ describe 'Awards Emoji', feature: true do expect(thumbsup_emoji).to have_text("0") end end + + it 'toggles the smiley emoji on a note', js: true do + toggle_smiley_emoji(true) + + within('.note-awards') do + expect(find(emoji_counter)).to have_text("1") + end + + toggle_smiley_emoji(false) + + within('.note-awards') do + expect(page).not_to have_selector(emoji_counter) + end + end end def thumbsup_emoji - page.all('span.js-counter').first + page.all(emoji_counter).first end def thumbsdown_emoji - page.all('span.js-counter').last + page.all(emoji_counter).last + end + + def emoji_counter + 'span.js-counter' + end + + def toggle_smiley_emoji(status) + within('.note') do + find('.note-emoji-button').click + end + + unless status + first('[data-emoji="smiley"]').click + else + find('[data-emoji="smiley"]').click + end + + wait_for_ajax end end diff --git a/spec/features/issues/reset_filters_spec.rb b/spec/features/issues/reset_filters_spec.rb index f4d0f13c3d5..c9a3ecf16ea 100644 --- a/spec/features/issues/reset_filters_spec.rb +++ b/spec/features/issues/reset_filters_spec.rb @@ -75,6 +75,14 @@ feature 'Issues filter reset button', feature: true, js: true do end end + context 'when no filters have been applied' do + it 'the reset link should not be visible' do + visit_issues(project) + expect(page).to have_css('.issue', count: 2) + expect(page).not_to have_css '.reset_filters' + end + end + def reset_filters find('.reset-filters').click end diff --git a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb index bc2b0ff3e2c..c3c3ab33872 100644 --- a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb +++ b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb @@ -101,7 +101,7 @@ feature 'Merge When Build Succeeds', feature: true, js: true do expect(page).not_to have_link "Merge When Build Succeeds" end end - + def visit_merge_request(merge_request) visit namespace_project_merge_request_path(merge_request.project.namespace, merge_request.project, merge_request) end diff --git a/spec/features/merge_requests/widget_deployments_spec.rb b/spec/features/merge_requests/widget_deployments_spec.rb index 8e23ec50d4a..6676821b807 100644 --- a/spec/features/merge_requests/widget_deployments_spec.rb +++ b/spec/features/merge_requests/widget_deployments_spec.rb @@ -4,23 +4,58 @@ feature 'Widget Deployments Header', feature: true, js: true do include WaitForAjax describe 'when deployed to an environment' do - let(:project) { merge_request.target_project } - let(:merge_request) { create(:merge_request, :merged) } - let(:environment) { create(:environment, project: project) } - let!(:deployment) do - create(:deployment, environment: environment, sha: project.commit('master').id) - end + given(:user) { create(:user) } + given(:project) { merge_request.target_project } + given(:merge_request) { create(:merge_request, :merged) } + given(:environment) { create(:environment, project: project) } + given(:role) { :developer } + given(:sha) { project.commit('master').id } + given!(:deployment) { create(:deployment, environment: environment, sha: sha) } + given!(:manual) { } - before do - login_as :admin + background do + login_as(user) + project.team << [user, role] visit namespace_project_merge_request_path(project.namespace, project, merge_request) end - it 'displays that the environment is deployed' do + scenario 'displays that the environment is deployed' do wait_for_ajax expect(page).to have_content("Deployed to #{environment.name}") expect(find('.ci_widget > span > span')['data-title']).to eq(deployment.created_at.to_time.in_time_zone.to_s(:medium)) end + + context 'with stop action' do + given(:pipeline) { create(:ci_pipeline, project: project) } + given(:build) { create(:ci_build, pipeline: pipeline) } + given(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') } + given(:deployment) do + create(:deployment, environment: environment, ref: merge_request.target_branch, + sha: sha, deployable: build, on_stop: 'close_app') + end + + background do + wait_for_ajax + end + + scenario 'does show stop button' do + expect(page).to have_link('Stop environment') + end + + scenario 'does start build when stop button clicked' do + click_link('Stop environment') + + expect(page).to have_content('close_app') + end + + context 'for reporter' do + given(:role) { :reporter } + + scenario 'does not show stop button' do + expect(page).not_to have_link('Stop environment') + end + end + end end end diff --git a/spec/features/projects/import_export/test_project_export.tar.gz b/spec/features/projects/import_export/test_project_export.tar.gz index d04bdea0fe4..bfe59bdb90e 100644 Binary files a/spec/features/projects/import_export/test_project_export.tar.gz and b/spec/features/projects/import_export/test_project_export.tar.gz differ diff --git a/spec/features/projects/labels/update_prioritization_spec.rb b/spec/features/projects/labels/update_prioritization_spec.rb index cb7495da8eb..c9fa8315e79 100644 --- a/spec/features/projects/labels/update_prioritization_spec.rb +++ b/spec/features/projects/labels/update_prioritization_spec.rb @@ -3,18 +3,56 @@ require 'spec_helper' feature 'Prioritize labels', feature: true do include WaitForAjax - context 'when project belongs to user' do - let(:user) { create(:user) } - let(:project) { create(:project, name: 'test', namespace: user.namespace) } + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:project) { create(:empty_project, :public, namespace: group) } + let!(:bug) { create(:label, project: project, title: 'bug') } + let!(:wontfix) { create(:label, project: project, title: 'wontfix') } + let!(:feature) { create(:group_label, group: group, title: 'feature') } - scenario 'user can prioritize a label', js: true do - bug = create(:label, title: 'bug') - wontfix = create(:label, title: 'wontfix') - - project.labels << bug - project.labels << wontfix + context 'when user belongs to project team' do + before do + project.team << [user, :developer] login_as user + end + + scenario 'user can prioritize a group label', js: true do + visit namespace_project_labels_path(project.namespace, project) + + expect(page).to have_content('No prioritized labels yet') + + page.within('.other-labels') do + all('.js-toggle-priority')[1].click + wait_for_ajax + expect(page).not_to have_content('feature') + end + + page.within('.prioritized-labels') do + expect(page).not_to have_content('No prioritized labels yet') + expect(page).to have_content('feature') + end + end + + scenario 'user can unprioritize a group label', js: true do + create(:label_priority, project: project, label: feature, priority: 1) + + visit namespace_project_labels_path(project.namespace, project) + + page.within('.prioritized-labels') do + expect(page).to have_content('feature') + + first('.js-toggle-priority').click + wait_for_ajax + expect(page).not_to have_content('bug') + end + + page.within('.other-labels') do + expect(page).to have_content('feature') + end + end + + scenario 'user can prioritize a project label', js: true do visit namespace_project_labels_path(project.namespace, project) expect(page).to have_content('No prioritized labels yet') @@ -31,19 +69,14 @@ feature 'Prioritize labels', feature: true do end end - scenario 'user can unprioritize a label', js: true do - bug = create(:label, title: 'bug', priority: 1) - wontfix = create(:label, title: 'wontfix') + scenario 'user can unprioritize a project label', js: true do + create(:label_priority, project: project, label: bug, priority: 1) - project.labels << bug - project.labels << wontfix - - login_as user visit namespace_project_labels_path(project.namespace, project) - expect(page).to have_content('bug') - page.within('.prioritized-labels') do + expect(page).to have_content('bug') + first('.js-toggle-priority').click wait_for_ajax expect(page).not_to have_content('bug') @@ -56,23 +89,20 @@ feature 'Prioritize labels', feature: true do end scenario 'user can sort prioritized labels and persist across reloads', js: true do - bug = create(:label, title: 'bug', priority: 1) - wontfix = create(:label, title: 'wontfix', priority: 2) + create(:label_priority, project: project, label: bug, priority: 1) + create(:label_priority, project: project, label: feature, priority: 2) - project.labels << bug - project.labels << wontfix - - login_as user visit namespace_project_labels_path(project.namespace, project) expect(page).to have_content 'bug' + expect(page).to have_content 'feature' expect(page).to have_content 'wontfix' # Sort labels - find("#label_#{bug.id}").drag_to find("#label_#{wontfix.id}") + find("#project_label_#{bug.id}").drag_to find("#group_label_#{feature.id}") page.within('.prioritized-labels') do - expect(first('li')).to have_content('wontfix') + expect(first('li')).to have_content('feature') expect(page.all('li').last).to have_content('bug') end @@ -80,7 +110,7 @@ feature 'Prioritize labels', feature: true do wait_for_ajax page.within('.prioritized-labels') do - expect(first('li')).to have_content('wontfix') + expect(first('li')).to have_content('feature') expect(page.all('li').last).to have_content('bug') end end @@ -88,28 +118,26 @@ feature 'Prioritize labels', feature: true do context 'as a guest' do it 'does not prioritize labels' do - user = create(:user) guest = create(:user) - project = create(:project, name: 'test', namespace: user.namespace) - - create(:label, title: 'bug') login_as guest + visit namespace_project_labels_path(project.namespace, project) + expect(page).to have_content 'bug' + expect(page).to have_content 'wontfix' + expect(page).to have_content 'feature' expect(page).not_to have_css('.prioritized-labels') end end context 'as a non signed in user' do it 'does not prioritize labels' do - user = create(:user) - project = create(:project, name: 'test', namespace: user.namespace) - - create(:label, title: 'bug') - visit namespace_project_labels_path(project.namespace, project) + expect(page).to have_content 'bug' + expect(page).to have_content 'wontfix' + expect(page).to have_content 'feature' expect(page).not_to have_css('.prioritized-labels') end end diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb new file mode 100644 index 00000000000..27acc464ea2 --- /dev/null +++ b/spec/finders/labels_finder_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe LabelsFinder do + describe '#execute' do + let(:group_1) { create(:group) } + let(:group_2) { create(:group) } + let(:group_3) { create(:group) } + + let(:project_1) { create(:empty_project, namespace: group_1) } + let(:project_2) { create(:empty_project, namespace: group_2) } + let(:project_3) { create(:empty_project) } + let(:project_4) { create(:empty_project, :public) } + let(:project_5) { create(:empty_project, namespace: group_1) } + + let!(:project_label_1) { create(:label, project: project_1, title: 'Label 1') } + let!(:project_label_2) { create(:label, project: project_2, title: 'Label 2') } + let!(:project_label_4) { create(:label, project: project_4, title: 'Label 4') } + let!(:project_label_5) { create(:label, project: project_5, title: 'Label 5') } + + let!(:group_label_1) { create(:group_label, group: group_1, title: 'Label 1') } + let!(:group_label_2) { create(:group_label, group: group_1, title: 'Group Label 2') } + let!(:group_label_3) { create(:group_label, group: group_2, title: 'Group Label 3') } + + let(:user) { create(:user) } + + before do + create(:label, project: project_3, title: 'Label 3') + create(:group_label, group: group_3, title: 'Group Label 4') + + project_1.team << [user, :developer] + end + + context 'with no filter' do + it 'returns labels from projects the user have access' do + group_2.add_developer(user) + + finder = described_class.new(user) + + expect(finder.execute).to eq [group_label_2, group_label_3, project_label_1, group_label_1, project_label_2, project_label_4] + end + end + + context 'filtering by group_id' do + it 'returns labels available for any project within the group' do + group_1.add_developer(user) + + finder = described_class.new(user, group_id: group_1.id) + + expect(finder.execute).to eq [group_label_2, project_label_1, group_label_1, project_label_5] + end + end + + context 'filtering by project_id' do + it 'returns labels available for the project' do + finder = described_class.new(user, project_id: project_1.id) + + expect(finder.execute).to eq [group_label_2, project_label_1, group_label_1] + end + end + + context 'filtering by title' do + it 'returns label with that title' do + finder = described_class.new(user, title: 'Group Label 2') + + expect(finder.execute).to eq [group_label_2] + end + end + end +end diff --git a/spec/fixtures/api/schemas/list.json b/spec/fixtures/api/schemas/list.json index f070fa3b254..8d94cf26ecb 100644 --- a/spec/fixtures/api/schemas/list.json +++ b/spec/fixtures/api/schemas/list.json @@ -13,7 +13,7 @@ "enum": ["backlog", "label", "done"] }, "label": { - "type": ["object"], + "type": ["object", "null"], "required": [ "id", "color", diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb index 501f150cfda..d30daf47543 100644 --- a/spec/helpers/labels_helper_spec.rb +++ b/spec/helpers/labels_helper_spec.rb @@ -5,27 +5,26 @@ describe LabelsHelper do let(:project) { create(:empty_project) } let(:label) { create(:label, project: project) } - context 'with @project set' do - before do - @project = project - end - - it 'uses the instance variable' do - expect(link_to_label(label)).to match %r{} - end - end - - context 'without @project set' do + context 'without subject' do it "uses the label's project" do expect(link_to_label(label)).to match %r{.*} end end - context 'with a project argument' do - let(:another_project) { double('project', namespace: 'foo3', to_param: 'bar3') } + context 'with a project as subject' do + let(:namespace) { build(:namespace, name: 'foo3') } + let(:another_project) { build(:empty_project, namespace: namespace, name: 'bar3') } - it 'links to merge requests page' do - expect(link_to_label(label, project: another_project)).to match %r{.*} + it 'links to project issues page' do + expect(link_to_label(label, subject: another_project)).to match %r{.*} + end + end + + context 'with a group as subject' do + let(:group) { build(:group, name: 'bar') } + + it 'links to group issues page' do + expect(link_to_label(label, subject: group)).to match %r{.*} end end diff --git a/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb b/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb index 7116c09fb21..2f9343fadaf 100644 --- a/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb @@ -7,12 +7,7 @@ describe Banzai::Filter::ExternalIssueReferenceFilter, lib: true do IssuesHelper end - let(:project) { create(:jira_project) } - - context 'JIRA issue references' do - let(:issue) { ExternalIssue.new('JIRA-123', project) } - let(:reference) { issue.to_reference } - + shared_examples_for "external issue tracker" do it 'requires project context' do expect { described_class.call('') }.to raise_error(ArgumentError, /:project/) end @@ -20,6 +15,7 @@ describe Banzai::Filter::ExternalIssueReferenceFilter, lib: true do %w(pre code a style).each do |elem| it "ignores valid references contained inside '#{elem}' element" do exp = act = "<#{elem}>Issue #{reference}" + expect(filter(act).to_html).to eq exp end end @@ -33,25 +29,30 @@ describe Banzai::Filter::ExternalIssueReferenceFilter, lib: true do it 'links to a valid reference' do doc = filter("Issue #{reference}") + issue_id = doc.css('a').first.attr("data-external-issue") + expect(doc.css('a').first.attr('href')) - .to eq helper.url_for_issue(reference, project) + .to eq helper.url_for_issue(issue_id, project) end it 'links to the external tracker' do doc = filter("Issue #{reference}") - link = doc.css('a').first.attr('href') - expect(link).to eq "http://jira.example/browse/#{reference}" + link = doc.css('a').first.attr('href') + issue_id = doc.css('a').first.attr("data-external-issue") + + expect(link).to eq(helper.url_for_issue(issue_id, project)) end it 'links with adjacent text' do doc = filter("Issue (#{reference}.)") + expect(doc.to_html).to match(/\(#{reference}<\/a>\.\)/) end it 'includes a title attribute' do doc = filter("Issue #{reference}") - expect(doc.css('a').first.attr('title')).to eq "Issue in JIRA tracker" + expect(doc.css('a').first.attr('title')).to include("Issue in #{project.issues_tracker.title}") end it 'escapes the title attribute' do @@ -69,9 +70,60 @@ describe Banzai::Filter::ExternalIssueReferenceFilter, lib: true do it 'supports an :only_path context' do doc = filter("Issue #{reference}", only_path: true) - link = doc.css('a').first.attr('href') - expect(link).to eq helper.url_for_issue("#{reference}", project, only_path: true) + link = doc.css('a').first.attr('href') + issue_id = doc.css('a').first["data-external-issue"] + + expect(link).to eq helper.url_for_issue(issue_id, project, only_path: true) + end + + context 'with RequestStore enabled' do + let(:reference_filter) { HTML::Pipeline.new([described_class]) } + + before { allow(RequestStore).to receive(:active?).and_return(true) } + + it 'queries the collection on the first call' do + expect_any_instance_of(Project).to receive(:default_issues_tracker?).once.and_call_original + expect_any_instance_of(Project).to receive(:issue_reference_pattern).once.and_call_original + + not_cached = reference_filter.call("look for #{reference}", { project: project }) + + expect_any_instance_of(Project).not_to receive(:default_issues_tracker?) + expect_any_instance_of(Project).not_to receive(:issue_reference_pattern) + + cached = reference_filter.call("look for #{reference}", { project: project }) + + # Links must be the same + expect(cached[:output].css('a').first[:href]).to eq(not_cached[:output].css('a').first[:href]) + end + end + end + + context "redmine project" do + let(:project) { create(:redmine_project) } + let(:issue) { ExternalIssue.new("#123", project) } + let(:reference) { issue.to_reference } + + it_behaves_like "external issue tracker" + end + + context "jira project" do + let(:project) { create(:jira_project) } + let(:reference) { issue.to_reference } + + context "with right markdown" do + let(:issue) { ExternalIssue.new("JIRA-123", project) } + + it_behaves_like "external issue tracker" + end + + context "with wrong markdown" do + let(:issue) { ExternalIssue.new("#123", project) } + + it "ignores reference" do + exp = act = "Issue #{reference}" + expect(filter(act).to_html).to eq exp + end end end end diff --git a/spec/lib/banzai/filter/html_entity_filter_spec.rb b/spec/lib/banzai/filter/html_entity_filter_spec.rb index 4c68ce6d6e4..f9e6bd609f0 100644 --- a/spec/lib/banzai/filter/html_entity_filter_spec.rb +++ b/spec/lib/banzai/filter/html_entity_filter_spec.rb @@ -11,4 +11,9 @@ describe Banzai::Filter::HtmlEntityFilter, lib: true do expect(output).to eq(escaped) end + + it 'does not double-escape' do + escaped = ERB::Util.html_escape("Merge branch 'blabla' into 'master'") + expect(filter(escaped)).to eq(escaped) + end end diff --git a/spec/lib/banzai/filter/issue_reference_filter_spec.rb b/spec/lib/banzai/filter/issue_reference_filter_spec.rb index fce86a9b6ad..a2025672ad9 100644 --- a/spec/lib/banzai/filter/issue_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/issue_reference_filter_spec.rb @@ -25,9 +25,7 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do let(:reference) { issue.to_reference } it 'ignores valid references when using non-default tracker' do - expect_any_instance_of(described_class).to receive(:find_object). - with(project, issue.iid). - and_return(nil) + allow(project).to receive(:default_issues_tracker?).and_return(false) exp = act = "Issue #{reference}" expect(reference_filter(act).to_html).to eq exp @@ -199,19 +197,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do end end - context 'referencing external issues' do - let(:project) { create(:redmine_project) } - - it 'renders internal issue IDs as external issue links' do - doc = reference_filter('#1') - link = doc.css('a').first - - expect(link.attr('data-reference-type')).to eq('external_issue') - expect(link.attr('title')).to eq('Issue in Redmine') - expect(link.attr('data-external-issue')).to eq('1') - end - end - describe '#issues_per_Project' do context 'using an internal issue tracker' do it 'returns a Hash containing the issues per project' do diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb index 908ccebbf87..9c09f00ae8a 100644 --- a/spec/lib/banzai/filter/label_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/label_reference_filter_spec.rb @@ -305,6 +305,58 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do end end + describe 'group label references' do + let(:group) { create(:group) } + let(:project) { create(:empty_project, :public, namespace: group) } + let(:group_label) { create(:group_label, name: 'gfm references', group: group) } + + context 'without project reference' do + let(:reference) { group_label.to_reference(format: :name) } + + it 'links to a valid reference' do + doc = reference_filter("See #{reference}", project: project) + + expect(doc.css('a').first.attr('href')).to eq urls. + namespace_project_issues_url(project.namespace, project, label_name: group_label.name) + expect(doc.text).to eq 'See gfm references' + end + + it 'links with adjacent text' do + doc = reference_filter("Label (#{reference}.)") + expect(doc.to_html).to match(%r(\(#{group_label.name}\.\))) + end + + it 'ignores invalid label names' do + exp = act = %(Label #{Label.reference_prefix}"#{group_label.name.reverse}") + + expect(reference_filter(act).to_html).to eq exp + end + end + + context 'with project reference' do + let(:reference) { project.to_reference + group_label.to_reference(format: :name) } + + it 'links to a valid reference' do + doc = reference_filter("See #{reference}", project: project) + + expect(doc.css('a').first.attr('href')).to eq urls. + namespace_project_issues_url(project.namespace, project, label_name: group_label.name) + expect(doc.text).to eq 'See gfm references' + end + + it 'links with adjacent text' do + doc = reference_filter("Label (#{reference}.)") + expect(doc.to_html).to match(%r(\(#{group_label.name}\.\))) + end + + it 'ignores invalid label names' do + exp = act = %(Label #{project.to_reference}#{Label.reference_prefix}"#{group_label.name.reverse}") + + expect(reference_filter(act).to_html).to eq exp + end + end + end + describe 'cross project label references' do context 'valid project referenced' do let(:another_project) { create(:empty_project, :public) } @@ -339,4 +391,34 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do end end end + + describe 'cross group label references' do + context 'valid project referenced' do + let(:group) { create(:group) } + let(:project) { create(:empty_project, :public, namespace: group) } + let(:another_group) { create(:group) } + let(:another_project) { create(:empty_project, :public, namespace: another_group) } + let(:project_name) { another_project.name_with_namespace } + let(:group_label) { create(:group_label, group: another_group, color: '#00ff00') } + let(:reference) { another_project.to_reference + group_label.to_reference } + + let!(:result) { reference_filter("See #{reference}", project: project) } + + it 'points to referenced project issues page' do + expect(result.css('a').first.attr('href')) + .to eq urls.namespace_project_issues_url(another_project.namespace, + another_project, + label_name: group_label.name) + end + + it 'has valid color' do + expect(result.css('a span').first.attr('style')) + .to match /background-color: #00ff00/ + end + + it 'contains cross project content' do + expect(result.css('a').first.text).to eq "#{group_label.name} in #{project_name}" + end + end + end end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index 6dedd25e9d3..84f21631719 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -754,7 +754,7 @@ module Ci it 'does return production' do expect(builds.size).to eq(1) expect(builds.first[:environment]).to eq(environment) - expect(builds.first[:options]).to include(environment: { name: environment }) + expect(builds.first[:options]).to include(environment: { name: environment, action: "start" }) end end @@ -796,6 +796,52 @@ module Ci expect { builds }.to raise_error("jobs:deploy_to_production:environment name #{Gitlab::Regex.environment_name_regex_message}") end end + + context 'when on_stop is specified' do + let(:review) { { stage: 'deploy', script: 'test', environment: { name: 'review', on_stop: 'close_review' } } } + let(:config) { { review: review, close_review: close_review }.compact } + + context 'with matching job' do + let(:close_review) { { stage: 'deploy', script: 'test', environment: { name: 'review', action: 'stop' } } } + + it 'does return a list of builds' do + expect(builds.size).to eq(2) + expect(builds.first[:environment]).to eq('review') + end + end + + context 'without matching job' do + let(:close_review) { nil } + + it 'raises error' do + expect { builds }.to raise_error('review job: on_stop job close_review is not defined') + end + end + + context 'with close job without environment' do + let(:close_review) { { stage: 'deploy', script: 'test' } } + + it 'raises error' do + expect { builds }.to raise_error('review job: on_stop job close_review does not have environment defined') + end + end + + context 'with close job for different environment' do + let(:close_review) { { stage: 'deploy', script: 'test', environment: 'production' } } + + it 'raises error' do + expect { builds }.to raise_error('review job: on_stop job close_review have different environment name') + end + end + + context 'with close job without stop action' do + let(:close_review) { { stage: 'deploy', script: 'test', environment: { name: 'review' } } } + + it 'raises error' do + expect { builds }.to raise_error('review job: on_stop job close_review needs to have action stop defined') + end + end + end end describe "Dependencies" do diff --git a/spec/lib/gitlab/ci/config/node/environment_spec.rb b/spec/lib/gitlab/ci/config/node/environment_spec.rb index df453223da7..df925ff1afd 100644 --- a/spec/lib/gitlab/ci/config/node/environment_spec.rb +++ b/spec/lib/gitlab/ci/config/node/environment_spec.rb @@ -28,7 +28,7 @@ describe Gitlab::Ci::Config::Node::Environment do describe '#value' do it 'returns valid hash' do - expect(entry.value).to eq(name: 'production') + expect(entry.value).to include(name: 'production') end end @@ -87,6 +87,68 @@ describe Gitlab::Ci::Config::Node::Environment do end end + context 'when valid action is used' do + let(:config) do + { name: 'production', + action: 'start' } + end + + it 'is valid' do + expect(entry).to be_valid + end + end + + context 'when invalid action is used' do + let(:config) do + { name: 'production', + action: 'invalid' } + end + + describe '#valid?' do + it 'is not valid' do + expect(entry).not_to be_valid + end + end + + describe '#errors' do + it 'contains error about invalid action' do + expect(entry.errors) + .to include 'environment action should be start or stop' + end + end + end + + context 'when on_stop is used' do + let(:config) do + { name: 'production', + on_stop: 'close_app' } + end + + it 'is valid' do + expect(entry).to be_valid + end + end + + context 'when invalid on_stop is used' do + let(:config) do + { name: 'production', + on_stop: false } + end + + describe '#valid?' do + it 'is not valid' do + expect(entry).not_to be_valid + end + end + + describe '#errors' do + it 'contains error about invalid action' do + expect(entry.errors) + .to include 'environment on stop should be a string' + end + end + end + context 'when variables are used for environment' do let(:config) do { name: 'review/$CI_BUILD_REF_NAME', diff --git a/spec/lib/gitlab/gfm/reference_rewriter_spec.rb b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb index 0af249d8690..f045463c1cb 100644 --- a/spec/lib/gitlab/gfm/reference_rewriter_spec.rb +++ b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' describe Gitlab::Gfm::ReferenceRewriter do let(:text) { 'some text' } - let(:old_project) { create(:project) } - let(:new_project) { create(:project) } + let(:old_project) { create(:project, name: 'old') } + let(:new_project) { create(:project, name: 'new') } let(:user) { create(:user) } before { old_project.team << [user, :guest] } @@ -62,7 +62,7 @@ describe Gitlab::Gfm::ReferenceRewriter do it { is_expected.to eq "#{ref}, `#1`, #{ref}, `#1`" } end - context 'description with labels' do + context 'description with project labels' do let!(:label) { create(:label, id: 123, name: 'test', project: old_project) } let(:project_ref) { old_project.to_reference } @@ -76,6 +76,26 @@ describe Gitlab::Gfm::ReferenceRewriter do it { is_expected.to eq %Q{#{project_ref}#1 and #{project_ref}~123} } end end + + context 'description with group labels' do + let(:old_group) { create(:group) } + let!(:group_label) { create(:group_label, id: 321, name: 'group label', group: old_group) } + let(:project_ref) { old_project.to_reference } + + before do + old_project.update(namespace: old_group) + end + + context 'label referenced by id' do + let(:text) { '#1 and ~321' } + it { is_expected.to eq %Q{#{project_ref}#1 and #{project_ref}~321} } + end + + context 'label referenced by text' do + let(:text) { '#1 and ~"group label"' } + it { is_expected.to eq %Q{#{project_ref}#1 and #{project_ref}~321} } + end + end end context 'reference contains milestone' do diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index de68e32e5b4..a5aa387f4f7 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -185,6 +185,7 @@ describe Gitlab::GitAccess, lib: true do end end + # Run permission checks for a user def self.run_permission_checks(permissions_matrix) permissions_matrix.keys.each do |role| describe "#{role} access" do @@ -194,13 +195,12 @@ describe Gitlab::GitAccess, lib: true do else project.team << [user, role] end - end - permissions_matrix[role].each do |action, allowed| - context action do - subject { access.push_access_check(changes[action]) } - - it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey } + permissions_matrix[role].each do |action, allowed| + context action do + subject { access.push_access_check(changes[action]) } + it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey } + end end end end diff --git a/spec/lib/gitlab/github_import/importer_spec.rb b/spec/lib/gitlab/github_import/importer_spec.rb index 8854c8431b5..1af553f8f03 100644 --- a/spec/lib/gitlab/github_import/importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer_spec.rb @@ -157,7 +157,7 @@ describe Gitlab::GithubImport::Importer, lib: true do { type: :pull_request, url: "https://api.github.com/repos/octocat/Hello-World/pulls/1347", errors: "Validation failed: Validate branches Cannot Create: This merge request already exists: [\"New feature\"]" }, { type: :wiki, errors: "Gitlab::Shell::Error" }, { type: :release, url: 'https://api.github.com/repos/octocat/Hello-World/releases/2', errors: "Validation failed: Description can't be blank" } - ] + ] } described_class.new(project).execute diff --git a/spec/lib/gitlab/google_code_import/importer_spec.rb b/spec/lib/gitlab/google_code_import/importer_spec.rb index 54f85f8cffc..097861fd34d 100644 --- a/spec/lib/gitlab/google_code_import/importer_spec.rb +++ b/spec/lib/gitlab/google_code_import/importer_spec.rb @@ -15,6 +15,7 @@ describe Gitlab::GoogleCodeImport::Importer, lib: true do subject { described_class.new(project) } before do + project.team << [project.creator, :master] project.create_import_data(data: import_data) end @@ -31,9 +32,9 @@ describe Gitlab::GoogleCodeImport::Importer, lib: true do subject.execute %w( - Type-Defect Type-Enhancement Type-Task Type-Review Type-Other Milestone-0.12 Priority-Critical - Priority-High Priority-Medium Priority-Low OpSys-All OpSys-Windows OpSys-Linux OpSys-OSX Security - Performance Usability Maintainability Component-Panel Component-Taskbar Component-Battery + Type-Defect Type-Enhancement Type-Task Type-Review Type-Other Milestone-0.12 Priority-Critical + Priority-High Priority-Medium Priority-Low OpSys-All OpSys-Windows OpSys-Linux OpSys-OSX Security + Performance Usability Maintainability Component-Panel Component-Taskbar Component-Battery Component-Systray Component-Clock Component-Launcher Component-Tint2conf Component-Docs Component-New ).each do |label| label.sub!("-", ": ") diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 8fcbf12eab8..02b11bd999a 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -38,6 +38,7 @@ label: - label_links - issues - merge_requests +- priorities milestone: - project - issues @@ -186,3 +187,5 @@ project: award_emoji: - awardable - user +priorities: +- label \ No newline at end of file diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 98323fe6be4..ed9df468ced 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -2,6 +2,21 @@ "description": "Nisi et repellendus ut enim quo accusamus vel magnam.", "visibility_level": 10, "archived": false, + "labels": [ + { + "id": 2, + "title": "test2", + "color": "#428bca", + "project_id": 8, + "created_at": "2016-07-22T08:55:44.161Z", + "updated_at": "2016-07-22T08:55:44.161Z", + "template": false, + "description": "", + "type": "ProjectLabel", + "priorities": [ + ] + } + ], "issues": [ { "id": 40, @@ -64,7 +79,37 @@ "updated_at": "2016-07-22T08:55:44.161Z", "template": false, "description": "", - "priority": null + "type": "ProjectLabel" + } + }, + { + "id": 3, + "label_id": 3, + "target_id": 40, + "target_type": "Issue", + "created_at": "2016-07-22T08:57:02.841Z", + "updated_at": "2016-07-22T08:57:02.841Z", + "label": { + "id": 3, + "title": "test3", + "color": "#428bca", + "group_id": 8, + "created_at": "2016-07-22T08:55:44.161Z", + "updated_at": "2016-07-22T08:55:44.161Z", + "template": false, + "description": "", + "project_id": null, + "type": "GroupLabel", + "priorities": [ + { + "id": 1, + "project_id": 5, + "label_id": 1, + "priority": 1, + "created_at": "2016-10-18T09:35:43.338Z", + "updated_at": "2016-10-18T09:35:43.338Z" + } + ] } } ], @@ -536,7 +581,7 @@ "updated_at": "2016-07-22T08:55:44.161Z", "template": false, "description": "", - "priority": null + "type": "ProjectLabel" } } ], @@ -2226,9 +2271,6 @@ } ] } - ], - "labels": [ - ], "milestones": [ { diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 7582a732cdf..069ea960321 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -32,7 +32,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do it 'has the same label associated to two issues' do restored_project_json - expect(Label.first.issues.count).to eq(2) + expect(ProjectLabel.find_by_title('test2').issues.count).to eq(2) end it 'has milestones associated to two separate issues' do @@ -107,6 +107,41 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do expect(Label.first.label_links.first.target).not_to be_nil end + it 'has project labels' do + restored_project_json + + expect(ProjectLabel.count).to eq(2) + end + + it 'has no group labels' do + restored_project_json + + expect(GroupLabel.count).to eq(0) + end + + context 'with group' do + let!(:project) do + create(:empty_project, + name: 'project', + path: 'project', + builds_access_level: ProjectFeature::DISABLED, + issues_access_level: ProjectFeature::DISABLED, + group: create(:group)) + end + + it 'has group labels' do + restored_project_json + + expect(GroupLabel.count).to eq(1) + end + + it 'has label priorities' do + restored_project_json + + expect(GroupLabel.first.priorities).not_to be_empty + end + end + it 'has a project feature' do restored_project_json diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index cf8f2200c57..c8bba553558 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -111,6 +111,18 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do expect(saved_project_json['issues'].first['label_links'].first['label']).not_to be_empty end + it 'has project and group labels' do + label_types = saved_project_json['issues'].first['label_links'].map { |link| link['label']['type']} + + expect(label_types).to match_array(['ProjectLabel', 'GroupLabel']) + end + + it 'has priorities associated to labels' do + priorities = saved_project_json['issues'].first['label_links'].map { |link| link['label']['priorities']} + + expect(priorities.flatten).not_to be_empty + end + it 'saves the correct service type' do expect(saved_project_json['services'].first['type']).to eq('CustomIssueTrackerService') end @@ -135,15 +147,20 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do issue = create(:issue, assignee: user) snippet = create(:project_snippet) release = create(:release) + group = create(:group) project = create(:project, :public, issues: [issue], snippets: [snippet], - releases: [release] + releases: [release], + group: group ) - label = create(:label, project: project) - create(:label_link, label: label, target: issue) + project_label = create(:label, project: project) + group_label = create(:group_label, group: group) + create(:label_link, label: project_label, target: issue) + create(:label_link, label: group_label, target: issue) + create(:label_priority, label: group_label, priority: 1) milestone = create(:milestone, project: project) merge_request = create(:merge_request, source_project: project, milestone: milestone) commit_status = create(:commit_status, project: project) diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 8c8be66df9f..feee0f025d8 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -60,11 +60,13 @@ LabelLink: - target_type - created_at - updated_at -Label: +ProjectLabel: - id - title - color +- group_id - project_id +- type - created_at - updated_at - template @@ -329,3 +331,10 @@ AwardEmoji: - awardable_type - created_at - updated_at +LabelPriority: +- id +- project_id +- label_id +- priority +- created_at +- updated_at \ No newline at end of file diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 01a4a53a264..ca594a320c0 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -48,4 +48,50 @@ describe Deployment, models: true do end end end + + describe '#stop_action' do + let(:build) { create(:ci_build) } + + subject { deployment.stop_action } + + context 'when no other actions' do + let(:deployment) { FactoryGirl.build(:deployment, deployable: build) } + + it { is_expected.to be_nil } + end + + context 'with other actions' do + let!(:close_action) { create(:ci_build, pipeline: build.pipeline, name: 'close_app', when: :manual) } + + context 'when matching action is defined' do + let(:deployment) { FactoryGirl.build(:deployment, deployable: build, on_stop: 'close_other_app') } + + it { is_expected.to be_nil } + end + + context 'when no matching action is defined' do + let(:deployment) { FactoryGirl.build(:deployment, deployable: build, on_stop: 'close_app') } + + it { is_expected.to eq(close_action) } + end + end + end + + describe '#stoppable?' do + subject { deployment.stoppable? } + + context 'when no other actions' do + let(:deployment) { build(:deployment) } + + it { is_expected.to be_falsey } + end + + context 'when matching action is defined' do + let(:build) { create(:ci_build) } + let(:deployment) { FactoryGirl.build(:deployment, deployable: build, on_stop: 'close_app') } + let!(:close_action) { create(:ci_build, pipeline: build.pipeline, name: 'close_app', when: :manual) } + + it { is_expected.to be_truthy } + end + end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index e172ee8e590..a94e6d0165f 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -8,6 +8,8 @@ describe Environment, models: true do it { is_expected.to delegate_method(:last_deployment).to(:deployments).as(:last) } + it { is_expected.to delegate_method(:stop_action).to(:last_deployment) } + it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) } it { is_expected.to validate_length_of(:name).is_within(0..255) } @@ -96,4 +98,72 @@ describe Environment, models: true do is_expected.to be_nil end end + + describe '#stoppable?' do + subject { environment.stoppable? } + + context 'when no other actions' do + it { is_expected.to be_falsey } + end + + context 'when matching action is defined' do + let(:build) { create(:ci_build) } + let!(:deployment) { create(:deployment, environment: environment, deployable: build, on_stop: 'close_app') } + let!(:close_action) { create(:ci_build, pipeline: build.pipeline, name: 'close_app', when: :manual) } + + context 'when environment is available' do + before do + environment.start + end + + it { is_expected.to be_truthy } + end + + context 'when environment is stopped' do + before do + environment.stop + end + + it { is_expected.to be_falsey } + end + end + end + + describe '#stop!' do + let(:user) { create(:user) } + + subject { environment.stop!(user) } + + before do + expect(environment).to receive(:stoppable?).and_call_original + end + + context 'when no other actions' do + it { is_expected.to be_nil } + end + + context 'when matching action is defined' do + let(:build) { create(:ci_build) } + let!(:deployment) { create(:deployment, environment: environment, deployable: build, on_stop: 'close_app') } + + context 'when action did not yet finish' do + let!(:close_action) { create(:ci_build, :manual, pipeline: build.pipeline, name: 'close_app') } + + it 'returns the same action' do + expect(subject).to eq(close_action) + expect(subject.user).to eq(user) + end + end + + context 'if action did finish' do + let!(:close_action) { create(:ci_build, :manual, :success, pipeline: build.pipeline, name: 'close_app') } + + it 'returns a new action of the same type' do + is_expected.to be_persisted + expect(subject.name).to eq(close_action.name) + expect(subject.user).to eq(user) + end + end + end + end end diff --git a/spec/models/external_issue_spec.rb b/spec/models/external_issue_spec.rb index 4fc3b065592..ebba6e14578 100644 --- a/spec/models/external_issue_spec.rb +++ b/spec/models/external_issue_spec.rb @@ -10,21 +10,6 @@ describe ExternalIssue, models: true do it { is_expected.to include_module(Referable) } end - describe '.reference_pattern' do - it 'allows underscores in the project name' do - expect(ExternalIssue.reference_pattern.match('EXT_EXT-1234')[0]).to eq 'EXT_EXT-1234' - end - - it 'allows numbers in the project name' do - expect(ExternalIssue.reference_pattern.match('EXT3_EXT-1234')[0]).to eq 'EXT3_EXT-1234' - end - - it 'requires the project name to begin with A-Z' do - expect(ExternalIssue.reference_pattern.match('3EXT_EXT-1234')).to eq nil - expect(ExternalIssue.reference_pattern.match('EXT_EXT-1234')[0]).to eq 'EXT_EXT-1234' - end - end - describe '#to_reference' do it 'returns a String reference to the object' do expect(issue.to_reference).to eq issue.id diff --git a/spec/models/group_label_spec.rb b/spec/models/group_label_spec.rb new file mode 100644 index 00000000000..85eb889225b --- /dev/null +++ b/spec/models/group_label_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe GroupLabel, models: true do + describe 'relationships' do + it { is_expected.to belong_to(:group) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:group) } + end + + describe '#subject' do + it 'aliases group to subject' do + subject = described_class.new(group: build(:group)) + + expect(subject.subject).to be(subject.group) + end + end + + describe '#to_reference' do + let(:label) { create(:group_label) } + + context 'using id' do + it 'returns a String reference to the object' do + expect(label.to_reference).to eq "~#{label.id}" + end + end + + context 'using name' do + it 'returns a String reference to the object' do + expect(label.to_reference(format: :name)).to eq %(~"#{label.name}") + end + + it 'uses id when name contains double quote' do + label = create(:label, name: %q{"irony"}) + expect(label.to_reference(format: :name)).to eq "~#{label.id}" + end + end + + context 'using invalid format' do + it 'raises error' do + expect { label.to_reference(format: :invalid) } + .to raise_error StandardError, /Unknown format/ + end + end + end +end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 0b3ef9b98fd..ac862055ebc 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -12,6 +12,7 @@ describe Group, models: true do it { is_expected.to have_many(:project_group_links).dependent(:destroy) } it { is_expected.to have_many(:shared_projects).through(:project_group_links) } it { is_expected.to have_many(:notification_settings).dependent(:destroy) } + it { is_expected.to have_many(:labels).class_name('GroupLabel') } describe '#members & #requesters' do let(:requester) { create(:user) } diff --git a/spec/models/label_priority_spec.rb b/spec/models/label_priority_spec.rb new file mode 100644 index 00000000000..d18c2f7949a --- /dev/null +++ b/spec/models/label_priority_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe LabelPriority, models: true do + describe 'relationships' do + it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:label) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_presence_of(:label) } + it { is_expected.to validate_numericality_of(:priority).only_integer.is_greater_than_or_equal_to(0) } + + it 'validates uniqueness of label_id scoped to project_id' do + create(:label_priority) + + expect(subject).to validate_uniqueness_of(:label_id).scoped_to(:project_id) + end + end +end diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index 5a5d1a5d60c..0c163659a71 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -1,46 +1,42 @@ require 'spec_helper' describe Label, models: true do - let(:label) { create(:label) } - - describe 'associations' do - it { is_expected.to belong_to(:project) } - - it { is_expected.to have_many(:label_links).dependent(:destroy) } - it { is_expected.to have_many(:issues).through(:label_links).source(:target) } - it { is_expected.to have_many(:lists).dependent(:destroy) } + describe 'modules' do + it { is_expected.to include_module(Referable) } + it { is_expected.to include_module(Subscribable) } end - describe 'modules' do - subject { described_class } - - it { is_expected.to include_module(Referable) } + describe 'associations' do + it { is_expected.to have_many(:issues).through(:label_links).source(:target) } + it { is_expected.to have_many(:label_links).dependent(:destroy) } + it { is_expected.to have_many(:lists).dependent(:destroy) } + it { is_expected.to have_many(:priorities).class_name('LabelPriority') } end describe 'validation' do - it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_uniqueness_of(:title).scoped_to([:group_id, :project_id]) } it 'validates color code' do - expect(label).not_to allow_value('G-ITLAB').for(:color) - expect(label).not_to allow_value('AABBCC').for(:color) - expect(label).not_to allow_value('#AABBCCEE').for(:color) - expect(label).not_to allow_value('GGHHII').for(:color) - expect(label).not_to allow_value('#').for(:color) - expect(label).not_to allow_value('').for(:color) + is_expected.not_to allow_value('G-ITLAB').for(:color) + is_expected.not_to allow_value('AABBCC').for(:color) + is_expected.not_to allow_value('#AABBCCEE').for(:color) + is_expected.not_to allow_value('GGHHII').for(:color) + is_expected.not_to allow_value('#').for(:color) + is_expected.not_to allow_value('').for(:color) - expect(label).to allow_value('#AABBCC').for(:color) - expect(label).to allow_value('#abcdef').for(:color) + is_expected.to allow_value('#AABBCC').for(:color) + is_expected.to allow_value('#abcdef').for(:color) end it 'validates title' do - expect(label).not_to allow_value('G,ITLAB').for(:title) - expect(label).not_to allow_value('').for(:title) + is_expected.not_to allow_value('G,ITLAB').for(:title) + is_expected.not_to allow_value('').for(:title) - expect(label).to allow_value('GITLAB').for(:title) - expect(label).to allow_value('gitlab').for(:title) - expect(label).to allow_value('G?ITLAB').for(:title) - expect(label).to allow_value('G&ITLAB').for(:title) - expect(label).to allow_value("customer's request").for(:title) + is_expected.to allow_value('GITLAB').for(:title) + is_expected.to allow_value('gitlab').for(:title) + is_expected.to allow_value('G?ITLAB').for(:title) + is_expected.to allow_value('G&ITLAB').for(:title) + is_expected.to allow_value("customer's request").for(:title) end end @@ -51,45 +47,59 @@ describe Label, models: true do end end - describe '#to_reference' do - context 'using id' do - it 'returns a String reference to the object' do - expect(label.to_reference).to eq "~#{label.id}" - end - end + describe 'priorization' do + subject(:label) { create(:label) } - context 'using name' do - it 'returns a String reference to the object' do - expect(label.to_reference(format: :name)).to eq %(~"#{label.name}") - end + let(:project) { label.project } - it 'uses id when name contains double quote' do - label = create(:label, name: %q{"irony"}) - expect(label.to_reference(format: :name)).to eq "~#{label.id}" - end - end + describe '#prioritize!' do + context 'when label is not prioritized' do + it 'creates a label priority' do + expect { label.prioritize!(project, 1) }.to change(label.priorities, :count).by(1) + end - context 'using invalid format' do - it 'raises error' do - expect { label.to_reference(format: :invalid) } - .to raise_error StandardError, /Unknown format/ - end - end + it 'sets label priority' do + label.prioritize!(project, 1) - context 'cross project reference' do - let(:project) { create(:project) } - - context 'using name' do - it 'returns cross reference with label name' do - expect(label.to_reference(project, format: :name)) - .to eq %Q(#{label.project.to_reference}~"#{label.name}") + expect(label.priorities.first.priority).to eq 1 end end - context 'using id' do - it 'returns cross reference with label id' do - expect(label.to_reference(project, format: :id)) - .to eq %Q(#{label.project.to_reference}~#{label.id}) + context 'when label is prioritized' do + let!(:priority) { create(:label_priority, project: project, label: label, priority: 0) } + + it 'does not create a label priority' do + expect { label.prioritize!(project, 1) }.not_to change(label.priorities, :count) + end + + it 'updates label priority' do + label.prioritize!(project, 1) + + expect(priority.reload.priority).to eq 1 + end + end + end + + describe '#unprioritize!' do + it 'removes label priority' do + create(:label_priority, project: project, label: label, priority: 0) + + expect { label.unprioritize!(project) }.to change(label.priorities, :count).by(-1) + end + end + + describe '#priority' do + context 'when label is not prioritized' do + it 'returns nil' do + expect(label.priority(project)).to be_nil + end + end + + context 'when label is prioritized' do + it 'returns label priority' do + create(:label_priority, project: project, label: label, priority: 1) + + expect(label.priority(project)).to eq 1 end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 1acc8d748af..6db5e7f7d80 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -640,32 +640,56 @@ describe MergeRequest, models: true do end describe '#all_commits_sha' do - let(:all_commits_sha) do - subject.merge_request_diffs.flat_map(&:commits).map(&:sha).uniq - end + context 'when merge request is persisted' do + let(:all_commits_sha) do + subject.merge_request_diffs.flat_map(&:commits).map(&:sha).uniq + end - shared_examples 'returning all SHA' do - it 'returns all SHA from all merge_request_diffs' do - expect(subject.merge_request_diffs.size).to eq(2) - expect(subject.all_commits_sha).to eq(all_commits_sha) + shared_examples 'returning all SHA' do + it 'returns all SHA from all merge_request_diffs' do + expect(subject.merge_request_diffs.size).to eq(2) + expect(subject.all_commits_sha).to eq(all_commits_sha) + end + end + + context 'with a completely different branch' do + before do + subject.update(target_branch: 'v1.0.0') + end + + it_behaves_like 'returning all SHA' + end + + context 'with a branch having no difference' do + before do + subject.update(target_branch: 'v1.1.0') + subject.reload # make sure commits were not cached + end + + it_behaves_like 'returning all SHA' end end - context 'with a completely different branch' do - before do - subject.update(target_branch: 'v1.0.0') + context 'when merge request is not persisted' do + context 'when compare commits are set in the service' do + let(:commit) { spy('commit') } + + subject do + build(:merge_request, compare_commits: [commit, commit]) + end + + it 'returns commits from compare commits temporary data' do + expect(subject.all_commits_sha).to eq [commit, commit] + end end - it_behaves_like 'returning all SHA' - end + context 'when compare commits are not set in the service' do + subject { build(:merge_request) } - context 'with a branch having no difference' do - before do - subject.update(target_branch: 'v1.1.0') - subject.reload # make sure commits were not cached + it 'returns array with diff head sha element only' do + expect(subject.all_commits_sha).to eq [subject.diff_head_sha] + end end - - it_behaves_like 'returning all SHA' end end diff --git a/spec/models/project_label_spec.rb b/spec/models/project_label_spec.rb new file mode 100644 index 00000000000..18c9d449ee5 --- /dev/null +++ b/spec/models/project_label_spec.rb @@ -0,0 +1,120 @@ +require 'spec_helper' + +describe ProjectLabel, models: true do + describe 'relationships' do + it { is_expected.to belong_to(:project) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + + context 'validates if title must not exist at group level' do + let(:group) { create(:group, name: 'gitlab-org') } + let(:project) { create(:empty_project, group: group) } + + before do + create(:group_label, group: group, title: 'Bug') + end + + it 'returns error if title already exists at group level' do + label = described_class.new(project: project, title: 'Bug') + + label.valid? + + expect(label.errors[:title]).to include 'already exists at group level for gitlab-org. Please choose another one.' + end + + it 'does not returns error if title does not exist at group level' do + label = described_class.new(project: project, title: 'Security') + + label.valid? + + expect(label.errors[:title]).to be_empty + end + + it 'does not returns error if project does not belong to group' do + another_project = create(:empty_project) + label = described_class.new(project: another_project, title: 'Bug') + + label.valid? + + expect(label.errors[:title]).to be_empty + end + + it 'does not returns error when title does not change' do + project_label = create(:label, project: project, name: 'Security') + create(:group_label, group: group, name: 'Security') + project_label.description = 'Security related stuff.' + + project_label.valid? + + expect(project_label.errors[:title]).to be_empty + end + end + + context 'when attempting to add more than one priority to the project label' do + it 'returns error' do + subject.priorities.build + subject.priorities.build + + subject.valid? + + expect(subject.errors[:priorities]).to include 'Number of permitted priorities exceeded' + end + end + end + + describe '#subject' do + it 'aliases project to subject' do + subject = described_class.new(project: build(:empty_project)) + + expect(subject.subject).to be(subject.project) + end + end + + describe '#to_reference' do + let(:label) { create(:label) } + + context 'using id' do + it 'returns a String reference to the object' do + expect(label.to_reference).to eq "~#{label.id}" + end + end + + context 'using name' do + it 'returns a String reference to the object' do + expect(label.to_reference(format: :name)).to eq %(~"#{label.name}") + end + + it 'uses id when name contains double quote' do + label = create(:label, name: %q{"irony"}) + expect(label.to_reference(format: :name)).to eq "~#{label.id}" + end + end + + context 'using invalid format' do + it 'raises error' do + expect { label.to_reference(format: :invalid) } + .to raise_error StandardError, /Unknown format/ + end + end + + context 'cross project reference' do + let(:project) { create(:project) } + + context 'using name' do + it 'returns cross reference with label name' do + expect(label.to_reference(project, format: :name)) + .to eq %Q(#{label.project.to_reference}~"#{label.name}") + end + end + + context 'using id' do + it 'returns cross reference with label id' do + expect(label.to_reference(project, format: :id)) + .to eq %Q(#{label.project.to_reference}~#{label.id}) + end + end + end + end +end diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index b48a3176007..6ff32aea018 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -30,6 +30,15 @@ describe JiraService, models: true do end end + describe '#reference_pattern' do + it_behaves_like 'allows project key on reference pattern' + + it 'does not allow # on the code' do + expect(subject.reference_pattern.match('#123')).to be_nil + expect(subject.reference_pattern.match('1#23#12')).to be_nil + end + end + describe "Execute" do let(:user) { create(:user) } let(:project) { create(:project) } diff --git a/spec/models/project_services/redmine_service_spec.rb b/spec/models/project_services/redmine_service_spec.rb index b8679cd2563..0a7b237a051 100644 --- a/spec/models/project_services/redmine_service_spec.rb +++ b/spec/models/project_services/redmine_service_spec.rb @@ -26,4 +26,12 @@ describe RedmineService, models: true do it { is_expected.not_to validate_presence_of(:new_issue_url) } end end + + describe '#reference_pattern' do + it_behaves_like 'allows project key on reference pattern' + + it 'does allow # on the reference' do + expect(subject.reference_pattern.match('#123')[:issue]).to eq('123') + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 67dbcc362f6..e6d98e25d0b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -56,7 +56,7 @@ describe Project, models: true do it { is_expected.to have_many(:runners) } it { is_expected.to have_many(:variables) } it { is_expected.to have_many(:triggers) } - it { is_expected.to have_many(:labels).dependent(:destroy) } + it { is_expected.to have_many(:labels).class_name('ProjectLabel').dependent(:destroy) } it { is_expected.to have_many(:users_star_projects).dependent(:destroy) } it { is_expected.to have_many(:environments).dependent(:destroy) } it { is_expected.to have_many(:deployments).dependent(:destroy) } diff --git a/spec/requests/api/boards_spec.rb b/spec/requests/api/boards_spec.rb index f4b04445c6c..4f5c09a3029 100644 --- a/spec/requests/api/boards_spec.rb +++ b/spec/requests/api/boards_spec.rb @@ -106,9 +106,20 @@ describe API::API, api: true do describe "POST /projects/:id/board/lists" do let(:base_url) { "/projects/#{project.id}/boards/#{board.id}/lists" } - it 'creates a new issue board list' do - post api(base_url, user), - label_id: ux_label.id + it 'creates a new issue board list for group labels' do + group = create(:group) + group_label = create(:group_label, group: group) + project.update(group: group) + + post api(base_url, user), label_id: group_label.id + + expect(response).to have_http_status(201) + expect(json_response['label']['name']).to eq(group_label.title) + expect(json_response['position']).to eq(3) + end + + it 'creates a new issue board list for project labels' do + post api(base_url, user), label_id: ux_label.id expect(response).to have_http_status(201) expect(json_response['label']['name']).to eq(ux_label.title) @@ -116,15 +127,13 @@ describe API::API, api: true do end it 'returns 400 when creating a new list if label_id is invalid' do - post api(base_url, user), - label_id: 23423 + post api(base_url, user), label_id: 23423 expect(response).to have_http_status(400) end - it "returns 403 for project members with guest role" do - put api("#{base_url}/#{test_list.id}", guest), - position: 1 + it 'returns 403 for project members with guest role' do + put api("#{base_url}/#{test_list.id}", guest), position: 1 expect(response).to have_http_status(403) end diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index 7aa7e85a9e2..335efc4db6c 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -196,7 +196,7 @@ describe API::CommitStatuses, api: true do end context 'reporter user' do - before { post api(post_url, reporter) } + before { post api(post_url, reporter), state: 'running' } it 'does not create commit status' do expect(response).to have_http_status(403) @@ -204,7 +204,7 @@ describe API::CommitStatuses, api: true do end context 'guest user' do - before { post api(post_url, guest) } + before { post api(post_url, guest), state: 'running' } it 'does not create commit status' do expect(response).to have_http_status(403) diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 83789223019..1da9988978b 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -12,12 +12,18 @@ describe API::API, api: true do end describe 'GET /projects/:id/labels' do - it 'returns project labels' do + it 'returns all available labels to the project' do + group = create(:group) + group_label = create(:group_label, group: group) + project.update(group: group) + get api("/projects/#{project.id}/labels", user) + expect(response).to have_http_status(200) expect(json_response).to be_an Array - expect(json_response.size).to eq(1) - expect(json_response.first['name']).to eq(label1.name) + expect(json_response.size).to eq(2) + expect(json_response.first['name']).to eq(group_label.name) + expect(json_response.second['name']).to eq(label1.name) end end diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb index 1ce2658569e..f8a1aed5441 100644 --- a/spec/requests/api/system_hooks_spec.rb +++ b/spec/requests/api/system_hooks_spec.rb @@ -73,9 +73,10 @@ describe API::API, api: true do end.to change { SystemHook.count }.by(-1) end - it "returns success if hook id not found" do - delete api("/hooks/12345", admin) - expect(response).to have_http_status(200) + it 'returns 404 if the system hook does not exist' do + delete api('/hooks/12345', admin) + + expect(response).to have_http_status(404) end end end diff --git a/spec/services/boards/lists/create_service_spec.rb b/spec/services/boards/lists/create_service_spec.rb index e7806add916..a7e9efcf93f 100644 --- a/spec/services/boards/lists/create_service_spec.rb +++ b/spec/services/boards/lists/create_service_spec.rb @@ -9,6 +9,10 @@ describe Boards::Lists::CreateService, services: true do subject(:service) { described_class.new(project, user, label_id: label.id) } + before do + project.team << [user, :developer] + end + context 'when board lists is empty' do it 'creates a new list at beginning of the list' do list = service.execute(board) diff --git a/spec/services/boards/lists/generate_service_spec.rb b/spec/services/boards/lists/generate_service_spec.rb index 8b2f5e81338..ed0337662af 100644 --- a/spec/services/boards/lists/generate_service_spec.rb +++ b/spec/services/boards/lists/generate_service_spec.rb @@ -8,6 +8,10 @@ describe Boards::Lists::GenerateService, services: true do subject(:service) { described_class.new(project, user) } + before do + project.team << [user, :developer] + end + context 'when board lists is empty' do it 'creates the default lists' do expect { service.execute(board) }.to change(board.lists, :count).by(2) diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index 0b84c7262c3..cf0a18aacec 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -7,11 +7,13 @@ describe CreateDeploymentService, services: true do let(:service) { described_class.new(project, user, params) } describe '#execute' do + let(:options) { nil } let(:params) do { environment: 'production', ref: 'master', tag: false, sha: '97de212e80737a608d939f648d959671fb0a0142', + options: options } end @@ -28,7 +30,7 @@ describe CreateDeploymentService, services: true do end context 'when environment exist' do - before { create(:environment, project: project, name: 'production') } + let!(:environment) { create(:environment, project: project, name: 'production') } it 'does not create a new environment' do expect { subject }.not_to change { Environment.count } @@ -37,6 +39,46 @@ describe CreateDeploymentService, services: true do it 'does create a deployment' do expect(subject).to be_persisted end + + context 'and start action is defined' do + let(:options) { { action: 'start' } } + + context 'and environment is stopped' do + before do + environment.stop + end + + it 'makes environment available' do + subject + + expect(environment.reload).to be_available + end + + it 'does create a deployment' do + expect(subject).to be_persisted + end + end + end + + context 'and stop action is defined' do + let(:options) { { action: 'stop' } } + + context 'and environment is available' do + before do + environment.start + end + + it 'makes environment stopped' do + subject + + expect(environment.reload).to be_stopped + end + + it 'does not create a deployment' do + expect(subject).to be_nil + end + end + end end context 'for environment with invalid name' do @@ -53,7 +95,7 @@ describe CreateDeploymentService, services: true do end it 'does not create a deployment' do - expect(subject).not_to be_persisted + expect(subject).to be_nil end end @@ -83,6 +125,25 @@ describe CreateDeploymentService, services: true do it 'does create a new deployment' do expect(subject).to be_persisted end + + context 'and environment exist' do + let!(:environment) { create(:environment, project: project, name: 'review-apps/feature-review-apps') } + + it 'does not create a new environment' do + expect { subject }.not_to change { Environment.count } + end + + it 'updates external url' do + subject + + expect(subject.environment.name).to eq('review-apps/feature-review-apps') + expect(subject.environment.external_url).to eq('http://feature-review-apps.review-apps.gitlab.com') + end + + it 'does create a new deployment' do + expect(subject).to be_persisted + end + end end context 'when project was removed' do diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 8dda34c7a03..ad5170afc21 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -415,7 +415,7 @@ describe GitPushService, services: true do it "doesn't close issues when external issue tracker is in use" do allow_any_instance_of(Project).to receive(:default_issues_tracker?). and_return(false) - external_issue_tracker = double(title: 'My Tracker', issue_path: issue.iid) + external_issue_tracker = double(title: 'My Tracker', issue_path: issue.iid, reference_pattern: project.issue_reference_pattern) allow_any_instance_of(Project).to receive(:external_issue_tracker).and_return(external_issue_tracker) # The push still shouldn't create cross-reference notes. @@ -484,30 +484,46 @@ describe GitPushService, services: true do end context "closing an issue" do - let(:message) { "this is some work.\n\ncloses JIRA-1" } + let(:message) { "this is some work.\n\ncloses JIRA-1" } + let(:transition_body) { { transition: { id: '2' } }.to_json } + let(:comment_body) { { body: "Issue solved with [#{closing_commit.id}|http://localhost/#{project.path_with_namespace}/commit/#{closing_commit.id}]." }.to_json } - it "initiates one api call to jira server to close the issue" do - transition_body = { - transition: { - id: '2' - } - }.to_json + context "using right markdown" do + it "initiates one api call to jira server to close the issue" do + execute_service(project, commit_author, @oldrev, @newrev, @ref ) - execute_service(project, commit_author, @oldrev, @newrev, @ref ) - expect(WebMock).to have_requested(:post, jira_api_transition_url).with( - body: transition_body - ).once + expect(WebMock).to have_requested(:post, jira_api_transition_url).with( + body: transition_body + ).once + end + + it "initiates one api call to jira server to comment on the issue" do + execute_service(project, commit_author, @oldrev, @newrev, @ref ) + + expect(WebMock).to have_requested(:post, jira_api_comment_url).with( + body: comment_body + ).once + end end - it "initiates one api call to jira server to comment on the issue" do - comment_body = { - body: "Issue solved with [#{closing_commit.id}|http://localhost/#{project.path_with_namespace}/commit/#{closing_commit.id}]." - }.to_json + context "using wrong markdown" do + let(:message) { "this is some work.\n\ncloses #1" } - execute_service(project, commit_author, @oldrev, @newrev, @ref ) - expect(WebMock).to have_requested(:post, jira_api_comment_url).with( - body: comment_body - ).once + it "does not initiates one api call to jira server to close the issue" do + execute_service(project, commit_author, @oldrev, @newrev, @ref ) + + expect(WebMock).not_to have_requested(:post, jira_api_transition_url).with( + body: transition_body + ) + end + + it "does not initiates one api call to jira server to comment on the issue" do + execute_service(project, commit_author, @oldrev, @newrev, @ref ) + + expect(WebMock).not_to have_requested(:post, jira_api_comment_url).with( + body: comment_body + ).once + end end end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 1050502fa19..5c0331ebe66 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -67,6 +67,27 @@ describe Issues::CreateService, services: true do expect(Todo.where(attributes).count).to eq 1 end + context 'when label belongs to project group' do + let(:group) { create(:group) } + let(:group_labels) { create_pair(:group_label, group: group) } + + let(:opts) do + { + title: 'Title', + description: 'Description', + label_ids: group_labels.map(&:id) + } + end + + before do + project.update(group: group) + end + + it 'assigns group labels' do + expect(issue.labels).to match_array group_labels + end + end + context 'when label belongs to different project' do let(:label) { create(:label) } diff --git a/spec/services/labels/find_or_create_service_spec.rb b/spec/services/labels/find_or_create_service_spec.rb new file mode 100644 index 00000000000..cbfc63de811 --- /dev/null +++ b/spec/services/labels/find_or_create_service_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe Labels::FindOrCreateService, services: true do + describe '#execute' do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:project) { create(:project, namespace: group) } + + let(:params) do + { + title: 'Security', + description: 'Security related stuff.', + color: '#FF0000' + } + end + + subject(:service) { described_class.new(user, project, params) } + + before do + project.team << [user, :developer] + end + + context 'when label does not exist at group level' do + it 'creates a new label at project level' do + expect { service.execute }.to change(project.labels, :count).by(1) + end + end + + context 'when label exists at group level' do + it 'returns the group label' do + group_label = create(:group_label, group: group, title: 'Security') + + expect(service.execute).to eq group_label + end + end + + context 'when label does not exist at group level' do + it 'creates a new label at project leve' do + expect { service.execute }.to change(project.labels, :count).by(1) + end + end + + context 'when label exists at project level' do + it 'returns the project label' do + project_label = create(:label, project: project, title: 'Security') + + expect(service.execute).to eq project_label + end + end + end +end diff --git a/spec/services/labels/transfer_service_spec.rb b/spec/services/labels/transfer_service_spec.rb new file mode 100644 index 00000000000..ddf3527dc0f --- /dev/null +++ b/spec/services/labels/transfer_service_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +describe Labels::TransferService, services: true do + describe '#execute' do + let(:user) { create(:user) } + let(:group_1) { create(:group) } + let(:group_2) { create(:group) } + let(:group_3) { create(:group) } + let(:project_1) { create(:project, namespace: group_2) } + let(:project_2) { create(:project, namespace: group_3) } + + let(:group_label_1) { create(:group_label, group: group_1, name: 'Group Label 1') } + let(:group_label_2) { create(:group_label, group: group_1, name: 'Group Label 2') } + let(:group_label_3) { create(:group_label, group: group_1, name: 'Group Label 3') } + let(:group_label_4) { create(:group_label, group: group_2, name: 'Group Label 4') } + let(:group_label_5) { create(:group_label, group: group_3, name: 'Group Label 5') } + let(:project_label_1) { create(:label, project: project_1, name: 'Project Label 1') } + + subject(:service) { described_class.new(user, group_1, project_1) } + + before do + create(:labeled_issue, project: project_1, labels: [group_label_1]) + create(:labeled_issue, project: project_1, labels: [group_label_4]) + create(:labeled_issue, project: project_1, labels: [project_label_1]) + create(:labeled_issue, project: project_2, labels: [group_label_5]) + create(:labeled_merge_request, source_project: project_1, labels: [group_label_1, group_label_2]) + create(:labeled_merge_request, source_project: project_2, labels: [group_label_5]) + end + + it 'recreates the missing group labels at project level' do + expect { service.execute }.to change(project_1.labels, :count).by(2) + end + + it 'recreates label priorities related to the missing group labels' do + create(:label_priority, project: project_1, label: group_label_1, priority: 1) + + service.execute + + new_project_label = project_1.labels.find_by(title: group_label_1.title) + expect(new_project_label.id).not_to eq group_label_1.id + expect(new_project_label.priorities).not_to be_empty + end + + it 'does not recreate missing group labels that are not applied to issues or merge requests' do + service.execute + + expect(project_1.labels.where(title: group_label_3.title)).to be_empty + end + + it 'does not recreate missing group labels that already exist in the project group' do + service.execute + + expect(project_1.labels.where(title: group_label_4.title)).to be_empty + end + end +end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index ee53e110aee..f93d7732a9a 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -74,6 +74,18 @@ describe MergeRequests::MergeService, services: true do service.execute(merge_request) end + + context "wrong issue markdown" do + it 'does not close issues on JIRA issue tracker' do + jira_issue = ExternalIssue.new('#123', project) + commit = double('commit', safe_message: "Fixes #{jira_issue.to_reference}") + allow(merge_request).to receive(:commits).and_return([commit]) + + expect_any_instance_of(JiraService).not_to receive(:close_issue) + + service.execute(merge_request) + end + end end end @@ -120,13 +132,13 @@ describe MergeRequests::MergeService, services: true do let(:service) { MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message') } it 'saves error if there is an exception' do - allow(service).to receive(:repository).and_raise("error") + allow(service).to receive(:repository).and_raise("error message") allow(service).to receive(:execute_hooks) service.execute(merge_request) - expect(merge_request.merge_error).to eq("Something went wrong during merge") + expect(merge_request.merge_error).to eq("Something went wrong during merge: error message") end it 'saves error if there is an PreReceiveError exception' do diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 57c71544dff..1540b90163a 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -71,4 +71,14 @@ describe Projects::TransferService, services: true do it { expect(private_project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) } end end + + context 'missing group labels applied to issues or merge requests' do + it 'delegates tranfer to Labels::TransferService' do + group.add_owner(user) + + expect_any_instance_of(Labels::TransferService).to receive(:execute).once.and_call_original + + transfer_project(project, user, group) + end + end end diff --git a/spec/support/issue_tracker_service_shared_example.rb b/spec/support/issue_tracker_service_shared_example.rb index b6d7436c360..e70b3963d9d 100644 --- a/spec/support/issue_tracker_service_shared_example.rb +++ b/spec/support/issue_tracker_service_shared_example.rb @@ -5,3 +5,18 @@ RSpec.shared_examples 'issue tracker service URL attribute' do |url_attr| it { is_expected.not_to allow_value('ftp://example.com').for(url_attr) } it { is_expected.not_to allow_value('herp-and-derp').for(url_attr) } end + +RSpec.shared_examples 'allows project key on reference pattern' do |url_attr| + it 'allows underscores in the project name' do + expect(subject.reference_pattern.match('EXT_EXT-1234')[0]).to eq 'EXT_EXT-1234' + end + + it 'allows numbers in the project name' do + expect(subject.reference_pattern.match('EXT3_EXT-1234')[0]).to eq 'EXT3_EXT-1234' + end + + it 'requires the project name to begin with A-Z' do + expect(subject.reference_pattern.match('3EXT_EXT-1234')).to eq nil + expect(subject.reference_pattern.match('EXT_EXT-1234')[0]).to eq 'EXT_EXT-1234' + end +end