diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2017-07-07 15:40:28 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2017-07-07 15:40:28 +0000 |
commit | 8b33e654c09aaa20545e7c246585fa2d3217cecb (patch) | |
tree | 9cd6fc560ea034f41ae7607e77a10561e67a9213 | |
parent | 9fc63aa7f65ef74eb40ebc884de8fc8e031969db (diff) | |
parent | 1dab640357fa1ba8992757499e4167fcd4ce6276 (diff) | |
download | gitlab-ce-8b33e654c09aaa20545e7c246585fa2d3217cecb.tar.gz |
Merge branch 'master' into '33929-allow-to-enable-perf-bar-for-a-group'
# Conflicts:
# db/schema.rb
136 files changed, 2105 insertions, 529 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index a803cc227fe..a5510516948 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.14.0 +0.15.0 @@ -386,7 +386,7 @@ gem 'vmstat', '~> 2.3.0' gem 'sys-filesystem', '~> 1.1.6' # Gitaly GRPC client -gem 'gitaly', '~> 0.9.0' +gem 'gitaly', '~> 0.13.0' gem 'toml-rb', '~> 0.3.15', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 70abc0669df..ba580bd1309 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -278,7 +278,7 @@ GEM po_to_json (>= 1.0.0) rails (>= 3.2.0) gherkin-ruby (0.3.2) - gitaly (0.9.0) + gitaly (0.13.0) google-protobuf (~> 3.1) grpc (~> 1.0) github-linguist (4.7.6) @@ -980,7 +980,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.2.0) - gitaly (~> 0.9.0) + gitaly (~> 0.13.0) github-linguist (~> 4.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-markup (~> 1.5.1) diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 8f716696605..ae19592ecbe 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -322,7 +322,7 @@ import PerformanceBar from './performance_bar'; new gl.Members(); new UsersSelect(); break; - case 'projects:settings:members:show': + case 'projects:project_members:index': new gl.MemberExpirationDate('.js-access-expiration-date-groups'); new GroupsSelect(); new gl.MemberExpirationDate(); diff --git a/app/assets/javascripts/pipeline_schedules/pipeline_schedule_form_bundle.js b/app/assets/javascripts/pipeline_schedules/pipeline_schedule_form_bundle.js index b424e7f205d..50c725aa3d5 100644 --- a/app/assets/javascripts/pipeline_schedules/pipeline_schedule_form_bundle.js +++ b/app/assets/javascripts/pipeline_schedules/pipeline_schedule_form_bundle.js @@ -3,6 +3,7 @@ import Translate from '../vue_shared/translate'; import intervalPatternInput from './components/interval_pattern_input.vue'; import TimezoneDropdown from './components/timezone_dropdown'; import TargetBranchDropdown from './components/target_branch_dropdown'; +import { setupPipelineVariableList } from './setup_pipeline_variable_list'; Vue.use(Translate); @@ -39,4 +40,6 @@ document.addEventListener('DOMContentLoaded', () => { gl.timezoneDropdown = new TimezoneDropdown(); gl.targetBranchDropdown = new TargetBranchDropdown(); gl.pipelineScheduleFieldErrors = new gl.GlFieldErrors(formElement); + + setupPipelineVariableList($('.js-pipeline-variable-list')); }); diff --git a/app/assets/javascripts/pipeline_schedules/setup_pipeline_variable_list.js b/app/assets/javascripts/pipeline_schedules/setup_pipeline_variable_list.js new file mode 100644 index 00000000000..644efd10509 --- /dev/null +++ b/app/assets/javascripts/pipeline_schedules/setup_pipeline_variable_list.js @@ -0,0 +1,71 @@ +function insertRow($row) { + const $rowClone = $row.clone(); + $rowClone.removeAttr('data-is-persisted'); + $rowClone.find('input, textarea').val(''); + $row.after($rowClone); +} + +function removeRow($row) { + const isPersisted = gl.utils.convertPermissionToBoolean($row.attr('data-is-persisted')); + + if (isPersisted) { + $row.hide(); + $row + .find('.js-destroy-input') + .val(1); + } else { + $row.remove(); + } +} + +function checkIfRowTouched($row) { + return $row.find('.js-user-input').toArray().some(el => $(el).val().length > 0); +} + +function setupPipelineVariableList(parent = document) { + const $parent = $(parent); + + $parent.on('click', '.js-row-remove-button', (e) => { + const $row = $(e.currentTarget).closest('.js-row'); + removeRow($row); + + e.preventDefault(); + }); + + // Remove any empty rows except the last r + $parent.on('blur', '.js-user-input', (e) => { + const $row = $(e.currentTarget).closest('.js-row'); + + const isTouched = checkIfRowTouched($row); + if ($row.is(':not(:last-child)') && !isTouched) { + removeRow($row); + } + }); + + // Always make sure there is an empty last row + $parent.on('input', '.js-user-input', () => { + const $lastRow = $parent.find('.js-row').last(); + + const isTouched = checkIfRowTouched($lastRow); + if (isTouched) { + insertRow($lastRow); + } + }); + + // Clear out the empty last row so it + // doesn't get submitted and throw validation errors + $parent.closest('form').on('submit', () => { + const $lastRow = $parent.find('.js-row').last(); + + const isTouched = checkIfRowTouched($lastRow); + if (!isTouched) { + $lastRow.find('input, textarea').attr('name', ''); + } + }); +} + +export { + setupPipelineVariableList, + insertRow, + removeRow, +}; diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 2e4b1280a97..8b4ea0fbff2 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -153,6 +153,7 @@ $code_line_height: 1.6; * Padding */ $gl-padding: 16px; +$gl-col-padding: 15px; $gl-btn-padding: 10px; $gl-input-padding: 10px; $gl-vert-padding: 6px; @@ -443,6 +444,7 @@ $logs-p-color: #333; /* * Forms */ +$input-height: 34px; $input-danger-bg: #f2dede; $input-danger-border: $red-400; $input-group-addon-bg: #f7f8fa; @@ -575,6 +577,12 @@ $stage-hover-border: #d1e7fc; $action-icon-color: #d6d6d6; /* +Pipeline Schedules +*/ +$pipeline-variable-remove-button-width: calc(1em + #{2 * $gl-padding}); + + +/* Filtered Search */ $filter-name-resting-color: #f8f8f8; diff --git a/app/assets/stylesheets/pages/pipeline_schedules.scss b/app/assets/stylesheets/pages/pipeline_schedules.scss index 595eb40fec7..dc719a6ba94 100644 --- a/app/assets/stylesheets/pages/pipeline_schedules.scss +++ b/app/assets/stylesheets/pages/pipeline_schedules.scss @@ -74,3 +74,84 @@ margin-right: 3px; } } + +.pipeline-variable-list { + margin-left: 0; + margin-bottom: 0; + padding-left: 0; + list-style: none; + clear: both; +} + +.pipeline-variable-row { + display: flex; + align-items: flex-end; + + &:not(:last-child) { + margin-bottom: $gl-btn-padding; + } + + @media (max-width: $screen-sm-max) { + padding-right: $gl-col-padding; + } + + &:last-child { + & .pipeline-variable-row-remove-button { + display: none; + } + + @media (max-width: $screen-sm-max) { + & .pipeline-variable-value-input { + margin-right: $pipeline-variable-remove-button-width; + } + } + + @media (max-width: $screen-xs-max) { + .pipeline-variable-row-body { + margin-right: $pipeline-variable-remove-button-width; + } + } + } +} + +.pipeline-variable-row-body { + display: flex; + width: calc(75% - #{$gl-col-padding}); + padding-left: $gl-col-padding; + + @media (max-width: $screen-sm-max) { + width: 100%; + } + + @media (max-width: $screen-xs-max) { + display: block; + } +} + +.pipeline-variable-key-input { + margin-right: $gl-btn-padding; + + @media (max-width: $screen-xs-max) { + margin-bottom: $gl-btn-padding; + } +} + +.pipeline-variable-row-remove-button { + flex-shrink: 0; + display: flex; + justify-content: center; + align-items: center; + width: $pipeline-variable-remove-button-width; + height: $input-height; + padding: 0; + background: transparent; + border: 0; + color: $gl-text-color-secondary; + @include transition(color); + + &:hover, + &:focus { + outline: none; + color: $gl-text-color; + } +} diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index 47d9ae350ae..c6b1e443de6 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -70,7 +70,7 @@ module MembershipActions def members_page_url if membershipable.is_a?(Project) - project_settings_members_path(membershipable) + project_project_members_path(membershipable) else polymorphic_url([membershipable, :members]) end diff --git a/app/controllers/dashboard/labels_controller.rb b/app/controllers/dashboard/labels_controller.rb index dd1d46a68c7..9dcb3a0eb6d 100644 --- a/app/controllers/dashboard/labels_controller.rb +++ b/app/controllers/dashboard/labels_controller.rb @@ -1,9 +1,14 @@ class Dashboard::LabelsController < Dashboard::ApplicationController def index - labels = LabelsFinder.new(current_user).execute - respond_to do |format| format.json { render json: LabelSerializer.new.represent_appearance(labels) } end end + + def labels + finder_params = { project_ids: projects.select(:id) } + labels = LabelsFinder.new(current_user, finder_params).execute + + GlobalLabel.build_collection(labels) + end end diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb index 6b1d418fc9a..5c10d7bc261 100644 --- a/app/controllers/groups/milestones_controller.rb +++ b/app/controllers/groups/milestones_controller.rb @@ -2,13 +2,13 @@ class Groups::MilestonesController < Groups::ApplicationController include MilestoneActions before_action :group_projects - before_action :milestone, only: [:show, :update, :merge_requests, :participants, :labels] - before_action :authorize_admin_milestones!, only: [:new, :create, :update] + before_action :milestone, only: [:edit, :show, :update, :merge_requests, :participants, :labels] + before_action :authorize_admin_milestones!, only: [:edit, :new, :create, :update] def index respond_to do |format| format.html do - @milestone_states = GlobalMilestone.states_count(@projects) + @milestone_states = GlobalMilestone.states_count(group_projects, group) @milestones = Kaminari.paginate_array(milestones).page(params[:page]) end format.json do @@ -22,49 +22,41 @@ class Groups::MilestonesController < Groups::ApplicationController end def create - project_ids = params[:milestone][:project_ids].reject(&:blank?) - title = milestone_params[:title] + @milestone = Milestones::CreateService.new(group, current_user, milestone_params).execute - if create_milestones(project_ids) - redirect_to milestone_path(title) + if @milestone.persisted? + redirect_to milestone_path else - render_new_with_error(project_ids.empty?) + render "new" end end def show end - def update - @milestone.milestones.each do |milestone| - Milestones::UpdateService.new(milestone.project, current_user, milestone_params).execute(milestone) - end - - redirect_back_or_default(default: milestone_path(@milestone.title)) + def edit + render_404 if @milestone.is_legacy_group_milestone? end - private - - def create_milestones(project_ids) - return false unless project_ids.present? + def update + # Keep this compatible with legacy group milestones where we have to update + # all projects milestones states at once. + if @milestone.is_legacy_group_milestone? + update_params = milestone_params.select { |key| key == "state_event" } + milestones = @milestone.milestones + else + update_params = milestone_params + milestones = [@milestone] + end - ActiveRecord::Base.transaction do - @projects.where(id: project_ids).each do |project| - Milestones::CreateService.new(project, current_user, milestone_params).execute - end + milestones.each do |milestone| + Milestones::UpdateService.new(milestone.parent, current_user, update_params).execute(milestone) end - true - rescue ActiveRecord::ActiveRecordError => e - flash.now[:alert] = "An error occurred while creating the milestone: #{e.message}" - false + redirect_to milestone_path end - def render_new_with_error(empty_project_ids) - @milestone = Milestone.new(milestone_params) - @milestone.errors.add(:base, "Please select at least one project.") if empty_project_ids - render :new - end + private def authorize_admin_milestones! return render_404 unless can?(current_user, :admin_milestones, group) @@ -74,16 +66,31 @@ class Groups::MilestonesController < Groups::ApplicationController params.require(:milestone).permit(:title, :description, :start_date, :due_date, :state_event) end - def milestone_path(title) - group_milestone_path(@group, title.to_slug.to_s, title: title) + def milestone_path + if @milestone.is_legacy_group_milestone? + group_milestone_path(group, @milestone.safe_title, title: @milestone.title) + else + group_milestone_path(group, @milestone.iid) + end end def milestones - @milestones = GroupMilestone.build_collection(@group, @projects, params) + search_params = params.merge(group_ids: group.id) + + milestones = MilestonesFinder.new(search_params).execute + legacy_milestones = GroupMilestone.build_collection(group, group_projects, params) + + milestones + legacy_milestones end def milestone - @milestone = GroupMilestone.build(@group, @projects, params[:title]) + @milestone = + if params[:title] + GroupMilestone.build(group, group_projects, params[:title]) + else + group.milestones.find_by_iid(params[:id]) + end + render_404 unless @milestone end end diff --git a/app/controllers/projects/group_links_controller.rb b/app/controllers/projects/group_links_controller.rb index 8fc614b414d..f59200d3b1f 100644 --- a/app/controllers/projects/group_links_controller.rb +++ b/app/controllers/projects/group_links_controller.rb @@ -22,7 +22,7 @@ class Projects::GroupLinksController < Projects::ApplicationController flash[:alert] = 'Please select a group.' end - redirect_to project_settings_members_path(project) + redirect_to project_project_members_path(project) end def update @@ -36,7 +36,7 @@ class Projects::GroupLinksController < Projects::ApplicationController respond_to do |format| format.html do - redirect_to project_settings_members_path(project), status: 302 + redirect_to project_project_members_path(project), status: 302 end format.js { head :ok } end diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb index a80562e77ce..c94384d2a1a 100644 --- a/app/controllers/projects/milestones_controller.rb +++ b/app/controllers/projects/milestones_controller.rb @@ -13,20 +13,16 @@ class Projects::MilestonesController < Projects::ApplicationController respond_to :html def index - @milestones = - case params[:state] - when 'all' then @project.milestones - when 'closed' then @project.milestones.closed - else @project.milestones.active - end - @sort = params[:sort] || 'due_date_asc' - @milestones = @milestones.sort(@sort) + @milestones = milestones.sort(@sort) respond_to do |format| format.html do @project_namespace = @project.namespace.becomes(Namespace) - @milestones = @milestones.includes(:project) + # We need to show group milestones in the JSON response + # so that people can filter by and assign group milestones, + # but we don't need to show them on the project milestones page itself. + @milestones = @milestones.for_projects @milestones = @milestones.page(params[:page]) end format.json do @@ -45,12 +41,13 @@ class Projects::MilestonesController < Projects::ApplicationController end def show + @project_namespace = @project.namespace.becomes(Namespace) end def create @milestone = Milestones::CreateService.new(project, current_user, milestone_params).execute - if @milestone.save + if @milestone.valid? redirect_to project_milestone_path(@project, @milestone) else render "new" @@ -85,6 +82,18 @@ class Projects::MilestonesController < Projects::ApplicationController protected + def milestones + @milestones ||= begin + if @project.group && can?(current_user, :read_group, @project.group) + group = @project.group + end + + search_params = params.merge(project_ids: @project.id, group_ids: group&.id) + + MilestonesFinder.new(search_params).execute + end + end + def milestone @milestone ||= @project.milestones.find_by!(iid: params[:id]) end diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb index 0d967a7e691..ec7c645df5a 100644 --- a/app/controllers/projects/pipeline_schedules_controller.rb +++ b/app/controllers/projects/pipeline_schedules_controller.rb @@ -1,11 +1,11 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController + before_action :schedule, except: [:index, :new, :create] + before_action :authorize_read_pipeline_schedule! before_action :authorize_create_pipeline_schedule!, only: [:new, :create] - before_action :authorize_update_pipeline_schedule!, only: [:edit, :take_ownership, :update] + before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create] before_action :authorize_admin_pipeline_schedule!, only: [:destroy] - before_action :schedule, only: [:edit, :update, :destroy, :take_ownership] - def index @scope = params[:scope] @all_schedules = PipelineSchedulesFinder.new(@project).execute @@ -53,7 +53,7 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController redirect_to pipeline_schedules_path(@project), status: 302 else redirect_to pipeline_schedules_path(@project), - status: 302, + status: :forbidden, alert: _("Failed to remove the pipeline schedule") end end @@ -66,6 +66,15 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController def schedule_params params.require(:schedule) - .permit(:description, :cron, :cron_timezone, :ref, :active) + .permit(:description, :cron, :cron_timezone, :ref, :active, + variables_attributes: [:id, :key, :value, :_destroy] ) + end + + def authorize_update_pipeline_schedule! + return access_denied! unless can?(current_user, :update_pipeline_schedule, schedule) + end + + def authorize_admin_pipeline_schedule! + return access_denied! unless can?(current_user, :admin_pipeline_schedule, schedule) end end diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 57a6686f66c..f8ff7413b53 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -6,8 +6,23 @@ class Projects::ProjectMembersController < Projects::ApplicationController before_action :authorize_admin_project_member!, except: [:index, :leave, :request_access] def index - sort = params[:sort].presence || sort_value_name - redirect_to project_settings_members_path(@project, sort: sort) + @sort = params[:sort].presence || sort_value_name + @group_links = @project.project_group_links + + @skip_groups = @group_links.pluck(:group_id) + @skip_groups << @project.namespace_id unless @project.personal? + @skip_groups += @project.group.ancestors.pluck(:id) if @project.group + + @project_members = MembersFinder.new(@project, current_user).execute + + if params[:search].present? + @project_members = @project_members.joins(:user).merge(User.search(params[:search])) + @group_links = @group_links.where(group_id: @project.invited_groups.search(params[:search]).select(:id)) + end + + @project_members = @project_members.sort(@sort).page(params[:page]) + @requesters = AccessRequestsFinder.new(@project).execute(current_user) + @project_member = @project.project_members.new end def update @@ -19,7 +34,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def resend_invite - redirect_path = project_settings_members_path(@project) + redirect_path = project_project_members_path(@project) @project_member = @project.project_members.find(params[:id]) @@ -42,7 +57,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController return render_404 end - redirect_to(project_settings_members_path(project), + redirect_to(project_project_members_path(project), notice: notice) end diff --git a/app/controllers/projects/settings/members_controller.rb b/app/controllers/projects/settings/members_controller.rb deleted file mode 100644 index 54f9dceddef..00000000000 --- a/app/controllers/projects/settings/members_controller.rb +++ /dev/null @@ -1,27 +0,0 @@ -module Projects - module Settings - class MembersController < Projects::ApplicationController - include SortingHelper - - def show - @sort = params[:sort].presence || sort_value_name - @group_links = @project.project_group_links - - @skip_groups = @group_links.pluck(:group_id) - @skip_groups << @project.namespace_id unless @project.personal? - @skip_groups += @project.group.ancestors.pluck(:id) if @project.group - - @project_members = MembersFinder.new(@project, current_user).execute - - if params[:search].present? - @project_members = @project_members.joins(:user).merge(User.search(params[:search])) - @group_links = @group_links.where(group_id: @project.invited_groups.search(params[:search]).select(:id)) - end - - @project_members = @project_members.sort(@sort).page(params[:page]) - @requesters = AccessRequestsFinder.new(@project).execute(current_user) - @project_member = @project.project_members.new - end - end - end -end diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 7bc2117f61e..d81e9ed17d4 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -147,9 +147,17 @@ class IssuableFinder @milestones = if milestones? - scope = Milestone.where(project_id: projects) + if project? + group_id = project.group&.id + project_id = project.id + end + + group_id = group.id if group - scope.where(title: params[:milestone_title]) + search_params = + { title: params[:milestone_title], project_ids: project_id, group_ids: group_id } + + MilestonesFinder.new(search_params).execute else Milestone.none end @@ -331,11 +339,6 @@ class IssuableFinder items = items.left_joins_milestones.where('milestones.start_date <= NOW()') else items = items.with_milestone(params[:milestone_title]) - items_projects = projects(items) - - if items_projects - items = items.where(milestones: { project_id: items_projects }) - end end end diff --git a/app/finders/milestones_finder.rb b/app/finders/milestones_finder.rb index 630c73c2a94..23c42a5f662 100644 --- a/app/finders/milestones_finder.rb +++ b/app/finders/milestones_finder.rb @@ -1,12 +1,55 @@ +# Search for milestones +# +# params - Hash +# project_ids: Array of project ids or single project id. +# group_ids: Array of group ids or single group id. +# order - Orders by field default due date asc. +# title - filter by title. +# state - filters by state. + class MilestonesFinder - def execute(projects, params) - milestones = Milestone.of_projects(projects) - milestones = milestones.reorder("due_date ASC") - - case params[:state] - when 'closed' then milestones.closed - when 'all' then milestones - else milestones.active + attr_reader :params, :project_ids, :group_ids + + def initialize(params = {}) + @project_ids = Array(params[:project_ids]) + @group_ids = Array(params[:group_ids]) + @params = params + end + + def execute + return Milestone.none if project_ids.empty? && group_ids.empty? + + items = Milestone.all + items = by_groups_and_projects(items) + items = by_title(items) + items = by_state(items) + + order(items) + end + + private + + def by_groups_and_projects(items) + items.for_projects_and_groups(project_ids, group_ids) + end + + def by_title(items) + if params[:title] + items.where(title: params[:title]) + else + items + end + end + + def by_state(items) + Milestone.filter_by_state(items, params[:state]) + end + + def order(items) + if params.has_key?(:order) + items.reorder(params[:order]) + else + items.reorder('due_date ASC') end end end diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb index b5f4bbe97dc..2e36d0fdb5a 100644 --- a/app/helpers/gitlab_routing_helper.rb +++ b/app/helpers/gitlab_routing_helper.rb @@ -97,7 +97,7 @@ module GitlabRoutingHelper ## Members def project_members_url(project, *args) - project_project_members_url(project) + project_project_members_url(project, *args) end def project_member_path(project_member, *args) diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb index 8c7851dcfc2..f8860bfee99 100644 --- a/app/helpers/milestones_helper.rb +++ b/app/helpers/milestones_helper.rb @@ -54,8 +54,10 @@ module MilestonesHelper def milestone_class_for_state(param, check, match_blank_param = false) if match_blank_param 'active' if param.blank? || param == check + elsif param == check + 'active' else - 'active' if param == check + check end end @@ -147,4 +149,14 @@ module MilestonesHelper labels_dashboard_milestone_path(milestone, title: milestone.title, format: :json) end end + + def group_milestone_route(milestone, params = {}) + params = nil if params.empty? + + if milestone.is_legacy_group_milestone? + group_milestone_path(@group, milestone.safe_title, title: milestone.title, milestone: params) + else + group_milestone_path(@group, milestone.iid, milestone: params) + end + end end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 5022b291f7f..25969adb649 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -267,15 +267,15 @@ module ProjectsHelper def tab_ability_map { - environments: :read_environment, - milestones: :read_milestone, - snippets: :read_project_snippet, - settings: :admin_project, - builds: :read_build, - labels: :read_label, - issues: :read_issue, - team: :read_project_member, - wiki: :read_wiki + environments: :read_environment, + milestones: :read_milestone, + snippets: :read_project_snippet, + settings: :admin_project, + builds: :read_build, + labels: :read_label, + issues: :read_issue, + project_members: :read_project_member, + wiki: :read_wiki } end diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index 8c44f4b0934..fd7ab59ce64 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -75,7 +75,7 @@ module SearchHelper { category: "Current Project", label: "Merge Requests", url: project_merge_requests_path(@project) }, { category: "Current Project", label: "Milestones", url: project_milestones_path(@project) }, { category: "Current Project", label: "Snippets", url: project_snippets_path(@project) }, - { category: "Current Project", label: "Members", url: project_settings_members_path(@project) }, + { category: "Current Project", label: "Members", url: project_project_members_path(@project) }, { category: "Current Project", label: "Wiki", url: project_wikis_path(@project) } ] else diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 25e75a19f37..432f3f242eb 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -203,6 +203,7 @@ module Ci variables += project.group.secret_variables_for(ref, project).map(&:to_runner_variable) if project.group variables += secret_variables(environment: environment) variables += trigger_request.user_variables if trigger_request + variables += pipeline.pipeline_schedule.job_variables if pipeline.pipeline_schedule variables += persisted_environment_variables if environment variables diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb index 45d8cd34359..e4ae1b35f66 100644 --- a/app/models/ci/pipeline_schedule.rb +++ b/app/models/ci/pipeline_schedule.rb @@ -9,17 +9,21 @@ module Ci belongs_to :owner, class_name: 'User' has_one :last_pipeline, -> { order(id: :desc) }, class_name: 'Ci::Pipeline' has_many :pipelines + has_many :variables, class_name: 'Ci::PipelineScheduleVariable' validates :cron, unless: :importing?, cron: true, presence: { unless: :importing? } validates :cron_timezone, cron_timezone: true, presence: { unless: :importing? } validates :ref, presence: { unless: :importing? } validates :description, presence: true + validates :variables, variable_duplicates: true before_save :set_next_run_at scope :active, -> { where(active: true) } scope :inactive, -> { where(active: false) } + accepts_nested_attributes_for :variables, allow_destroy: true + def owned_by?(current_user) owner == current_user end @@ -56,5 +60,9 @@ module Ci Gitlab::Ci::CronParser.new(worker_cron, worker_time_zone) .next_time_from(next_run_at) end + + def job_variables + variables&.map(&:to_runner_variable) || [] + end end end diff --git a/app/models/ci/pipeline_schedule_variable.rb b/app/models/ci/pipeline_schedule_variable.rb new file mode 100644 index 00000000000..1ff177616e8 --- /dev/null +++ b/app/models/ci/pipeline_schedule_variable.rb @@ -0,0 +1,8 @@ +module Ci + class PipelineScheduleVariable < ActiveRecord::Base + extend Ci::Model + include HasVariable + + belongs_to :pipeline_schedule + end +end diff --git a/app/models/concerns/internal_id.rb b/app/models/concerns/internal_id.rb index 5382dde6765..67a0adfcd56 100644 --- a/app/models/concerns/internal_id.rb +++ b/app/models/concerns/internal_id.rb @@ -8,7 +8,8 @@ module InternalId def set_iid if iid.blank? - records = project.send(self.class.name.tableize) + parent = project || group + records = parent.send(self.class.name.tableize) records = records.with_deleted if self.paranoid? max_iid = records.maximum(:iid) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 23cb85600da..13fe9d09c69 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -30,6 +30,7 @@ module Issuable belongs_to :updated_by, class_name: "User" belongs_to :last_edited_by, class_name: 'User' belongs_to :milestone + has_many :notes, as: :noteable, inverse_of: :noteable, dependent: :destroy do # rubocop:disable Cop/ActiveRecordDependent def authors_loaded? # We check first if we're loaded to not load unnecessarily. diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb index 01599ce49c6..f0998465822 100644 --- a/app/models/concerns/milestoneish.rb +++ b/app/models/concerns/milestoneish.rb @@ -70,6 +70,22 @@ module Milestoneish due_date && due_date.past? end + def is_group_milestone? + false + end + + def is_project_milestone? + false + end + + def is_legacy_group_milestone? + false + end + + def is_dashboard_milestone? + false + end + private def count_issues_by_state(user) diff --git a/app/models/dashboard_milestone.rb b/app/models/dashboard_milestone.rb index 646c1e5ce1a..fac7c5e5c85 100644 --- a/app/models/dashboard_milestone.rb +++ b/app/models/dashboard_milestone.rb @@ -2,4 +2,8 @@ class DashboardMilestone < GlobalMilestone def issues_finder_params { authorized_only: true } end + + def is_dashboard_milestone? + true + end end diff --git a/app/models/global_label.rb b/app/models/global_label.rb index 698a7bbd327..2a1b7564962 100644 --- a/app/models/global_label.rb +++ b/app/models/global_label.rb @@ -2,7 +2,7 @@ class GlobalLabel attr_accessor :title, :labels alias_attribute :name, :title - delegate :color, :description, to: :@first_label + delegate :color, :text_color, :description, to: :@first_label def for_display @first_label diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb index 538615130a7..c0864769314 100644 --- a/app/models/global_milestone.rb +++ b/app/models/global_milestone.rb @@ -2,6 +2,7 @@ class GlobalMilestone include Milestoneish EPOCH = DateTime.parse('1970-01-01') + STATE_COUNT_HASH = { opened: 0, closed: 0, all: 0 }.freeze attr_accessor :title, :milestones alias_attribute :name, :title @@ -11,7 +12,10 @@ class GlobalMilestone end def self.build_collection(projects, params) - child_milestones = MilestonesFinder.new.execute(projects, params) + params = + { project_ids: projects.map(&:id), state: params[:state] } + + child_milestones = MilestonesFinder.new(params).execute milestones = child_milestones.select(:id, :title).group_by(&:title).map do |title, grouped| milestones_relation = Milestone.where(id: grouped.map(&:id)) @@ -28,13 +32,42 @@ class GlobalMilestone new(title, child_milestones) end - def self.states_count(projects) - relation = MilestonesFinder.new.execute(projects, state: 'all') - milestones_by_state_and_title = relation.reorder(nil).group(:state, :title).count + def self.states_count(projects, group = nil) + legacy_group_milestones_count = legacy_group_milestone_states_count(projects) + group_milestones_count = group_milestones_states_count(group) + + legacy_group_milestones_count.merge(group_milestones_count) do |k, legacy_group_milestones_count, group_milestones_count| + legacy_group_milestones_count + group_milestones_count + end + end + + def self.group_milestones_states_count(group) + return STATE_COUNT_HASH unless group + + params = { group_ids: [group.id], state: 'all', order: nil } + + relation = MilestonesFinder.new(params).execute + grouped_by_state = relation.group(:state).count + + { + opened: grouped_by_state['active'] || 0, + closed: grouped_by_state['closed'] || 0, + all: grouped_by_state.values.sum + } + end + + # Counts the legacy group milestones which must be grouped by title + def self.legacy_group_milestone_states_count(projects) + return STATE_COUNT_HASH unless projects + + params = { project_ids: projects.map(&:id), state: 'all', order: nil } + + relation = MilestonesFinder.new(params).execute + project_milestones_by_state_and_title = relation.group(:state, :title).count - opened = count_by_state(milestones_by_state_and_title, 'active') - closed = count_by_state(milestones_by_state_and_title, 'closed') - all = milestones_by_state_and_title.map { |(_, title), _| title }.uniq.count + opened = count_by_state(project_milestones_by_state_and_title, 'active') + closed = count_by_state(project_milestones_by_state_and_title, 'closed') + all = project_milestones_by_state_and_title.map { |(_, title), _| title }.uniq.count { opened: opened, diff --git a/app/models/group.rb b/app/models/group.rb index f29e642ac91..70a4ceeffd8 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -18,6 +18,7 @@ class Group < Namespace has_many :requesters, -> { where.not(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'GroupMember' # rubocop:disable Cop/ActiveRecordDependent + has_many :milestones has_many :project_group_links, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :shared_projects, through: :project_group_links, source: :project has_many :notification_settings, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent diff --git a/app/models/group_milestone.rb b/app/models/group_milestone.rb index 86d38e5468b..65249bd7bfc 100644 --- a/app/models/group_milestone.rb +++ b/app/models/group_milestone.rb @@ -16,4 +16,8 @@ class GroupMilestone < GlobalMilestone def issues_finder_params { group_id: group.id } end + + def is_legacy_group_milestone? + true + end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 2fc6191e785..6ea774470af 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -849,7 +849,10 @@ class MergeRequest < ActiveRecord::Base # def all_commit_shas if persisted? - merge_request_diffs.preload(:merge_request_diff_commits).flat_map(&:commit_shas).uniq + column_shas = MergeRequestDiffCommit.where(merge_request_diff: merge_request_diffs).pluck('DISTINCT(sha)') + serialised_shas = merge_request_diffs.where.not(st_commits: nil).flat_map(&:commit_shas) + + (column_shas + serialised_shas).uniq elsif compare_commits compare_commits.to_a.reverse.map(&:id) else diff --git a/app/models/milestone.rb b/app/models/milestone.rb index c0ccbf8e27e..48d00764965 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -18,17 +18,32 @@ class Milestone < ActiveRecord::Base cache_markdown_field :description belongs_to :project + belongs_to :group + has_many :issues has_many :labels, -> { distinct.reorder('labels.title') }, through: :issues has_many :merge_requests has_many :events, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + scope :of_projects, ->(ids) { where(project_id: ids) } + scope :of_groups, ->(ids) { where(group_id: ids) } scope :active, -> { with_state(:active) } scope :closed, -> { with_state(:closed) } - scope :of_projects, ->(ids) { where(project_id: ids) } + scope :for_projects, -> { where(group: nil).includes(:project) } + + scope :for_projects_and_groups, -> (project_ids, group_ids) do + conditions = [] + conditions << arel_table[:project_id].in(project_ids) if project_ids.compact.any? + conditions << arel_table[:group_id].in(group_ids) if group_ids.compact.any? + + where(conditions.reduce(:or)) + end + + validates :group, presence: true, unless: :project + validates :project, presence: true, unless: :group - validates :title, presence: true, uniqueness: { scope: :project_id } - validates :project, presence: true + validate :uniqueness_of_title, if: :title_changed? + validate :milestone_type_check validate :start_date_should_be_less_than_due_date, if: proc { |m| m.start_date.present? && m.due_date.present? } strip_attributes :title @@ -63,6 +78,14 @@ class Milestone < ActiveRecord::Base where(t[:title].matches(pattern).or(t[:description].matches(pattern))) end + + def filter_by_state(milestones, state) + case state + when 'closed' then milestones.closed + when 'all' then milestones + else milestones.active + end + end end def self.reference_prefix @@ -138,6 +161,8 @@ class Milestone < ActiveRecord::Base # Milestone.first.to_reference(same_namespace_project) # => "gitlab-ce%1" # def to_reference(from_project = nil, format: :iid, full: false) + return if is_group_milestone? + format_reference = milestone_format_reference(format) reference = "#{self.class.reference_prefix}#{format_reference}" @@ -152,6 +177,10 @@ class Milestone < ActiveRecord::Base id end + def for_display + self + end + def can_be_closed? active? && issues.opened.count.zero? end @@ -164,8 +193,45 @@ class Milestone < ActiveRecord::Base write_attribute(:title, sanitize_title(value)) if value.present? end + def safe_title + title.to_slug.normalize.to_s + end + + def parent + group || project + end + + def is_group_milestone? + group_id.present? + end + + def is_project_milestone? + project_id.present? + end + private + # Milestone titles must be unique across project milestones and group milestones + def uniqueness_of_title + if project + relation = Milestone.for_projects_and_groups([project_id], [project.group&.id]) + elsif group + project_ids = group.projects.map(&:id) + relation = Milestone.for_projects_and_groups(project_ids, [group.id]) + end + + title_exists = relation.find_by_title(title) + errors.add(:title, "already being used for another group or project milestone.") if title_exists + end + + # Milestone should be either a project milestone or a group milestone + def milestone_type_check + if group_id && project_id + field = project_id_changed? ? :project_id : :group_id + errors.add(field, "milestone should belong either to a project or a group.") + end + end + def milestone_format_reference(format = :iid) raise ArgumentError, 'Unknown format' unless [:iid, :name].include?(format) diff --git a/app/policies/ci/pipeline_schedule_policy.rb b/app/policies/ci/pipeline_schedule_policy.rb index 1877e89bb23..6b7598e1821 100644 --- a/app/policies/ci/pipeline_schedule_policy.rb +++ b/app/policies/ci/pipeline_schedule_policy.rb @@ -1,4 +1,14 @@ module Ci class PipelineSchedulePolicy < PipelinePolicy + alias_method :pipeline_schedule, :subject + + condition(:owner_of_schedule) do + can?(:developer_access) && pipeline_schedule.owned_by?(@user) + end + + rule { can?(:master_access) | owner_of_schedule }.policy do + enable :update_pipeline_schedule + enable :admin_pipeline_schedule + end end end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 7cbca63fab4..323131c0f7e 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -162,7 +162,6 @@ class ProjectPolicy < BasePolicy enable :create_pipeline enable :update_pipeline enable :create_pipeline_schedule - enable :update_pipeline_schedule enable :create_merge_request enable :create_wiki enable :push_code @@ -188,7 +187,6 @@ class ProjectPolicy < BasePolicy enable :admin_build enable :admin_container_image enable :admin_pipeline - enable :admin_pipeline_schedule enable :admin_environment enable :admin_deployment enable :admin_pages diff --git a/app/serializers/label_entity.rb b/app/serializers/label_entity.rb index ad565654342..4452161051e 100644 --- a/app/serializers/label_entity.rb +++ b/app/serializers/label_entity.rb @@ -1,5 +1,6 @@ class LabelEntity < Grape::Entity - expose :id + expose :id, if: ->(label, _) { !label.is_a?(GlobalLabel) } + expose :title expose :color expose :description diff --git a/app/services/boards/create_service.rb b/app/services/boards/create_service.rb index 68f6a8619e5..9eedb9e65a2 100644 --- a/app/services/boards/create_service.rb +++ b/app/services/boards/create_service.rb @@ -1,19 +1,22 @@ module Boards class CreateService < BaseService def execute - if project.boards.empty? - create_board! - else - project.boards.first - end + create_board! if can_create_board? end private + def can_create_board? + project.boards.size == 0 + end + def create_board! - board = project.boards.create - board.lists.create(list_type: :backlog) - board.lists.create(list_type: :closed) + board = project.boards.create(params) + + if board.persisted? + board.lists.create(list_type: :backlog) + board.lists.create(list_type: :closed) + end board end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 8dd0846f3bc..a03a7abfeb1 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -2,8 +2,11 @@ class IssuableBaseService < BaseService private def create_milestone_note(issuable) + milestone = issuable.milestone + return if milestone && milestone.is_group_milestone? + SystemNoteService.change_milestone( - issuable, issuable.project, current_user, issuable.milestone) + issuable, issuable.project, current_user, milestone) end def create_labels_note(issuable, old_labels) @@ -89,10 +92,12 @@ class IssuableBaseService < BaseService milestone_id = params[:milestone_id] return unless milestone_id - if milestone_id == IssuableFinder::NONE || - project.milestones.find_by(id: milestone_id).nil? - params[:milestone_id] = '' - end + params[:milestone_id] = '' if milestone_id == IssuableFinder::NONE + + milestone = + Milestone.for_projects_and_groups([project.id], [project.group&.id]).find_by_id(milestone_id) + + params[:milestone_id] = '' unless milestone end def filter_labels diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index 711f4035c55..29def25719d 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -61,8 +61,18 @@ module Issues end def cloneable_milestone_id - @new_project.milestones - .find_by(title: @old_issue.milestone.try(:title)).try(:id) + title = @old_issue.milestone&.title + return unless title + + if @new_project.group && can?(current_user, :read_group, @new_project.group) + group_id = @new_project.group.id + end + + params = + { title: title, project_ids: @new_project.id, group_ids: group_id } + + milestones = MilestonesFinder.new(params).execute + milestones.first&.id end def rewrite_notes diff --git a/app/services/milestones/base_service.rb b/app/services/milestones/base_service.rb index 176ab9f1ab5..4963601ea8b 100644 --- a/app/services/milestones/base_service.rb +++ b/app/services/milestones/base_service.rb @@ -1,4 +1,10 @@ module Milestones class BaseService < ::BaseService + # Parent can either a group or a project + attr_accessor :parent, :current_user, :params + + def initialize(parent, user, params = {}) + @parent, @current_user, @params = parent, user, params.dup + end end end diff --git a/app/services/milestones/close_service.rb b/app/services/milestones/close_service.rb index 608fc49d766..776ec4b287b 100644 --- a/app/services/milestones/close_service.rb +++ b/app/services/milestones/close_service.rb @@ -1,7 +1,7 @@ module Milestones class CloseService < Milestones::BaseService def execute(milestone) - if milestone.close + if milestone.close && milestone.is_project_milestone? event_service.close_milestone(milestone, current_user) end diff --git a/app/services/milestones/create_service.rb b/app/services/milestones/create_service.rb index b8e08c9f1eb..aef3124c7e3 100644 --- a/app/services/milestones/create_service.rb +++ b/app/services/milestones/create_service.rb @@ -1,9 +1,9 @@ module Milestones class CreateService < Milestones::BaseService def execute - milestone = project.milestones.new(params) + milestone = parent.milestones.new(params) - if milestone.save + if milestone.save && milestone.is_project_milestone? event_service.open_milestone(milestone, current_user) end diff --git a/app/services/milestones/reopen_service.rb b/app/services/milestones/reopen_service.rb index 573f9ee5c21..5b8b682caaf 100644 --- a/app/services/milestones/reopen_service.rb +++ b/app/services/milestones/reopen_service.rb @@ -1,7 +1,7 @@ module Milestones class ReopenService < Milestones::BaseService def execute(milestone) - if milestone.activate + if milestone.activate && milestone.is_project_milestone? event_service.reopen_milestone(milestone, current_user) end diff --git a/app/services/milestones/update_service.rb b/app/services/milestones/update_service.rb index ed64847f429..31b441ed476 100644 --- a/app/services/milestones/update_service.rb +++ b/app/services/milestones/update_service.rb @@ -5,9 +5,9 @@ module Milestones case state when 'activate' - Milestones::ReopenService.new(project, current_user, {}).execute(milestone) + Milestones::ReopenService.new(parent, current_user, {}).execute(milestone) when 'close' - Milestones::CloseService.new(project, current_user, {}).execute(milestone) + Milestones::CloseService.new(parent, current_user, {}).execute(milestone) end if params.present? diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index e4dfe87e614..6f82159e6c7 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -146,32 +146,6 @@ module QuickActions end end - desc do - "Change assignee#{'(s)' if issuable.allows_multiple_assignees?}" - end - explanation do |users| - users = issuable.allows_multiple_assignees? ? users : users.take(1) - "Change #{'assignee'.pluralize(users.size)} to #{users.map(&:to_reference).to_sentence}." - end - params do - issuable.allows_multiple_assignees? ? '@user1 @user2' : '@user' - end - condition do - issuable.persisted? && - current_user.can?(:"admin_#{issuable.to_ability_name}", project) - end - parse_params do |assignee_param| - extract_users(assignee_param) - end - command :reassign do |users| - @updates[:assignee_ids] = - if issuable.allows_multiple_assignees? - users.map(&:id) - else - [users.last.id] - end - end - desc 'Set milestone' explanation do |milestone| "Sets the milestone to #{milestone.to_reference}." if milestone diff --git a/app/validators/variable_duplicates_validator.rb b/app/validators/variable_duplicates_validator.rb new file mode 100644 index 00000000000..8a9d8892e9b --- /dev/null +++ b/app/validators/variable_duplicates_validator.rb @@ -0,0 +1,13 @@ +# VariableDuplicatesValidator +# +# This validtor is designed for especially the following condition +# - Use `accepts_nested_attributes_for :xxx` in a parent model +# - Use `validates :xxx, uniqueness: { scope: :xxx_id }` in a child model +class VariableDuplicatesValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + duplicates = value.reject(&:marked_for_destruction?).group_by(&:key).select { |_, v| v.many? }.map(&:first) + if duplicates.any? + record.errors.add(attribute, "Duplicate variables: #{duplicates.join(", ")}") + end + end +end diff --git a/app/views/groups/milestones/_form.html.haml b/app/views/groups/milestones/_form.html.haml new file mode 100644 index 00000000000..7f450cd9a93 --- /dev/null +++ b/app/views/groups/milestones/_form.html.haml @@ -0,0 +1,27 @@ += form_for [@group, @milestone], html: { class: 'form-horizontal milestone-form common-note-form js-quick-submit js-requires-input' } do |f| + .row + = form_errors(@milestone) + + .col-md-6 + .form-group + = f.label :title, "Title", class: "control-label" + .col-sm-10 + = f.text_field :title, maxlength: 255, class: "form-control", required: true, autofocus: true + .form-group.milestone-description + = f.label :description, "Description", class: "control-label" + .col-sm-10 + = render layout: 'projects/md_preview', locals: { url: '' } do + = render 'projects/zen', f: f, attr: :description, classes: 'note-textarea', placeholder: 'Write milestone description...' + .clearfix + .error-alert + + = render "shared/milestones/form_dates", f: f + + .form-actions + - if @milestone.new_record? + = f.submit 'Create milestone', class: "btn-create btn" + = link_to "Cancel", group_milestones_path(@group), class: "btn btn-cancel" + - else + = f.submit 'Update milestone', class: "btn-create btn" + = link_to "Cancel", group_milestone_path(@group, @milestone), class: "btn btn-cancel" + diff --git a/app/views/groups/milestones/_milestone.html.haml b/app/views/groups/milestones/_milestone.html.haml index 4c4e0a26728..bae8997e24c 100644 --- a/app/views/groups/milestones/_milestone.html.haml +++ b/app/views/groups/milestones/_milestone.html.haml @@ -1,5 +1,6 @@ + = render 'shared/milestones/milestone', - milestone_path: group_milestone_path(@group, milestone.safe_title, title: milestone.title), + milestone_path: group_milestone_route(milestone), issues_path: issues_group_path(@group, milestone_title: milestone.title), merge_requests_path: merge_requests_group_path(@group, milestone_title: milestone.title), milestone: milestone diff --git a/app/views/groups/milestones/edit.html.haml b/app/views/groups/milestones/edit.html.haml new file mode 100644 index 00000000000..5f6d7d209d0 --- /dev/null +++ b/app/views/groups/milestones/edit.html.haml @@ -0,0 +1,7 @@ +- page_title "Milestones" +- render "header_title" + +%h3.page-title + Edit Milestone + += render "form" diff --git a/app/views/groups/milestones/index.html.haml b/app/views/groups/milestones/index.html.haml index f91bee0b610..6ceb4092307 100644 --- a/app/views/groups/milestones/index.html.haml +++ b/app/views/groups/milestones/index.html.haml @@ -9,11 +9,6 @@ = link_to new_group_milestone_path(@group), class: "btn btn-new" do New milestone -.row-content-block - Only milestones from - %strong= @group.name - group are listed here. - .milestones %ul.content-list - if @milestones.blank? diff --git a/app/views/groups/milestones/new.html.haml b/app/views/groups/milestones/new.html.haml index 7c7573862d0..e24844661ee 100644 --- a/app/views/groups/milestones/new.html.haml +++ b/app/views/groups/milestones/new.html.haml @@ -4,40 +4,4 @@ %h3.page-title New Milestone -%p.light - This will create milestone in every selected project -%hr - -= form_for @milestone, url: group_milestones_path(@group), html: { class: 'form-horizontal milestone-form common-note-form js-quick-submit js-requires-input' } do |f| - .row - - if @milestone.errors.any? - #error_explanation - .alert.alert-danger - %ul - - @milestone.errors.full_messages.each do |msg| - %li - = msg - - .col-md-6 - .form-group - = f.label :title, "Title", class: "control-label" - .col-sm-10 - = f.text_field :title, maxlength: 255, class: "form-control", required: true, autofocus: true - .form-group.milestone-description - = f.label :description, "Description", class: "control-label" - .col-sm-10 - = render layout: 'projects/md_preview', locals: { url: '' } do - = render 'projects/zen', f: f, attr: :description, classes: 'note-textarea', placeholder: 'Write milestone description...' - .clearfix - .error-alert - .form-group - = f.label :projects, "Projects", class: "control-label" - .col-sm-10 - = f.collection_select :project_ids, @group.projects.non_archived, :id, :name, - { selected: @group.projects.non_archived.pluck(:id) }, required: true, multiple: true, class: 'select2' - - = render "shared/milestones/form_dates", f: f - - .form-actions - = f.submit 'Create milestone', class: "btn-create btn" - = link_to "Cancel", group_milestones_path(@group), class: "btn btn-cancel" += render "form" diff --git a/app/views/groups/milestones/show.html.haml b/app/views/groups/milestones/show.html.haml index 33e68bc766e..54b1b7a734a 100644 --- a/app/views/groups/milestones/show.html.haml +++ b/app/views/groups/milestones/show.html.haml @@ -1,4 +1,4 @@ = render "header_title" = render 'shared/milestones/top', milestone: @milestone, group: @group -= render 'shared/milestones/tabs', milestone: @milestone, show_project_name: true += render 'shared/milestones/tabs', milestone: @milestone, show_project_name: true if @milestone.is_legacy_group_milestone? = render 'shared/milestones/sidebar', milestone: @milestone, affix_offset: 102 diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index 14deb46eee3..fb90bb4b472 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -57,16 +57,17 @@ %span Snippets + - if project_nav_tab? :project_members + = nav_link(controller: :project_members) do + = link_to project_project_members_path(@project), title: 'Members', class: 'shortcuts-members' do + %span + Members + - if project_nav_tab? :settings = nav_link(path: %w[projects#edit members#show integrations#show services#edit repository#show ci_cd#show pages#show]) do = link_to edit_project_path(@project), title: 'Settings', class: 'shortcuts-tree' do %span Settings - - else - = nav_link(path: %w[members#show]) do - = link_to project_settings_members_path(@project), title: 'Settings', class: 'shortcuts-tree' do - %span - Settings -# Shortcut to Project > Activity %li.hidden diff --git a/app/views/projects/pipeline_schedules/_form.html.haml b/app/views/projects/pipeline_schedules/_form.html.haml index fc7fa5c1876..857ae00d0ab 100644 --- a/app/views/projects/pipeline_schedules/_form.html.haml +++ b/app/views/projects/pipeline_schedules/_form.html.haml @@ -24,6 +24,14 @@ = f.text_field :ref, value: @schedule.ref, id: 'schedule_ref', class: 'hidden', name: 'schedule[ref]', required: true .form-group .col-md-9 + %label.label-light + #{ s_('PipelineSchedules|Variables') } + %ul.js-pipeline-variable-list.pipeline-variable-list + - @schedule.variables.each do |variable| + = render 'variable_row', id: variable.id, key: variable.key, value: variable.value + = render 'variable_row' + .form-group + .col-md-9 = f.label :active, s_('PipelineSchedules|Activated'), class: 'label-light' %div = f.check_box :active, required: false, value: @schedule.active? diff --git a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml index 08ccd57c81a..97c0407a01d 100644 --- a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml +++ b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml @@ -26,7 +26,7 @@ = pipeline_schedule.owner&.name %td .pull-right.btn-group - - if can?(current_user, :update_pipeline_schedule, @project) && !pipeline_schedule.owned_by?(current_user) + - if can?(current_user, :update_pipeline_schedule, pipeline_schedule) = link_to take_ownership_pipeline_schedule_path(pipeline_schedule), method: :post, title: s_('PipelineSchedules|Take ownership'), class: 'btn' do = s_('PipelineSchedules|Take ownership') - if can?(current_user, :update_pipeline_schedule, pipeline_schedule) diff --git a/app/views/projects/pipeline_schedules/_variable_row.html.haml b/app/views/projects/pipeline_schedules/_variable_row.html.haml new file mode 100644 index 00000000000..564cb5d1ca9 --- /dev/null +++ b/app/views/projects/pipeline_schedules/_variable_row.html.haml @@ -0,0 +1,17 @@ +- id = local_assigns.fetch(:id, nil) +- key = local_assigns.fetch(:key, "") +- value = local_assigns.fetch(:value, "") +%li.js-row.pipeline-variable-row{ data: { is_persisted: "#{!id.nil?}" } } + .pipeline-variable-row-body + %input{ type: "hidden", name: "schedule[variables_attributes][][id]", value: id } + %input.js-destroy-input{ type: "hidden", name: "schedule[variables_attributes][][_destroy]" } + %input.js-user-input.pipeline-variable-key-input.form-control{ type: "text", + name: "schedule[variables_attributes][][key]", + value: key, + placeholder: s_('PipelineSchedules|Input variable key') } + %textarea.js-user-input.pipeline-variable-value-input.form-control{ rows: 1, + name: "schedule[variables_attributes][][value]", + placeholder: s_('PipelineSchedules|Input variable value') } + = value + %button.js-row-remove-button.pipeline-variable-row-remove-button{ 'aria-label': s_('PipelineSchedules|Remove variable row') } + %i.fa.fa-minus-circle{ 'aria-hidden': "true" } diff --git a/app/views/projects/project_members/_team.html.haml b/app/views/projects/project_members/_team.html.haml index 7ed467c8841..1f18490594b 100644 --- a/app/views/projects/project_members/_team.html.haml +++ b/app/views/projects/project_members/_team.html.haml @@ -5,7 +5,7 @@ %strong #{@project.name} %span.badge= @project_members.total_count - = form_tag project_settings_members_path(@project), method: :get, class: 'form-inline member-search-form flex-project-members-form' do + = form_tag project_project_members_path(@project), method: :get, class: 'form-inline member-search-form flex-project-members-form' do .form-group = search_field_tag :search, params[:search], { placeholder: 'Find existing members by name', class: 'form-control', spellcheck: false } %button.member-search-btn{ type: "submit", "aria-label" => "Submit search" } diff --git a/app/views/projects/project_members/import.html.haml b/app/views/projects/project_members/import.html.haml index 03b33eb2da7..f6ca8d5a921 100644 --- a/app/views/projects/project_members/import.html.haml +++ b/app/views/projects/project_members/import.html.haml @@ -12,4 +12,4 @@ .form-actions = button_tag 'Import project members', class: "btn btn-create" - = link_to "Cancel", project_settings_members_path(@project), class: "btn btn-cancel" + = link_to "Cancel", project_project_members_path(@project), class: "btn btn-cancel" diff --git a/app/views/projects/project_members/_index.html.haml b/app/views/projects/project_members/index.html.haml index 08e2d6177ab..25153fd0b6f 100644 --- a/app/views/projects/project_members/_index.html.haml +++ b/app/views/projects/project_members/index.html.haml @@ -1,3 +1,5 @@ +- page_title "Members" + .row.prepend-top-default .col-lg-12 %h4 diff --git a/app/views/projects/settings/_head.html.haml b/app/views/projects/settings/_head.html.haml index b5773acb5a4..15ba09b10ba 100644 --- a/app/views/projects/settings/_head.html.haml +++ b/app/views/projects/settings/_head.html.haml @@ -9,10 +9,6 @@ = link_to edit_project_path(@project), title: 'General' do %span General - = nav_link(controller: :members) do - = link_to project_settings_members_path(@project), title: 'Members' do - %span - Members - if can_edit = nav_link(controller: [:integrations, :services, :hooks, :hook_logs]) do = link_to project_settings_integrations_path(@project), title: 'Integrations' do diff --git a/app/views/shared/milestones/_issuable.html.haml b/app/views/shared/milestones/_issuable.html.haml index a7c67ac9980..3739f4c221d 100644 --- a/app/views/shared/milestones/_issuable.html.haml +++ b/app/views/shared/milestones/_issuable.html.haml @@ -1,14 +1,13 @@ -# @project is present when viewing Project's milestone - project = @project || issuable.project - namespace = @project_namespace || project.namespace.becomes(Namespace) +- labels = issuable.labels - assignees = issuable.assignees -- issuable_type = issuable.class.table_name - base_url_args = [namespace, project] -- issuable_type_args = base_url_args + [issuable_type] +- issuable_type_args = base_url_args + [issuable.class.table_name] - issuable_url_args = base_url_args + [issuable] -- can_update = can?(current_user, :"update_#{issuable.to_ability_name}", issuable) -%li{ id: dom_id(issuable, 'sortable'), class: "issuable-row #{'is-disabled' unless can_update}", 'data-iid' => issuable.iid, 'data-id' => issuable.id, 'data-url' => polymorphic_path(issuable_url_args) } +%li.issuable-row %span - if show_project_name %strong #{project.name} · @@ -18,10 +17,10 @@ = confidential_icon(issuable) = link_to issuable.title, issuable_url_args, title: issuable.title .issuable-detail - = link_to [project.namespace.becomes(Namespace), project, issuable] do + = link_to [namespace, project, issuable] do %span.issuable-number= issuable.to_reference - - issuable.labels.each do |label| + - labels.each do |label| = link_to polymorphic_path(issuable_type_args, { milestone_title: @milestone.title, label_name: label.title, state: 'all' }) do - render_colored_label(label) diff --git a/app/views/shared/milestones/_milestone.html.haml b/app/views/shared/milestones/_milestone.html.haml index ecc8b42979c..6f6a036b13f 100644 --- a/app/views/shared/milestones/_milestone.html.haml +++ b/app/views/shared/milestones/_milestone.html.haml @@ -1,10 +1,15 @@ - dashboard = local_assigns[:dashboard] -- custom_dom_id = dom_id(@project ? milestone : milestone.milestones.first) +- custom_dom_id = dom_id(milestone.try(:milestones) ? milestone.milestones.first : milestone) %li{ class: "milestone milestone-#{milestone.closed? ? 'closed' : 'open'}", id: custom_dom_id } .row .col-sm-6 %strong= link_to truncate(milestone.title, length: 100), milestone_path + - if milestone.is_group_milestone? + %span - Group Milestone + - else + %span - Project Milestone + .col-sm-6 .pull-right.light #{milestone.percent_complete(current_user)}% complete .row @@ -13,26 +18,32 @@ · = link_to pluralize(milestone.merge_requests.size, 'Merge Request'), merge_requests_path .col-sm-6= milestone_progress_bar(milestone) - - if milestone.is_a?(GlobalMilestone) + - if milestone.is_a?(GlobalMilestone) || milestone.is_group_milestone? .row .col-sm-6 - .expiration= render('shared/milestone_expired', milestone: milestone) - .projects - - milestone.milestones.each do |milestone| - = link_to milestone_path(milestone) do - %span.label.label-gray - = dashboard ? milestone.project.name_with_namespace : milestone.project.name + - if milestone.is_legacy_group_milestone? + .expiration= render('shared/milestone_expired', milestone: milestone) + .projects + - milestone.milestones.each do |milestone| + = link_to milestone_path(milestone) do + %span.label.label-gray + = dashboard ? milestone.project.name_with_namespace : milestone.project.name - if @group - .col-sm-6 + .col-sm-6.milestone-actions - if can?(current_user, :admin_milestones, @group) + - if milestone.is_group_milestone? + = link_to edit_group_milestone_path(@group, milestone.id), class: "btn btn-xs btn-grouped" do + Edit + \ - if milestone.closed? - = link_to 'Reopen Milestone', group_milestone_path(@group, milestone.safe_title, title: milestone.title, milestone: {state_event: :activate }), method: :put, class: "btn btn-xs btn-grouped btn-reopen" + = link_to 'Reopen Milestone', group_milestone_route(milestone, {state_event: :activate }), method: :put, class: "btn btn-xs btn-grouped btn-reopen" - else - = link_to 'Close Milestone', group_milestone_path(@group, milestone.safe_title, title: milestone.title, milestone: {state_event: :close }), method: :put, class: "btn btn-xs btn-close" + = link_to 'Close Milestone', group_milestone_route(milestone, {state_event: :close }), method: :put, class: "btn btn-xs btn-grouped btn-close" - if @project .row - .col-sm-6= render('shared/milestone_expired', milestone: milestone) + .col-sm-6 + = render('shared/milestone_expired', milestone: milestone) .col-sm-6.milestone-actions - if can?(current_user, :admin_milestone, milestone.project) and milestone.active? = link_to edit_project_milestone_path(milestone.project, milestone), class: "btn btn-xs btn-grouped" do diff --git a/app/views/shared/milestones/_tabs.html.haml b/app/views/shared/milestones/_tabs.html.haml index e2d1695b7c3..b95a4ea674d 100644 --- a/app/views/shared/milestones/_tabs.html.haml +++ b/app/views/shared/milestones/_tabs.html.haml @@ -1,8 +1,10 @@ +- issues_accessible = milestone.is_a?(GlobalMilestone) || can?(current_user, :read_issue, @project) + .scrolling-tabs-container.inner-page-scroll-tabs.is-smaller .fade-left= icon('angle-left') .fade-right= icon('angle-right') %ul.nav-links.scrolling-tabs.js-milestone-tabs - - if milestone.is_a?(GlobalMilestone) || can?(current_user, :read_issue, @project) + - if issues_accessible %li.active = link_to '#tab-issues', 'data-toggle' => 'tab', 'data-show' => '.tab-issues-buttons' do Issues @@ -25,13 +27,14 @@ Labels %span.badge= milestone.labels.count +- issues = milestone.sorted_issues(current_user) - show_project_name = local_assigns.fetch(:show_project_name, false) - show_full_project_name = local_assigns.fetch(:show_full_project_name, false) .tab-content.milestone-content - - if milestone.is_a?(GlobalMilestone) || can?(current_user, :read_issue, @project) + - if issues_accessible .tab-pane.active#tab-issues{ data: { sort_endpoint: (sort_issues_project_milestone_path(@project, @milestone) if @project && current_user) } } - = render 'shared/milestones/issues_tab', issues: milestone.sorted_issues(current_user), show_project_name: show_project_name, show_full_project_name: show_full_project_name + = render 'shared/milestones/issues_tab', issues: issues, show_project_name: show_project_name, show_full_project_name: show_full_project_name .tab-pane#tab-merge-requests -# loaded async = render "shared/milestones/tab_loading" diff --git a/app/views/shared/milestones/_top.html.haml b/app/views/shared/milestones/_top.html.haml index 20a12613cfc..b93837e3087 100644 --- a/app/views/shared/milestones/_top.html.haml +++ b/app/views/shared/milestones/_top.html.haml @@ -22,39 +22,55 @@ - if group .pull-right - if can?(current_user, :admin_milestones, group) + - if milestone.is_group_milestone? + = link_to edit_group_milestone_path(group, milestone.iid), class: "btn btn btn-grouped" do + Edit - if milestone.active? - = link_to 'Close Milestone', group_milestone_path(group, milestone.safe_title, title: milestone.title, milestone: {state_event: :close }), method: :put, class: "btn btn-grouped btn-close" + = link_to 'Close Milestone', group_milestone_route(milestone, {state_event: :close }), method: :put, class: "btn btn-grouped btn-close" - else - = link_to 'Reopen Milestone', group_milestone_path(group, milestone.safe_title, title: milestone.title, milestone: {state_event: :activate }), method: :put, class: "btn btn-grouped btn-reopen" + = link_to 'Reopen Milestone', group_milestone_route(milestone, {state_event: :activate }), method: :put, class: "btn btn-grouped btn-reopen" .detail-page-description.milestone-detail %h2.title = markdown_field(milestone, :title) + - if @milestone.is_group_milestone? && @milestone.description.present? + %div + .description + .wiki + = markdown_field(@milestone, :description) - if milestone.complete?(current_user) && milestone.active? .alert.alert-success.prepend-top-default - close_msg = group ? 'You may close the milestone now.' : 'Navigate to the project to close the milestone.' %span All issues for this milestone are closed. #{close_msg} -.table-holder - %table.table - %thead - %tr - %th Project - %th Open issues - %th State - %th Due date - - milestone.milestones.each do |ms| - %tr - %td - - project_name = group ? ms.project.name : ms.project.name_with_namespace - = link_to project_name, project_milestone_path(ms.project, ms) - %td - = ms.issues_visible_to_user(current_user).opened.count - %td - - if ms.closed? - Closed - - else - Open - %td - = ms.expires_at +- if @milestone.is_legacy_group_milestone? || @milestone.is_dashboard_milestone? + .table-holder + %table.table + %thead + %tr + %th Project + %th Open issues + %th State + %th Due date + - milestone.milestones.each do |ms| + %tr + %td + - project_name = group ? ms.project.name : ms.project.name_with_namespace + = link_to project_name, project_milestone_path(ms.project, ms) + %td + = ms.issues_visible_to_user(current_user).opened.count + %td + - if ms.closed? + Closed + - else + Open + %td + = ms.expires_at +- elsif @milestone.is_group_milestone? + %br + View + = link_to 'Issues', issues_group_path(@group, milestone_title: milestone.title) + or + = link_to 'Merge Requests', merge_requests_group_path(@group, milestone_title: milestone.title) + in this milestone diff --git a/changelogs/unreleased/29893-change-menu-locations.yml b/changelogs/unreleased/29893-change-menu-locations.yml new file mode 100644 index 00000000000..d348adc2d74 --- /dev/null +++ b/changelogs/unreleased/29893-change-menu-locations.yml @@ -0,0 +1,3 @@ +--- +title: Moved "Members in a project" menu entry and path locations +merge_request: 11560 diff --git a/changelogs/unreleased/34590-fix-dashboard-labels-dropdown.yml b/changelogs/unreleased/34590-fix-dashboard-labels-dropdown.yml new file mode 100644 index 00000000000..11c01d28dc2 --- /dev/null +++ b/changelogs/unreleased/34590-fix-dashboard-labels-dropdown.yml @@ -0,0 +1,4 @@ +--- +title: Fix dashboard labels dropdown +merge_request: 12708 +author: diff --git a/changelogs/unreleased/34736-n-1-problem-on-milestone-page.yml b/changelogs/unreleased/34736-n-1-problem-on-milestone-page.yml new file mode 100644 index 00000000000..8df3a1a6940 --- /dev/null +++ b/changelogs/unreleased/34736-n-1-problem-on-milestone-page.yml @@ -0,0 +1,4 @@ +--- +title: N+1 problems on milestone page +merge_request: 12670 +author: Takuya Noguchi diff --git a/changelogs/unreleased/feature-intermediate-32568-adding-variables-to-pipelines-schedules.yml b/changelogs/unreleased/feature-intermediate-32568-adding-variables-to-pipelines-schedules.yml new file mode 100644 index 00000000000..d497575b7f3 --- /dev/null +++ b/changelogs/unreleased/feature-intermediate-32568-adding-variables-to-pipelines-schedules.yml @@ -0,0 +1,4 @@ +--- +title: Add variables to pipelines schedules +merge_request: 12372 +author: diff --git a/changelogs/unreleased/issue_30126_be.yml b/changelogs/unreleased/issue_30126_be.yml new file mode 100644 index 00000000000..96bb8d9574b --- /dev/null +++ b/changelogs/unreleased/issue_30126_be.yml @@ -0,0 +1,4 @@ +--- +title: Add native group milestones +merge_request: +author: diff --git a/changelogs/unreleased/speed-up-merge-request-all-commits-shas.yml b/changelogs/unreleased/speed-up-merge-request-all-commits-shas.yml new file mode 100644 index 00000000000..00f55edc2b7 --- /dev/null +++ b/changelogs/unreleased/speed-up-merge-request-all-commits-shas.yml @@ -0,0 +1,4 @@ +--- +title: Make loading new merge requests (those created after the 9.4 upgrade) faster +merge_request: +author: diff --git a/changelogs/unreleased/tc-follow-up-mia.yml b/changelogs/unreleased/tc-follow-up-mia.yml new file mode 100644 index 00000000000..6327f02032e --- /dev/null +++ b/changelogs/unreleased/tc-follow-up-mia.yml @@ -0,0 +1,4 @@ +--- +title: Undo adding the /reassign quick action +merge_request: 12701 +author: diff --git a/config/routes/group.rb b/config/routes/group.rb index e578dd8b082..23052a6c6dc 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -12,7 +12,7 @@ scope(path: 'groups/*group_id', end resource :avatar, only: [:destroy] - resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create] do + resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :edit, :update, :new, :create] do member do get :merge_requests get :participants diff --git a/config/routes/project.rb b/config/routes/project.rb index 8caee302651..62cab25c763 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -386,7 +386,7 @@ constraints(ProjectUrlConstrainer.new) do end end namespace :settings do - resource :members, only: [:show] + get :members, to: redirect('/%{namespace_id}/%{project_id}/project_members') resource :ci_cd, only: [:show], controller: 'ci_cd' resource :integrations, only: [:show] resource :repository, only: [:show], controller: :repository diff --git a/db/migrate/20170620064728_create_ci_pipeline_schedule_variables.rb b/db/migrate/20170620064728_create_ci_pipeline_schedule_variables.rb new file mode 100644 index 00000000000..92833765a82 --- /dev/null +++ b/db/migrate/20170620064728_create_ci_pipeline_schedule_variables.rb @@ -0,0 +1,25 @@ +class CreateCiPipelineScheduleVariables < ActiveRecord::Migration + DOWNTIME = false + + def up + create_table :ci_pipeline_schedule_variables do |t| + t.string :key, null: false + t.text :value + t.text :encrypted_value + t.string :encrypted_value_salt + t.string :encrypted_value_iv + t.integer :pipeline_schedule_id, null: false + + t.timestamps_with_timezone null: true + end + + add_index :ci_pipeline_schedule_variables, + [:pipeline_schedule_id, :key], + name: "index_ci_pipeline_schedule_variables_on_schedule_id_and_key", + unique: true + end + + def down + drop_table :ci_pipeline_schedule_variables + end +end diff --git a/db/migrate/20170620065449_add_foreign_key_to_ci_pipeline_schedule_variables.rb b/db/migrate/20170620065449_add_foreign_key_to_ci_pipeline_schedule_variables.rb new file mode 100644 index 00000000000..7bbf66e0ac3 --- /dev/null +++ b/db/migrate/20170620065449_add_foreign_key_to_ci_pipeline_schedule_variables.rb @@ -0,0 +1,15 @@ +class AddForeignKeyToCiPipelineScheduleVariables < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_foreign_key(:ci_pipeline_schedule_variables, :ci_pipeline_schedules, column: :pipeline_schedule_id) + end + + def down + remove_foreign_key(:ci_pipeline_schedule_variables, column: :pipeline_schedule_id) + end +end diff --git a/db/migrate/20170723183807_add_group_id_to_milestones.rb b/db/migrate/20170723183807_add_group_id_to_milestones.rb new file mode 100644 index 00000000000..e46fc4f80f0 --- /dev/null +++ b/db/migrate/20170723183807_add_group_id_to_milestones.rb @@ -0,0 +1,18 @@ +class AddGroupIdToMilestones < ActiveRecord::Migration + DOWNTIME = false + + def up + change_column_null :milestones, :project_id, true + + add_column :milestones, :group_id, :integer + end + + def down + # We cannot rollback project_id not null constraint if there are records + # with null values. + execute "DELETE from milestones WHERE project_id IS NULL" + + remove_column :milestones, :group_id + change_column :milestones, :project_id, :integer, null: false + end +end diff --git a/db/migrate/20170724184243_add_group_milestone_id_indexes.rb b/db/migrate/20170724184243_add_group_milestone_id_indexes.rb new file mode 100644 index 00000000000..d48b1884179 --- /dev/null +++ b/db/migrate/20170724184243_add_group_milestone_id_indexes.rb @@ -0,0 +1,19 @@ +class AddGroupMilestoneIdIndexes < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + DOWNTIME = false + + def up + add_concurrent_foreign_key :milestones, :namespaces, column: :group_id, on_delete: :cascade + + add_concurrent_index :milestones, :group_id + end + + def down + remove_foreign_key :milestones, column: :group_id + + remove_concurrent_index :milestones, :group_id + end +end diff --git a/db/schema.rb b/db/schema.rb index 7e780db492c..023783c2b3b 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: 20170706151212) do +ActiveRecord::Schema.define(version: 20170724184243) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -254,6 +254,19 @@ ActiveRecord::Schema.define(version: 20170706151212) do add_index "ci_builds", ["updated_at"], name: "index_ci_builds_on_updated_at", using: :btree add_index "ci_builds", ["user_id"], name: "index_ci_builds_on_user_id", using: :btree + create_table "ci_pipeline_schedule_variables", force: :cascade do |t| + t.string "key", null: false + t.text "value" + t.text "encrypted_value" + t.string "encrypted_value_salt" + t.string "encrypted_value_iv" + t.integer "pipeline_schedule_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "ci_pipeline_schedule_variables", ["pipeline_schedule_id", "key"], name: "index_ci_pipeline_schedule_variables_on_schedule_id_and_key", unique: true, using: :btree + create_table "ci_group_variables", force: :cascade do |t| t.string "key", null: false t.text "value" @@ -830,7 +843,7 @@ ActiveRecord::Schema.define(version: 20170706151212) do create_table "milestones", force: :cascade do |t| t.string "title", null: false - t.integer "project_id", null: false + t.integer "project_id" t.text "description" t.date "due_date" t.datetime "created_at" @@ -841,10 +854,12 @@ ActiveRecord::Schema.define(version: 20170706151212) do t.text "description_html" t.date "start_date" t.integer "cached_markdown_version" + t.integer "group_id" end add_index "milestones", ["description"], name: "index_milestones_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} add_index "milestones", ["due_date"], name: "index_milestones_on_due_date", using: :btree + add_index "milestones", ["group_id"], name: "index_milestones_on_group_id", using: :btree add_index "milestones", ["project_id", "iid"], name: "index_milestones_on_project_id_and_iid", unique: true, using: :btree add_index "milestones", ["title"], name: "index_milestones_on_title", using: :btree add_index "milestones", ["title"], name: "index_milestones_on_title_trigram", using: :gin, opclasses: {"title"=>"gin_trgm_ops"} @@ -1565,6 +1580,7 @@ ActiveRecord::Schema.define(version: 20170706151212) do add_foreign_key "ci_builds", "ci_pipelines", column: "auto_canceled_by_id", name: "fk_a2141b1522", on_delete: :nullify add_foreign_key "ci_builds", "ci_stages", column: "stage_id", name: "fk_3a9eaa254d", on_delete: :cascade add_foreign_key "ci_builds", "projects", name: "fk_befce0568a", on_delete: :cascade + add_foreign_key "ci_pipeline_schedule_variables", "ci_pipeline_schedules", column: "pipeline_schedule_id", name: "fk_41c35fda51", on_delete: :cascade add_foreign_key "ci_group_variables", "namespaces", column: "group_id", name: "fk_33ae4d58d8", on_delete: :cascade add_foreign_key "ci_pipeline_schedules", "projects", name: "fk_8ead60fcc4", on_delete: :cascade add_foreign_key "ci_pipeline_schedules", "users", column: "owner_id", name: "fk_9ea99f58d2", on_delete: :nullify @@ -1602,6 +1618,7 @@ ActiveRecord::Schema.define(version: 20170706151212) do add_foreign_key "merge_requests", "projects", column: "target_project_id", name: "fk_a6963e8447", on_delete: :cascade add_foreign_key "merge_requests_closing_issues", "issues", on_delete: :cascade add_foreign_key "merge_requests_closing_issues", "merge_requests", on_delete: :cascade + add_foreign_key "milestones", "namespaces", column: "group_id", name: "fk_95650a40d4", on_delete: :cascade add_foreign_key "milestones", "projects", name: "fk_9bd0a0c791", on_delete: :cascade add_foreign_key "notes", "projects", name: "fk_99e097b079", on_delete: :cascade add_foreign_key "oauth_openid_requests", "oauth_access_grants", column: "access_grant_id", name: "fk_oauth_openid_requests_oauth_access_grants_access_grant_id" diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index 548716448e6..22e7f6879ed 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -9,7 +9,7 @@ and a list of **user-defined variables**. The variables can be overwritten and they take precedence over each other in this order: -1. [Trigger variables][triggers] (take precedence over all) +1. [Trigger variables][triggers] or [scheduled pipeline variables](../../user/project/pipelines/schedules.md#making-use-of-scheduled-pipeline-variables) (take precedence over all) 1. Project-level [secret variables](#secret-variables) or [protected secret variables](#protected-secret-variables) 1. Group-level [secret variables](#secret-variables) or [protected secret variables](#protected-secret-variables) 1. YAML-defined [job-level variables](../yaml/README.md#job-variables) diff --git a/doc/user/project/milestones/index.md b/doc/user/project/milestones/index.md index 99233ed5ae2..1848514e2dd 100644 --- a/doc/user/project/milestones/index.md +++ b/doc/user/project/milestones/index.md @@ -21,14 +21,11 @@ Once you fill in all the details, hit the **Create milestone** button. >**Note:** You need [Master permissions](../../permissions.md) in order to create a milestone. -You can create a milestone for several projects in the same group simultaneously. -On the group's **Issues ➔ Milestones** page, you will be able to see the status -of that milestone across all of the selected projects. To create a new milestone -for selected projects in the group, click the **New milestone** button. The -form is the same as when creating a milestone for a specific project with the -addition of the selection of the projects you want to inherit this milestone. - -![Creating a group milestone](img/milestone_group_create.png) +You can create a milestone for a group that will be shared across group projects. +On the group's **Issues ➔ Milestones** page, you will be able to see the state +of that milestone and the issues/merge requests count that it shares across the group projects. To create a new milestone click the **New milestone** button. The form is the same as when creating a milestone for a specific project which you can find in the previous item. + +In addition to that you will be able to filter issues or merge requests by group milestones in all projects that belongs to the milestone group. ## Special milestone filters diff --git a/doc/user/project/pipelines/img/pipeline_schedule_variables.png b/doc/user/project/pipelines/img/pipeline_schedule_variables.png Binary files differnew file mode 100644 index 00000000000..47a0c6f3697 --- /dev/null +++ b/doc/user/project/pipelines/img/pipeline_schedule_variables.png diff --git a/doc/user/project/pipelines/img/pipeline_schedules_new_form.png b/doc/user/project/pipelines/img/pipeline_schedules_new_form.png Binary files differindex ea5394fa8a6..5a0e5965992 100644 --- a/doc/user/project/pipelines/img/pipeline_schedules_new_form.png +++ b/doc/user/project/pipelines/img/pipeline_schedules_new_form.png diff --git a/doc/user/project/pipelines/schedules.md b/doc/user/project/pipelines/schedules.md index 17cc21238ff..258b3a2f955 100644 --- a/doc/user/project/pipelines/schedules.md +++ b/doc/user/project/pipelines/schedules.md @@ -31,6 +31,15 @@ is installed on. ![Schedules list](img/pipeline_schedules_list.png) +### Making use of scheduled pipeline variables + +> [Introduced][ce-12328] in GitLab 9.4. + +You can pass any number of arbitrary variables and they will be available in +GitLab CI so that they can be used in your `.gitlab-ci.yml` file. + +![Scheduled pipeline variables](img/pipeline_schedule_variables.png) + ## Using only and except To configure that a job can be executed only when the pipeline has been @@ -79,4 +88,5 @@ don't have admin access to the server, ask your administrator. [ce-10533]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10533 [ce-10853]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10853 +[ce-12328]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12328 [settings]: https://about.gitlab.com/gitlab-com/settings/#cron-jobs diff --git a/features/group/milestones.feature b/features/group/milestones.feature index 1c1539b3e12..2211acfee20 100644 --- a/features/group/milestones.feature +++ b/features/group/milestones.feature @@ -22,12 +22,12 @@ Feature: Group Milestones Then I should see group milestone with descriptions and expiry date And I should see group milestone with all issues and MRs assigned to that milestone - Scenario: Create multiple milestones with one form + Scenario: Create group milestones Given I visit group "Owned" milestones page And I click new milestone button And I fill milestone name When I press create mileston button - Then milestone in each project should be created + Then group milestone should be created Scenario: I should see Issues listed with labels Given Group has projects with milestones diff --git a/features/project/active_tab.feature b/features/project/active_tab.feature index 34201cd8486..3ea0aab5a67 100644 --- a/features/project/active_tab.feature +++ b/features/project/active_tab.feature @@ -31,6 +31,11 @@ Feature: Project Active Tab Then the active main tab should be Wiki And no other main tabs should be active + Scenario: On Project Members + Given I visit my project's members page + Then the active main tab should be Members + And no other main tabs should be active + # Sub Tabs: Home Scenario: On Project Home/Show @@ -63,12 +68,6 @@ Feature: Project Active Tab And no other sub tabs should be active And the active main tab should be Settings - Scenario: On Project Members - Given I visit my project's members page - Then the active sub tab should be Members - And no other sub tabs should be active - And the active main tab should be Settings - # Sub Tabs: Repository Scenario: On Project Repository/Files diff --git a/features/steps/group/milestones.rb b/features/steps/group/milestones.rb index 7288dc87005..915d766dd60 100644 --- a/features/steps/group/milestones.rb +++ b/features/steps/group/milestones.rb @@ -54,14 +54,9 @@ class Spinach::Features::GroupMilestones < Spinach::FeatureSteps click_button "Create milestone" end - step 'milestone in each project should be created' do + step 'group milestone should be created' do group = Group.find_by(name: 'Owned') - expect(page).to have_content "Milestone v2.9.0" - expect(group.projects).to be_present - - group.projects.each do |project| - expect(page).to have_content project.name - end + expect(page).to have_content group.milestones.find_by_title('v2.9.0').title end step 'I should see the "bug" label' do diff --git a/features/steps/shared/project_tab.rb b/features/steps/shared/project_tab.rb index 0cb9229dbae..901f7f76ee9 100644 --- a/features/steps/shared/project_tab.rb +++ b/features/steps/shared/project_tab.rb @@ -32,6 +32,10 @@ module SharedProjectTab ensure_active_main_tab('Wiki') end + step 'the active main tab should be Members' do + ensure_active_main_tab('Members') + end + step 'the active main tab should be Settings' do ensure_active_main_tab('Settings') end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 94168fa4ebc..fdc0c562248 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -255,7 +255,7 @@ module API class ProjectEntity < Grape::Entity expose :id, :iid - expose(:project_id) { |entity| entity.project.id } + expose(:project_id) { |entity| entity&.project.try(:id) } expose :title, :description expose :state, :created_at, :updated_at end @@ -267,7 +267,12 @@ module API expose :deleted_file?, as: :deleted_file end - class Milestone < ProjectEntity + class Milestone < Grape::Entity + expose :id, :iid + expose(:project_id) { |entity| entity&.project_id } + expose(:group_id) { |entity| entity&.group_id } + expose :title, :description + expose :state, :created_at, :updated_at expose :due_date expose :start_date end diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 93d89209934..dbeaf9e17ef 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -74,9 +74,10 @@ module API optional :active, type: Boolean, desc: 'The activation of pipeline schedule' end put ':id/pipeline_schedules/:pipeline_schedule_id' do - authorize! :update_pipeline_schedule, user_project + authorize! :read_pipeline_schedule, user_project not_found!('PipelineSchedule') unless pipeline_schedule + authorize! :update_pipeline_schedule, pipeline_schedule if pipeline_schedule.update(declared_params(include_missing: false)) present pipeline_schedule, with: Entities::PipelineScheduleDetails @@ -92,9 +93,10 @@ module API requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' end post ':id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do - authorize! :update_pipeline_schedule, user_project + authorize! :read_pipeline_schedule, user_project not_found!('PipelineSchedule') unless pipeline_schedule + authorize! :update_pipeline_schedule, pipeline_schedule if pipeline_schedule.own!(current_user) present pipeline_schedule, with: Entities::PipelineScheduleDetails @@ -110,9 +112,10 @@ module API requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' end delete ':id/pipeline_schedules/:pipeline_schedule_id' do - authorize! :admin_pipeline_schedule, user_project + authorize! :read_pipeline_schedule, user_project not_found!('PipelineSchedule') unless pipeline_schedule + authorize! :admin_pipeline_schedule, pipeline_schedule status :accepted present pipeline_schedule.destroy, with: Entities::PipelineScheduleDetails diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index dd5a4d5ad55..e51966313d4 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -568,11 +568,17 @@ module Gitlab # Return total commits count accessible from passed ref def commit_count(ref) - walker = Rugged::Walker.new(rugged) - walker.sorting(Rugged::SORT_TOPO | Rugged::SORT_REVERSE) - oid = rugged.rev_parse_oid(ref) - walker.push(oid) - walker.count + gitaly_migrate(:commit_count) do |is_enabled| + if is_enabled + gitaly_commit_client.commit_count(ref) + else + walker = Rugged::Walker.new(rugged) + walker.sorting(Rugged::SORT_TOPO | Rugged::SORT_REVERSE) + oid = rugged.rev_parse_oid(ref) + walker.push(oid) + walker.count + end + end end # Sets HEAD to the commit specified by +ref+; +ref+ can be a branch or diff --git a/lib/gitlab/gitaly_client/commit.rb b/lib/gitlab/gitaly_client/commit.rb index b8877619797..aafc0520664 100644 --- a/lib/gitlab/gitaly_client/commit.rb +++ b/lib/gitlab/gitaly_client/commit.rb @@ -56,6 +56,15 @@ module Gitlab entry end + def commit_count(ref) + request = Gitaly::CountCommitsRequest.new( + repository: @gitaly_repo, + revision: ref + ) + + GitalyClient.call(@repository.storage, :commit_service, :count_commits, request).count + end + private def commit_diff_request_params(commit, options = {}) diff --git a/locale/en/gitlab.po b/locale/en/gitlab.po index bda3fc09e85..7065b7a635f 100644 --- a/locale/en/gitlab.po +++ b/locale/en/gitlab.po @@ -619,6 +619,12 @@ msgstr "" msgid "PipelineSchedules|Inactive" msgstr "" +msgid "PipelineSchedules|Input variable key" +msgstr "" + +msgid "PipelineSchedules|Input variable value" +msgstr "" + msgid "PipelineSchedules|Next Run" msgstr "" @@ -628,12 +634,18 @@ msgstr "" msgid "PipelineSchedules|Provide a short description for this pipeline" msgstr "" +msgid "PipelineSchedules|Remove variable row" +msgstr "" + msgid "PipelineSchedules|Take ownership" msgstr "" msgid "PipelineSchedules|Target" msgstr "" +msgid "PipelineSchedules|Variables" +msgstr "" + msgid "PipelineSheduleIntervalPattern|Custom" msgstr "" @@ -1057,6 +1069,12 @@ msgid "Withdraw Access Request" msgstr "" msgid "" +"You are going to remove %{group_name}.\n" +"Removed groups CANNOT be restored!\n" +"Are you ABSOLUTELY sure?" +msgstr "" + +msgid "" "You are going to remove %{project_name_with_namespace}.\n" "Removed project CANNOT be restored!\n" "Are you ABSOLUTELY sure?" diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9f1caeddaa7..6066314b32e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8,8 +8,8 @@ msgid "" msgstr "" "Project-Id-Version: gitlab 1.0.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2017-06-28 13:32+0200\n" -"PO-Revision-Date: 2017-06-28 13:32+0200\n" +"POT-Creation-Date: 2017-07-05 08:50-0500\n" +"PO-Revision-Date: 2017-07-05 08:50-0500\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language-Team: LANGUAGE <LL@li.org>\n" "Language: \n" @@ -620,6 +620,12 @@ msgstr "" msgid "PipelineSchedules|Inactive" msgstr "" +msgid "PipelineSchedules|Input variable key" +msgstr "" + +msgid "PipelineSchedules|Input variable value" +msgstr "" + msgid "PipelineSchedules|Next Run" msgstr "" @@ -629,12 +635,18 @@ msgstr "" msgid "PipelineSchedules|Provide a short description for this pipeline" msgstr "" +msgid "PipelineSchedules|Remove variable row" +msgstr "" + msgid "PipelineSchedules|Take ownership" msgstr "" msgid "PipelineSchedules|Target" msgstr "" +msgid "PipelineSchedules|Variables" +msgstr "" + msgid "PipelineSheduleIntervalPattern|Custom" msgstr "" @@ -1058,6 +1070,12 @@ msgid "Withdraw Access Request" msgstr "" msgid "" +"You are going to remove %{group_name}.\n" +"Removed groups CANNOT be restored!\n" +"Are you ABSOLUTELY sure?" +msgstr "" + +msgid "" "You are going to remove %{project_name_with_namespace}.\n" "Removed project CANNOT be restored!\n" "Are you ABSOLUTELY sure?" diff --git a/spec/controllers/dashboard/labels_controller_spec.rb b/spec/controllers/dashboard/labels_controller_spec.rb new file mode 100644 index 00000000000..2b63933008f --- /dev/null +++ b/spec/controllers/dashboard/labels_controller_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe Dashboard::LabelsController do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + let!(:label) { create(:label, project: project) } + + before do + sign_in(user) + project.add_reporter(user) + end + + describe "#index" do + let!(:unrelated_label) { create(:label, project: create(:empty_project, :public)) } + + it 'returns global labels for projects the user has a relationship with' do + get :index, format: :json + + expect(json_response).to be_kind_of(Array) + expect(json_response.size).to eq(1) + expect(json_response[0]["id"]).to be_nil + expect(json_response[0]["title"]).to eq(label.title) + end + end +end diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb index c6e5fb61cf9..aad67dd0164 100644 --- a/spec/controllers/groups/milestones_controller_spec.rb +++ b/spec/controllers/groups/milestones_controller_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' describe Groups::MilestonesController do let(:group) { create(:group) } - let(:project) { create(:empty_project, group: group) } - let(:project2) { create(:empty_project, group: group) } + let!(:project) { create(:empty_project, group: group) } + let!(:project2) { create(:empty_project, group: group) } let(:user) { create(:user) } let(:title) { '肯定不是中文的问题' } let(:milestone) do @@ -17,24 +17,67 @@ describe Groups::MilestonesController do end let(:milestone_path) { group_milestone_path(group, milestone.safe_title, title: milestone.title) } + let(:milestone_params) do + { + title: title, + start_date: Date.today, + due_date: 1.month.from_now.to_date + } + end + before do sign_in(user) group.add_owner(user) project.team << [user, :master] end - describe "#index" do + describe '#index' do it 'shows group milestones page' do get :index, group_id: group.to_param expect(response).to have_http_status(200) end - it 'shows group milestones JSON' do - get :index, group_id: group.to_param, format: :json + context 'as JSON' do + let!(:milestone) { create(:milestone, group: group, title: 'group milestone') } + let!(:legacy_milestone1) { create(:milestone, project: project, title: 'legacy') } + let!(:legacy_milestone2) { create(:milestone, project: project2, title: 'legacy') } - expect(response).to have_http_status(200) - expect(response.content_type).to eq 'application/json' + it 'lists legacy group milestones and group milestones' do + get :index, group_id: group.to_param, format: :json + + milestones = JSON.parse(response.body) + + expect(milestones.count).to eq(2) + expect(milestones.first["title"]).to eq("group milestone") + expect(milestones.second["title"]).to eq("legacy") + expect(response).to have_http_status(200) + expect(response.content_type).to eq 'application/json' + end + end + end + + describe '#show' do + let(:milestone1) { create(:milestone, project: project, title: 'legacy') } + let(:milestone2) { create(:milestone, project: project, title: 'legacy') } + let(:group_milestone) { create(:milestone, group: group) } + + context 'when there is a title parameter' do + it 'searchs for a legacy group milestone' do + expect(GlobalMilestone).to receive(:build) + expect(Milestone).not_to receive(:find_by_iid) + + get :show, group_id: group.to_param, id: title, title: milestone1.safe_title + end + end + + context 'when there is not a title parameter' do + it 'searchs for a group milestone' do + expect(GlobalMilestone).not_to receive(:build) + expect(Milestone).to receive(:find_by_iid) + + get :show, group_id: group.to_param, id: group_milestone.id + end end end @@ -44,16 +87,57 @@ describe Groups::MilestonesController do it "creates group milestone with Chinese title" do post :create, group_id: group.to_param, - milestone: { project_ids: [project.id, project2.id], title: title } + milestone: milestone_params - expect(response).to redirect_to(group_milestone_path(group, title.to_slug.to_s, title: title)) - expect(Milestone.where(title: title).count).to eq(2) + milestone = Milestone.find_by_title(title) + + expect(response).to redirect_to(group_milestone_path(group, milestone.iid)) + expect(milestone.group_id).to eq(group.id) + expect(milestone.due_date).to eq(milestone_params[:due_date]) + expect(milestone.start_date).to eq(milestone_params[:start_date]) + end + end + + describe "#update" do + let(:milestone) { create(:milestone, group: group) } + + it "updates group milestone" do + milestone_params[:title] = "title changed" + + put :update, + id: milestone.iid, + group_id: group.to_param, + milestone: milestone_params + + milestone.reload + expect(response).to redirect_to(group_milestone_path(group, milestone.iid)) + expect(milestone.title).to eq("title changed") end - it "redirects to new when there are no project ids" do - post :create, group_id: group.to_param, milestone: { title: title, project_ids: [""] } - expect(response).to render_template :new - expect(assigns(:milestone).errors).not_to be_nil + context "legacy group milestones" do + let!(:milestone1) { create(:milestone, project: project, title: 'legacy milestone', description: "old description") } + let!(:milestone2) { create(:milestone, project: project2, title: 'legacy milestone', description: "old description") } + + it "updates only group milestones state" do + milestone_params[:title] = "title changed" + milestone_params[:description] = "description changed" + milestone_params[:state_event] = "close" + + put :update, + id: milestone1.title.to_slug.to_s, + group_id: group.to_param, + milestone: milestone_params, + title: milestone1.title + + expect(response).to redirect_to(group_milestone_path(group, milestone1.safe_title, title: milestone1.title)) + + [milestone1, milestone2].each do |milestone| + milestone.reload + expect(milestone.title).to eq("legacy milestone") + expect(milestone.description).to eq("old description") + expect(milestone.state).to eq("closed") + end + end end end @@ -156,7 +240,7 @@ describe Groups::MilestonesController do it 'does not 404' do post :create, group_id: group.to_param, - milestone: { project_ids: [project.id, project2.id], title: title } + milestone: { title: title } expect(response).not_to have_http_status(404) end @@ -164,7 +248,7 @@ describe Groups::MilestonesController do it 'does not redirect to the correct casing' do post :create, group_id: group.to_param, - milestone: { project_ids: [project.id, project2.id], title: title } + milestone: { title: title } expect(response).not_to have_http_status(301) end @@ -176,7 +260,7 @@ describe Groups::MilestonesController do it 'returns not found' do post :create, group_id: redirect_route.path, - milestone: { project_ids: [project.id, project2.id], title: title } + milestone: { title: title } expect(response).to have_http_status(404) end diff --git a/spec/controllers/projects/group_links_controller_spec.rb b/spec/controllers/projects/group_links_controller_spec.rb index 48a2994cbc0..019a50882ab 100644 --- a/spec/controllers/projects/group_links_controller_spec.rb +++ b/spec/controllers/projects/group_links_controller_spec.rb @@ -34,7 +34,7 @@ describe Projects::GroupLinksController do it 'redirects to project group links page' do expect(response).to redirect_to( - project_settings_members_path(project) + project_project_members_path(project) ) end end @@ -65,7 +65,7 @@ describe Projects::GroupLinksController do it 'redirects to project group links page' do expect(response).to redirect_to( - project_settings_members_path(project) + project_project_members_path(project) ) end end @@ -79,7 +79,7 @@ describe Projects::GroupLinksController do it 'redirects to project group links page' do expect(response).to redirect_to( - project_settings_members_path(project) + project_project_members_path(project) ) expect(flash[:alert]).to eq('Please select a group.') end diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index 84a61b2784e..bb5a340cd96 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -31,6 +31,40 @@ describe Projects::MilestonesController do end end + describe "#index" do + context "as html" do + before do + get :index, namespace_id: project.namespace.id, project_id: project.id + end + + it "queries only projects milestones" do + milestones = assigns(:milestones) + + expect(milestones.count).to eq(1) + expect(milestones.where(project_id: nil)).to be_empty + end + end + + context "as json" do + let!(:group) { create(:group, :public) } + let!(:group_milestone) { create(:milestone, group: group) } + let!(:group_member) { create(:group_member, group: group, user: user) } + + before do + project.update(namespace: group) + get :index, namespace_id: project.namespace.id, project_id: project.id, format: :json + end + + it "queries projects milestones and groups milestones" do + milestones = assigns(:milestones) + + expect(milestones.count).to eq(2) + expect(milestones.where(project_id: nil).first).to eq(group_milestone) + expect(milestones.where(group_id: nil).first).to eq(milestone) + end + end + end + describe "#destroy" do it "removes milestone" do expect(issue.milestone_id).to eq(milestone.id) diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb index a8c44d5c313..41bf5580993 100644 --- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Projects::PipelineSchedulesController do + include AccessMatchersForController + set(:project) { create(:empty_project, :public) } let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) } @@ -17,6 +19,14 @@ describe Projects::PipelineSchedulesController do expect(response).to render_template(:index) end + it 'avoids N + 1 queries' do + control_count = ActiveRecord::QueryRecorder.new { visit_pipelines_schedules }.count + + create_list(:ci_pipeline_schedule, 2, project: project) + + expect { visit_pipelines_schedules }.not_to exceed_query_limit(control_count) + end + context 'when the scope is set to active' do let(:scope) { 'active' } @@ -36,104 +46,352 @@ describe Projects::PipelineSchedulesController do end end - describe 'GET edit' do - let(:user) { create(:user) } + describe 'GET #new' do + set(:user) { create(:user) } before do - project.add_master(user) - + project.add_developer(user) sign_in(user) end - it 'loads the pipeline schedule' do - get :edit, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + it 'initializes a pipeline schedule model' do + get :new, namespace_id: project.namespace.to_param, project_id: project expect(response).to have_http_status(:ok) - expect(assigns(:schedule)).to eq(pipeline_schedule) + expect(assigns(:schedule)).to be_a_new(Ci::PipelineSchedule) end end - describe 'DELETE #destroy' do - set(:user) { create(:user) } + describe 'POST #create' do + describe 'functionality' do + set(:user) { create(:user) } - context 'when a developer makes the request' do before do project.add_developer(user) sign_in(user) + end - delete :destroy, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + let(:basic_param) do + attributes_for(:ci_pipeline_schedule) end - it 'does not delete the pipeline schedule' do - expect(response).not_to have_http_status(:ok) + context 'when variables_attributes has one variable' do + let(:schedule) do + basic_param.merge({ + variables_attributes: [{ key: 'AAA', value: 'AAA123' }] + }) + end + + it 'creates a new schedule' do + expect { go } + .to change { Ci::PipelineSchedule.count }.by(1) + .and change { Ci::PipelineScheduleVariable.count }.by(1) + + expect(response).to have_http_status(:found) + + Ci::PipelineScheduleVariable.last.tap do |v| + expect(v.key).to eq("AAA") + expect(v.value).to eq("AAA123") + end + end + end + + context 'when variables_attributes has two variables and duplicted' do + let(:schedule) do + basic_param.merge({ + variables_attributes: [{ key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' }] + }) + end + + it 'returns an error that the keys of variable are duplicated' do + expect { go } + .to change { Ci::PipelineSchedule.count }.by(0) + .and change { Ci::PipelineScheduleVariable.count }.by(0) + + expect(assigns(:schedule).errors['variables']).not_to be_empty + end end end - context 'when a master makes the request' do + describe 'security' do + let(:schedule) { attributes_for(:ci_pipeline_schedule) } + + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_allowed_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + it { expect { go }.to be_denied_for(:visitor) } + end + + def go + post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule + end + end + + describe 'PUT #update' do + describe 'functionality' do + set(:user) { create(:user) } + let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } + before do - project.add_master(user) + project.add_developer(user) sign_in(user) end - it 'destroys the pipeline schedule' do - expect do - delete :destroy, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id - end.to change { project.pipeline_schedules.count }.by(-1) + context 'when a pipeline schedule has no variables' do + let(:basic_param) do + { description: 'updated_desc', cron: '0 1 * * *', cron_timezone: 'UTC', ref: 'patch-x', active: true } + end - expect(response).to have_http_status(302) + context 'when params include one variable' do + let(:schedule) do + basic_param.merge({ + variables_attributes: [{ key: 'AAA', value: 'AAA123' }] + }) + end + + it 'inserts new variable to the pipeline schedule' do + expect { go }.to change { Ci::PipelineScheduleVariable.count }.by(1) + + pipeline_schedule.reload + expect(response).to have_http_status(:found) + expect(pipeline_schedule.variables.last.key).to eq('AAA') + expect(pipeline_schedule.variables.last.value).to eq('AAA123') + end + end + + context 'when params include two duplicated variables' do + let(:schedule) do + basic_param.merge({ + variables_attributes: [{ key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' }] + }) + end + + it 'returns an error that variables are duplciated' do + go + + expect(assigns(:schedule).errors['variables']).not_to be_empty + end + end + end + + context 'when a pipeline schedule has one variable' do + let(:basic_param) do + { description: 'updated_desc', cron: '0 1 * * *', cron_timezone: 'UTC', ref: 'patch-x', active: true } + end + + let!(:pipeline_schedule_variable) do + create(:ci_pipeline_schedule_variable, + key: 'CCC', pipeline_schedule: pipeline_schedule) + end + + context 'when adds a new variable' do + let(:schedule) do + basic_param.merge({ + variables_attributes: [{ key: 'AAA', value: 'AAA123' }] + }) + end + + it 'adds the new variable' do + expect { go }.to change { Ci::PipelineScheduleVariable.count }.by(1) + + pipeline_schedule.reload + expect(pipeline_schedule.variables.last.key).to eq('AAA') + end + end + + context 'when adds a new duplicated variable' do + let(:schedule) do + basic_param.merge({ + variables_attributes: [{ key: 'CCC', value: 'AAA123' }] + }) + end + + it 'returns an error' do + expect { go }.not_to change { Ci::PipelineScheduleVariable.count } + + pipeline_schedule.reload + expect(assigns(:schedule).errors['variables']).not_to be_empty + end + end + + context 'when updates a variable' do + let(:schedule) do + basic_param.merge({ + variables_attributes: [{ id: pipeline_schedule_variable.id, value: 'new_value' }] + }) + end + + it 'updates the variable' do + expect { go }.not_to change { Ci::PipelineScheduleVariable.count } + + pipeline_schedule_variable.reload + expect(pipeline_schedule_variable.value).to eq('new_value') + end + end + + context 'when deletes a variable' do + let(:schedule) do + basic_param.merge({ + variables_attributes: [{ id: pipeline_schedule_variable.id, _destroy: true }] + }) + end + + it 'delete the existsed variable' do + expect { go }.to change { Ci::PipelineScheduleVariable.count }.by(-1) + end + end + + context 'when deletes and creates a same key simultaneously' do + let(:schedule) do + basic_param.merge({ + variables_attributes: [{ id: pipeline_schedule_variable.id, _destroy: true }, + { key: 'CCC', value: 'CCC123' }] + }) + end + + it 'updates the variable' do + expect { go }.not_to change { Ci::PipelineScheduleVariable.count } + + pipeline_schedule.reload + expect(pipeline_schedule.variables.last.key).to eq('CCC') + expect(pipeline_schedule.variables.last.value).to eq('CCC123') + end + end end end - end - describe 'security' do - include AccessMatchersForController + describe 'security' do + let(:schedule) { { description: 'updated_desc' } } - describe 'GET edit' do it { expect { go }.to be_allowed_for(:admin) } it { expect { go }.to be_allowed_for(:owner).of(project) } it { expect { go }.to be_allowed_for(:master).of(project) } - it { expect { go }.to be_allowed_for(:developer).of(project) } + it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) } it { expect { go }.to be_denied_for(:reporter).of(project) } it { expect { go }.to be_denied_for(:guest).of(project) } it { expect { go }.to be_denied_for(:user) } it { expect { go }.to be_denied_for(:external) } it { expect { go }.to be_denied_for(:visitor) } - def go + context 'when a developer created a pipeline schedule' do + let(:developer_1) { create(:user) } + let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: developer_1) } + + before do + project.add_developer(developer_1) + end + + it { expect { go }.to be_allowed_for(developer_1) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + end + + context 'when a master created a pipeline schedule' do + let(:master_1) { create(:user) } + let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: master_1) } + + before do + project.add_master(master_1) + end + + it { expect { go }.to be_allowed_for(master_1) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + end + end + + def go + put :update, namespace_id: project.namespace.to_param, + project_id: project, id: pipeline_schedule, + schedule: schedule + end + end + + describe 'GET #edit' do + describe 'functionality' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + it 'loads the pipeline schedule' do get :edit, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + + expect(response).to have_http_status(:ok) + expect(assigns(:schedule)).to eq(pipeline_schedule) end end - describe 'GET take_ownership' do + describe 'security' do it { expect { go }.to be_allowed_for(:admin) } it { expect { go }.to be_allowed_for(:owner).of(project) } it { expect { go }.to be_allowed_for(:master).of(project) } - it { expect { go }.to be_allowed_for(:developer).of(project) } + it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) } it { expect { go }.to be_denied_for(:reporter).of(project) } it { expect { go }.to be_denied_for(:guest).of(project) } it { expect { go }.to be_denied_for(:user) } it { expect { go }.to be_denied_for(:external) } it { expect { go }.to be_denied_for(:visitor) } + end - def go - post :take_ownership, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id - end + def go + get :edit, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id end + end - describe 'PUT update' do + describe 'GET #take_ownership' do + describe 'security' do it { expect { go }.to be_allowed_for(:admin) } it { expect { go }.to be_allowed_for(:owner).of(project) } it { expect { go }.to be_allowed_for(:master).of(project) } - it { expect { go }.to be_allowed_for(:developer).of(project) } + it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) } it { expect { go }.to be_denied_for(:reporter).of(project) } it { expect { go }.to be_denied_for(:guest).of(project) } it { expect { go }.to be_denied_for(:user) } it { expect { go }.to be_denied_for(:external) } it { expect { go }.to be_denied_for(:visitor) } + end + + def go + post :take_ownership, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + end + end + + describe 'DELETE #destroy' do + set(:user) { create(:user) } + + context 'when a developer makes the request' do + before do + project.add_developer(user) + sign_in(user) - def go - put :update, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id, - schedule: { description: 'a' } + delete :destroy, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + end + + it 'does not delete the pipeline schedule' do + expect(response).to have_http_status(:not_found) + end + end + + context 'when a master makes the request' do + before do + project.add_master(user) + sign_in(user) + end + + it 'destroys the pipeline schedule' do + expect do + delete :destroy, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + end.to change { project.pipeline_schedules.count }.by(-1) + + expect(response).to have_http_status(302) end end end diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 548ec8f487f..8671d7a78dd 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -5,11 +5,10 @@ describe Projects::ProjectMembersController do let(:project) { create(:empty_project, :public, :access_requestable) } describe 'GET index' do - it 'should have the settings/members address with a 302 status code' do + it 'should have the project_members address with a 200 status code' do get :index, namespace_id: project.namespace, project_id: project - expect(response).to have_http_status(302) - expect(response.location).to include project_settings_members_path(project) + expect(response).to have_http_status(200) end end @@ -50,7 +49,7 @@ describe Projects::ProjectMembersController do access_level: Gitlab::Access::GUEST expect(response).to set_flash.to 'Users were successfully added.' - expect(response).to redirect_to(project_settings_members_path(project)) + expect(response).to redirect_to(project_project_members_path(project)) end it 'adds no user to members' do @@ -62,7 +61,7 @@ describe Projects::ProjectMembersController do access_level: Gitlab::Access::GUEST expect(response).to set_flash.to 'Message' - expect(response).to redirect_to(project_settings_members_path(project)) + expect(response).to redirect_to(project_project_members_path(project)) end end end @@ -111,7 +110,7 @@ describe Projects::ProjectMembersController do id: member expect(response).to redirect_to( - project_settings_members_path(project) + project_project_members_path(project) ) expect(project.members).not_to include member end @@ -253,7 +252,7 @@ describe Projects::ProjectMembersController do id: member expect(response).to redirect_to( - project_settings_members_path(project) + project_project_members_path(project) ) expect(project.members).to include member end @@ -290,7 +289,7 @@ describe Projects::ProjectMembersController do expect(project.team_members).to include member expect(response).to set_flash.to 'Successfully imported' expect(response).to redirect_to( - project_settings_members_path(project) + project_project_members_path(project) ) end end diff --git a/spec/controllers/projects/settings/members_controller_spec.rb b/spec/controllers/projects/settings/members_controller_spec.rb deleted file mode 100644 index 076d6cd9c6e..00000000000 --- a/spec/controllers/projects/settings/members_controller_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -require('spec_helper') - -describe Projects::Settings::MembersController do - let(:project) { create(:empty_project, :public, :access_requestable) } - - describe 'GET show' do - it 'renders show with 200 status code' do - get :show, namespace_id: project.namespace, project_id: project - - expect(response).to have_http_status(200) - expect(response).to render_template(:show) - end - end -end diff --git a/spec/factories/ci/pipeline_schedule_variables.rb b/spec/factories/ci/pipeline_schedule_variables.rb new file mode 100644 index 00000000000..ca64d1aada0 --- /dev/null +++ b/spec/factories/ci/pipeline_schedule_variables.rb @@ -0,0 +1,8 @@ +FactoryGirl.define do + factory :ci_pipeline_schedule_variable, class: Ci::PipelineScheduleVariable do + sequence(:key) { |n| "VARIABLE_#{n}" } + value 'VARIABLE_VALUE' + + pipeline_schedule factory: :ci_pipeline_schedule + end +end diff --git a/spec/factories/milestones.rb b/spec/factories/milestones.rb index 841ab3c73b8..113665ff11b 100644 --- a/spec/factories/milestones.rb +++ b/spec/factories/milestones.rb @@ -1,7 +1,13 @@ FactoryGirl.define do factory :milestone do title - project factory: :empty_project + + transient do + project nil + group nil + project_id nil + group_id nil + end trait :active do state "active" @@ -11,6 +17,20 @@ FactoryGirl.define do state "closed" end + after(:build) do |milestone, evaluator| + if evaluator.group + milestone.group = evaluator.group + elsif evaluator.group_id + milestone.group_id = evaluator.group_id + elsif evaluator.project + milestone.project = evaluator.project + elsif evaluator.project_id + milestone.project_id = evaluator.project_id + else + milestone.project = create(:empty_project) + end + end + factory :active_milestone, traits: [:active] factory :closed_milestone, traits: [:closed] end diff --git a/spec/features/groups/milestone_spec.rb b/spec/features/groups/milestone_spec.rb index 330310eae6b..9b6eb946f4b 100644 --- a/spec/features/groups/milestone_spec.rb +++ b/spec/features/groups/milestone_spec.rb @@ -33,4 +33,32 @@ feature 'Group milestones', :feature, :js do expect(find('.start_date')).to have_content(Date.today.at_beginning_of_month.strftime('%b %-d, %Y')) end end + + context 'milestones list' do + let!(:other_project) { create(:project_empty_repo, group: group) } + + let!(:active_group_milestone) { create(:milestone, group: group, state: 'active') } + let!(:active_project_milestone1) { create(:milestone, project: project, state: 'active', title: 'v1.0') } + let!(:active_project_milestone2) { create(:milestone, project: other_project, state: 'active', title: 'v1.0') } + let!(:closed_group_milestone) { create(:milestone, group: group, state: 'closed') } + let!(:closed_project_milestone1) { create(:milestone, project: project, state: 'closed', title: 'v2.0') } + let!(:closed_project_milestone2) { create(:milestone, project: other_project, state: 'closed', title: 'v2.0') } + + before do + visit group_milestones_path(group) + end + + it 'counts milestones correctly' do + expect(find('.top-area .active .badge').text).to eq("2") + expect(find('.top-area .closed .badge').text).to eq("2") + expect(find('.top-area .all .badge').text).to eq("4") + end + + it 'lists legacy group milestones and group milestones' do + legacy_milestone = GroupMilestone.build_collection(group, group.projects, { state: 'active' }).first + + expect(page).to have_selector("#milestone_#{active_group_milestone.id}", count: 1) + expect(page).to have_selector("#milestone_#{legacy_milestone.milestones.first.id}", count: 1) + end + end end diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb index 5c75b0d56b0..4a2ca243755 100644 --- a/spec/features/issues/form_spec.rb +++ b/spec/features/issues/form_spec.rb @@ -23,7 +23,7 @@ describe 'New/edit issue', :feature, :js do visit new_project_issue_path(project) end - describe 'shorten users API pagination limit (CE)' do + describe 'shorten users API pagination limit' do before do # Using `allow_any_instance_of`/`and_wrap_original`, `original` would # somehow refer to the very block we defined to _wrap_ that method, instead of @@ -63,7 +63,7 @@ describe 'New/edit issue', :feature, :js do end end - describe 'single assignee (CE)' do + describe 'single assignee' do before do click_button 'Unassigned' diff --git a/spec/features/milestone_spec.rb b/spec/features/milestone_spec.rb index 880c53343bc..2afdd0d321c 100644 --- a/spec/features/milestone_spec.rb +++ b/spec/features/milestone_spec.rb @@ -1,10 +1,12 @@ require 'rails_helper' feature 'Milestone', feature: true do - let(:project) { create(:empty_project, :public) } + let(:group) { create(:group, :public) } + let(:project) { create(:empty_project, :public, namespace: group) } let(:user) { create(:user) } before do + create(:group_member, group: group, user: user) project.team << [user, :master] gitlab_sign_in(user) end @@ -37,8 +39,8 @@ feature 'Milestone', feature: true do end end - feature 'Open a milestone with an existing title' do - scenario 'displays validation message' do + feature 'Open a project milestone with an existing title' do + scenario 'displays validation message when there is a project milestone with same title' do milestone = create(:milestone, project: project, title: 8.7) visit new_project_milestone_path(project) @@ -47,7 +49,20 @@ feature 'Milestone', feature: true do end find('input[name="commit"]').click - expect(find('.alert-danger')).to have_content('Title has already been taken') + expect(find('.alert-danger')).to have_content('already being used for another group or project milestone.') + end + + scenario 'displays validation message when there is a group milestone with same title' do + milestone = create(:milestone, project_id: nil, group: project.group, title: 8.7) + + visit new_group_milestone_path(project.group) + + page.within '.milestone-form' do + fill_in "milestone_title", with: milestone.title + end + find('input[name="commit"]').click + + expect(find('.alert-danger')).to have_content('already being used for another group or project milestone.') end end end diff --git a/spec/features/projects/members/anonymous_user_sees_members_spec.rb b/spec/features/projects/members/anonymous_user_sees_members_spec.rb index 4958d5594ac..28c8d20aad5 100644 --- a/spec/features/projects/members/anonymous_user_sees_members_spec.rb +++ b/spec/features/projects/members/anonymous_user_sees_members_spec.rb @@ -11,10 +11,10 @@ feature 'Projects > Members > Anonymous user sees members', feature: true do end scenario "anonymous user visits the project's members page and sees the list of members" do - visit project_settings_members_path(project) + visit project_project_members_path(project) expect(current_path).to eq( - project_settings_members_path(project)) + project_project_members_path(project)) expect(page).to have_content(user.name) end end diff --git a/spec/features/projects/members/user_requests_access_spec.rb b/spec/features/projects/members/user_requests_access_spec.rb index 247cc0e6f2c..75a366a3eb7 100644 --- a/spec/features/projects/members/user_requests_access_spec.rb +++ b/spec/features/projects/members/user_requests_access_spec.rb @@ -46,10 +46,11 @@ feature 'Projects > Members > User requests access', feature: true do expect(project.requesters.exists?(user_id: user)).to be_truthy - open_project_settings_menu - click_link 'Members' + page.within('.layout-nav .nav-links') do + click_link('Members') + end - visit project_settings_members_path(project) + visit project_project_members_path(project) page.within('.content') do expect(page).not_to have_content(user.name) end diff --git a/spec/features/projects/pipeline_schedules_spec.rb b/spec/features/projects/pipeline_schedules_spec.rb index d8bb7ca9a83..fee54466b81 100644 --- a/spec/features/projects/pipeline_schedules_spec.rb +++ b/spec/features/projects/pipeline_schedules_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'Pipeline Schedules', :feature do +feature 'Pipeline Schedules', :feature, js: true do include PipelineSchedulesHelper let!(:project) { create(:project) } @@ -11,27 +11,20 @@ feature 'Pipeline Schedules', :feature do before do project.add_master(user) - gitlab_sign_in(user) - visit_page end describe 'GET /projects/pipeline_schedules' do - let(:visit_page) { visit_pipelines_schedules } - - it 'avoids N + 1 queries' do - control_count = ActiveRecord::QueryRecorder.new { visit_pipelines_schedules }.count - - create_list(:ci_pipeline_schedule, 2, project: project) - - expect { visit_pipelines_schedules }.not_to exceed_query_limit(control_count) + before do + visit_pipelines_schedules end describe 'The view' do it 'displays the required information description' do page.within('.pipeline-schedule-table-row') do expect(page).to have_content('pipeline schedule') - expect(page).to have_content(pipeline_schedule.real_next_run.strftime('%b %d, %Y')) + expect(find(".next-run-cell time")['data-original-title']) + .to include(pipeline_schedule.real_next_run.strftime('%b %-d, %Y')) expect(page).to have_link('master') expect(page).to have_link("##{pipeline.id}") end @@ -62,7 +55,7 @@ feature 'Pipeline Schedules', :feature do it 'deletes the pipeline' do click_link 'Delete' - expect(page).not_to have_content('pipeline schedule') + expect(page).not_to have_css(".pipeline-schedule-table-row") end end @@ -78,8 +71,10 @@ feature 'Pipeline Schedules', :feature do end end - describe 'POST /projects/pipeline_schedules/new', js: true do - let(:visit_page) { visit_new_pipeline_schedule } + describe 'POST /projects/pipeline_schedules/new' do + before do + visit_new_pipeline_schedule + end it 'sets defaults for timezone and target branch' do expect(page).to have_button('master') @@ -100,8 +95,8 @@ feature 'Pipeline Schedules', :feature do end end - describe 'PATCH /projects/pipelines_schedules/:id/edit', js: true do - let(:visit_page) do + describe 'PATCH /projects/pipelines_schedules/:id/edit' do + before do edit_pipeline_schedule end @@ -134,6 +129,72 @@ feature 'Pipeline Schedules', :feature do end end + context 'when user creates a new pipeline schedule with variables' do + background do + visit_pipelines_schedules + click_link 'New schedule' + fill_in_schedule_form + all('[name="schedule[variables_attributes][][key]"]')[0].set('AAA') + all('[name="schedule[variables_attributes][][value]"]')[0].set('AAA123') + all('[name="schedule[variables_attributes][][key]"]')[1].set('BBB') + all('[name="schedule[variables_attributes][][value]"]')[1].set('BBB123') + save_pipeline_schedule + end + + scenario 'user sees the new variable in edit window' do + find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click + page.within('.pipeline-variable-list') do + expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-key-input").value).to eq('AAA') + expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-value-input").value).to eq('AAA123') + expect(find(".pipeline-variable-row:nth-child(2) .pipeline-variable-key-input").value).to eq('BBB') + expect(find(".pipeline-variable-row:nth-child(2) .pipeline-variable-value-input").value).to eq('BBB123') + end + end + end + + context 'when user edits a variable of a pipeline schedule' do + background do + create(:ci_pipeline_schedule, project: project, owner: user).tap do |pipeline_schedule| + create(:ci_pipeline_schedule_variable, key: 'AAA', value: 'AAA123', pipeline_schedule: pipeline_schedule) + end + + visit_pipelines_schedules + find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click + all('[name="schedule[variables_attributes][][key]"]')[0].set('foo') + all('[name="schedule[variables_attributes][][value]"]')[0].set('bar') + click_button 'Save pipeline schedule' + end + + scenario 'user sees the updated variable in edit window' do + find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click + page.within('.pipeline-variable-list') do + expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-key-input").value).to eq('foo') + expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-value-input").value).to eq('bar') + end + end + end + + context 'when user removes a variable of a pipeline schedule' do + background do + create(:ci_pipeline_schedule, project: project, owner: user).tap do |pipeline_schedule| + create(:ci_pipeline_schedule_variable, key: 'AAA', value: 'AAA123', pipeline_schedule: pipeline_schedule) + end + + visit_pipelines_schedules + find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click + find('.pipeline-variable-list .pipeline-variable-row-remove-button').click + click_button 'Save pipeline schedule' + end + + scenario 'user does not see the removed variable in edit window' do + find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click + page.within('.pipeline-variable-list') do + expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-key-input").value).to eq('') + expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-value-input").value).to eq('') + end + end + end + def visit_new_pipeline_schedule visit new_project_pipeline_schedule_path(project, pipeline_schedule) end diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 4a52f0d5c58..bef4fd44331 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -59,6 +59,23 @@ describe IssuesFinder do end end + context 'filtering by group milestone' do + let!(:group) { create(:group, :public) } + let(:group_milestone) { create(:milestone, group: group) } + let!(:group_member) { create(:group_member, group: group, user: user) } + let(:params) { { milestone_title: group_milestone.title } } + + before do + project2.update(namespace: group) + issue2.update(milestone: group_milestone) + issue3.update(milestone: group_milestone) + end + + it 'returns issues assigned to that group milestone' do + expect(issues).to contain_exactly(issue2, issue3) + end + end + context 'filtering by no milestone' do let(:params) { { milestone_title: Milestone::None.title } } diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 5eb26de6c92..b46218bf72e 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -47,6 +47,25 @@ describe MergeRequestsFinder do expect(merge_requests).to contain_exactly(merge_request1) end + context 'filtering by group milestone' do + let!(:group) { create(:group, :public) } + let(:group_milestone) { create(:milestone, group: group) } + let!(:group_member) { create(:group_member, group: group, user: user) } + let(:params) { { milestone_title: group_milestone.title } } + + before do + project2.update(namespace: group) + merge_request2.update(milestone: group_milestone) + merge_request3.update(milestone: group_milestone) + end + + it 'returns issues assigned to that group milestone' do + merge_requests = described_class.new(user, params).execute + + expect(merge_requests).to contain_exactly(merge_request2, merge_request3) + end + end + context 'with created_after and created_before params' do let(:project4) { create(:empty_project, forked_from_project: project1) } diff --git a/spec/finders/milestones_finder_spec.rb b/spec/finders/milestones_finder_spec.rb new file mode 100644 index 00000000000..32ec983c5b8 --- /dev/null +++ b/spec/finders/milestones_finder_spec.rb @@ -0,0 +1,90 @@ +require 'spec_helper' + +describe MilestonesFinder do + let(:group) { create(:group) } + let(:project_1) { create(:empty_project, namespace: group) } + let(:project_2) { create(:empty_project, namespace: group) } + let!(:milestone_1) { create(:milestone, group: group, title: 'one test', due_date: Date.today) } + let!(:milestone_2) { create(:milestone, group: group) } + let!(:milestone_3) { create(:milestone, project: project_1, state: 'active', due_date: Date.tomorrow) } + let!(:milestone_4) { create(:milestone, project: project_2, state: 'active') } + + it 'it returns milestones for projects' do + result = described_class.new(project_ids: [project_1.id, project_2.id], state: 'all').execute + + expect(result).to contain_exactly(milestone_3, milestone_4) + end + + it 'returns milestones for groups' do + result = described_class.new(group_ids: group.id, state: 'all').execute + + expect(result).to contain_exactly(milestone_1, milestone_2) + end + + it 'returns milestones for groups and projects' do + result = described_class.new(project_ids: [project_1.id, project_2.id], group_ids: group.id, state: 'all').execute + + expect(result).to contain_exactly(milestone_1, milestone_2, milestone_3, milestone_4) + end + + context 'with filters' do + let(:params) do + { + project_ids: [project_1.id, project_2.id], + group_ids: group.id, + state: 'all' + } + end + + before do + milestone_1.close + milestone_3.close + end + + it 'filters by active state' do + params[:state] = 'active' + result = described_class.new(params).execute + + expect(result).to contain_exactly(milestone_2, milestone_4) + end + + it 'filters by closed state' do + params[:state] = 'closed' + result = described_class.new(params).execute + + expect(result).to contain_exactly(milestone_1, milestone_3) + end + + it 'filters by title' do + result = described_class.new(params.merge(title: 'one test')).execute + + expect(result.to_a).to contain_exactly(milestone_1) + end + end + + context 'with order' do + let(:params) do + { + project_ids: [project_1.id, project_2.id], + group_ids: group.id, + state: 'all' + } + end + + it "default orders by due date" do + result = described_class.new(params).execute + + expect(result.first).to eq(milestone_1) + expect(result.second).to eq(milestone_3) + end + + it "orders by parameter" do + result = described_class.new(params.merge(order: 'id DESC')).execute + + expect(result.first).to eq(milestone_4) + expect(result.second).to eq(milestone_3) + expect(result.third).to eq(milestone_2) + expect(result.fourth).to eq(milestone_1) + end + end +end diff --git a/spec/fixtures/api/schemas/public_api/v3/issues.json b/spec/fixtures/api/schemas/public_api/v3/issues.json index f2ee9c925ae..51b0822bc66 100644 --- a/spec/fixtures/api/schemas/public_api/v3/issues.json +++ b/spec/fixtures/api/schemas/public_api/v3/issues.json @@ -22,7 +22,8 @@ "properties": { "id": { "type": "integer" }, "iid": { "type": "integer" }, - "project_id": { "type": "integer" }, + "project_id": { "type": ["integer", "null"] }, + "group_id": { "type": ["integer", "null"] }, "title": { "type": "string" }, "description": { "type": ["string", "null"] }, "state": { "type": "string" }, diff --git a/spec/fixtures/api/schemas/public_api/v3/merge_requests.json b/spec/fixtures/api/schemas/public_api/v3/merge_requests.json index 01f9fbb2c89..b5c74bcc26e 100644 --- a/spec/fixtures/api/schemas/public_api/v3/merge_requests.json +++ b/spec/fixtures/api/schemas/public_api/v3/merge_requests.json @@ -53,7 +53,8 @@ "properties": { "id": { "type": "integer" }, "iid": { "type": "integer" }, - "project_id": { "type": "integer" }, + "project_id": { "type": ["integer", "null"] }, + "group_id": { "type": ["integer", "null"] }, "title": { "type": "string" }, "description": { "type": ["string", "null"] }, "state": { "type": "string" }, diff --git a/spec/fixtures/api/schemas/public_api/v4/issues.json b/spec/fixtures/api/schemas/public_api/v4/issues.json index 2d1c84ee93d..bd6bfc03199 100644 --- a/spec/fixtures/api/schemas/public_api/v4/issues.json +++ b/spec/fixtures/api/schemas/public_api/v4/issues.json @@ -22,7 +22,8 @@ "properties": { "id": { "type": "integer" }, "iid": { "type": "integer" }, - "project_id": { "type": "integer" }, + "project_id": { "type": ["integer", "null"] }, + "group_id": { "type": ["integer", "null"] }, "title": { "type": "string" }, "description": { "type": ["string", "null"] }, "state": { "type": "string" }, diff --git a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json index 51642e8cbb8..60aa47c1259 100644 --- a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json +++ b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json @@ -53,7 +53,8 @@ "properties": { "id": { "type": "integer" }, "iid": { "type": "integer" }, - "project_id": { "type": "integer" }, + "project_id": { "type": ["integer", "null"] }, + "group_id": { "type": ["integer", "null"] }, "title": { "type": "string" }, "description": { "type": ["string", "null"] }, "state": { "type": "string" }, diff --git a/spec/helpers/gitlab_routing_helper_spec.rb b/spec/helpers/gitlab_routing_helper_spec.rb index 7a522487a74..717ac1962d1 100644 --- a/spec/helpers/gitlab_routing_helper_spec.rb +++ b/spec/helpers/gitlab_routing_helper_spec.rb @@ -2,12 +2,6 @@ require 'spec_helper' describe GitlabRoutingHelper do describe 'Project URL helpers' do - describe '#project_members_url' do - let(:project) { build_stubbed(:empty_project) } - - it { expect(project_members_url(project)).to eq project_project_members_url(project) } - end - describe '#project_member_path' do let(:project_member) { create(:project_member) } diff --git a/spec/javascripts/pipeline_schedules/setup_pipeline_variable_list_spec.js b/spec/javascripts/pipeline_schedules/setup_pipeline_variable_list_spec.js new file mode 100644 index 00000000000..5b316b319a5 --- /dev/null +++ b/spec/javascripts/pipeline_schedules/setup_pipeline_variable_list_spec.js @@ -0,0 +1,145 @@ +import { + setupPipelineVariableList, + insertRow, + removeRow, +} from '~/pipeline_schedules/setup_pipeline_variable_list'; + +describe('Pipeline Variable List', () => { + let $markup; + + describe('insertRow', () => { + it('should insert another row', () => { + $markup = $(`<div> + <li class="js-row"> + <input> + <textarea></textarea> + </li> + </div>`); + + insertRow($markup.find('.js-row')); + + expect($markup.find('.js-row').length).toBe(2); + }); + + it('should clear `data-is-persisted` on cloned row', () => { + $markup = $(`<div> + <li class="js-row" data-is-persisted="true"></li> + </div>`); + + insertRow($markup.find('.js-row')); + + const $lastRow = $markup.find('.js-row').last(); + expect($lastRow.attr('data-is-persisted')).toBe(undefined); + }); + + it('should clear inputs on cloned row', () => { + $markup = $(`<div> + <li class="js-row"> + <input value="foo"> + <textarea>bar</textarea> + </li> + </div>`); + + insertRow($markup.find('.js-row')); + + const $lastRow = $markup.find('.js-row').last(); + expect($lastRow.find('input').val()).toBe(''); + expect($lastRow.find('textarea').val()).toBe(''); + }); + }); + + describe('removeRow', () => { + it('should remove dynamic row', () => { + $markup = $(`<div> + <li class="js-row"> + <input> + <textarea></textarea> + </li> + </div>`); + + removeRow($markup.find('.js-row')); + + expect($markup.find('.js-row').length).toBe(0); + }); + + it('should hide and mark to destroy with already persisted rows', () => { + $markup = $(`<div> + <li class="js-row" data-is-persisted="true"> + <input class="js-destroy-input"> + </li> + </div>`); + + const $row = $markup.find('.js-row'); + removeRow($row); + + expect($row.find('.js-destroy-input').val()).toBe('1'); + expect($markup.find('.js-row').length).toBe(1); + }); + }); + + describe('setupPipelineVariableList', () => { + beforeEach(() => { + $markup = $(`<form> + <li class="js-row"> + <input class="js-user-input" name="schedule[variables_attributes][][key]"> + <textarea class="js-user-input" name="schedule[variables_attributes][][value]"></textarea> + <button class="js-row-remove-button"></button> + <button class="js-row-add-button"></button> + </li> + </form>`); + + setupPipelineVariableList($markup); + }); + + it('should remove the row when clicking the remove button', () => { + $markup.find('.js-row-remove-button').trigger('click'); + + expect($markup.find('.js-row').length).toBe(0); + }); + + it('should add another row when editing the last rows key input', () => { + const $row = $markup.find('.js-row'); + $row.find('input.js-user-input') + .val('foo') + .trigger('input'); + + expect($markup.find('.js-row').length).toBe(2); + }); + + it('should add another row when editing the last rows value textarea', () => { + const $row = $markup.find('.js-row'); + $row.find('textarea.js-user-input') + .val('foo') + .trigger('input'); + + expect($markup.find('.js-row').length).toBe(2); + }); + + it('should remove empty row after blurring', () => { + const $row = $markup.find('.js-row'); + $row.find('input.js-user-input') + .val('foo') + .trigger('input'); + + expect($markup.find('.js-row').length).toBe(2); + + $row.find('input.js-user-input') + .val('') + .trigger('input') + .trigger('blur'); + + expect($markup.find('.js-row').length).toBe(1); + }); + + it('should clear out the `name` attribute on the inputs for the last empty row on form submission (avoid BE validation)', () => { + const $row = $markup.find('.js-row'); + expect($row.find('input').attr('name')).toBe('schedule[variables_attributes][][key]'); + expect($row.find('textarea').attr('name')).toBe('schedule[variables_attributes][][value]'); + + $markup.filter('form').submit(); + + expect($row.find('input').attr('name')).toBe(''); + expect($row.find('textarea').attr('name')).toBe(''); + }); + }); +}); diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 6aca181194a..acffd335e43 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -3,6 +3,20 @@ require "spec_helper" describe Gitlab::Git::Repository, seed_helper: true do include Gitlab::EncodingHelper + shared_examples 'wrapping gRPC errors' do |gitaly_client_class, gitaly_client_method| + it 'wraps gRPC not found error' do + expect_any_instance_of(gitaly_client_class).to receive(gitaly_client_method) + .and_raise(GRPC::NotFound) + expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository) + end + + it 'wraps gRPC unknown error' do + expect_any_instance_of(gitaly_client_class).to receive(gitaly_client_method) + .and_raise(GRPC::Unknown) + expect { subject }.to raise_error(Gitlab::Git::CommandError) + end + end + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } describe "Respond to" do @@ -35,16 +49,8 @@ describe Gitlab::Git::Repository, seed_helper: true do repository.root_ref end - it 'wraps GRPC not found' do - expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name) - .and_raise(GRPC::NotFound) - expect { repository.root_ref }.to raise_error(Gitlab::Git::Repository::NoRepository) - end - - it 'wraps GRPC exceptions' do - expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name) - .and_raise(GRPC::Unknown) - expect { repository.root_ref }.to raise_error(Gitlab::Git::CommandError) + it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :default_branch_name do + subject { repository.root_ref } end end @@ -130,17 +136,7 @@ describe Gitlab::Git::Repository, seed_helper: true do subject end - it 'wraps GRPC not found' do - expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names) - .and_raise(GRPC::NotFound) - expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository) - end - - it 'wraps GRPC other exceptions' do - expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names) - .and_raise(GRPC::Unknown) - expect { subject }.to raise_error(Gitlab::Git::CommandError) - end + it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :branch_names end describe '#tag_names' do @@ -168,17 +164,7 @@ describe Gitlab::Git::Repository, seed_helper: true do subject end - it 'wraps GRPC not found' do - expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names) - .and_raise(GRPC::NotFound) - expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository) - end - - it 'wraps GRPC exceptions' do - expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names) - .and_raise(GRPC::Unknown) - expect { subject }.to raise_error(Gitlab::Git::CommandError) - end + it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :tag_names end shared_examples 'archive check' do |extenstion| @@ -438,8 +424,21 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#commit_count' do - it { expect(repository.commit_count("master")).to eq(25) } - it { expect(repository.commit_count("feature")).to eq(9) } + shared_examples 'counting commits' do + it { expect(repository.commit_count("master")).to eq(25) } + it { expect(repository.commit_count("feature")).to eq(9) } + end + + context 'when Gitaly commit_count feature is enabled' do + it_behaves_like 'counting commits' + it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Commit, :commit_count do + subject { repository.commit_count('master') } + end + end + + context 'when Gitaly commit_count feature is disabled', skip_gitaly_mock: true do + it_behaves_like 'counting commits' + end end describe "#reset" do @@ -1298,16 +1297,8 @@ describe Gitlab::Git::Repository, seed_helper: true do @repo.local_branches end - it 'wraps GRPC not found' do - expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches) - .and_raise(GRPC::NotFound) - expect { @repo.local_branches }.to raise_error(Gitlab::Git::Repository::NoRepository) - end - - it 'wraps GRPC exceptions' do - expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches) - .and_raise(GRPC::Unknown) - expect { @repo.local_branches }.to raise_error(Gitlab::Git::CommandError) + it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :local_branches do + subject { @repo.local_branches } end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 562f0c2991c..977174a5fd2 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -45,6 +45,7 @@ label: - merge_requests - priorities milestone: +- group - project - issues - labels @@ -133,8 +134,11 @@ pipeline_schedules: - owner - pipelines - last_pipeline +- variables pipeline_schedule: - pipelines +pipeline_schedule_variables: +- pipeline_schedule deploy_keys: - user - deploy_keys_projects diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index b4a7e956686..4ef3db3721f 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -82,6 +82,7 @@ Milestone: - id - title - project_id +- group_id - description - due_date - start_date diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index cf6d356c524..154b6759f46 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1427,6 +1427,23 @@ describe Ci::Build, :models do it { is_expected.to include(predefined_trigger_variable) } end + context 'when a job was triggered by a pipeline schedule' do + let(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) } + + let!(:pipeline_schedule_variable) do + create(:ci_pipeline_schedule_variable, + key: 'SCHEDULE_VARIABLE_KEY', + pipeline_schedule: pipeline_schedule) + end + + before do + pipeline_schedule.pipelines << pipeline + pipeline_schedule.reload + end + + it { is_expected.to include(pipeline_schedule_variable.to_runner_variable) } + end + context 'when yaml_variables are undefined' do before do build.yaml_variables = nil diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index 56817baf79d..6427deda31e 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -5,6 +5,7 @@ describe Ci::PipelineSchedule, models: true do it { is_expected.to belong_to(:owner) } it { is_expected.to have_many(:pipelines) } + it { is_expected.to have_many(:variables) } it { is_expected.to respond_to(:ref) } it { is_expected.to respond_to(:cron) } @@ -117,4 +118,20 @@ describe Ci::PipelineSchedule, models: true do end end end + + describe '#job_variables' do + let!(:pipeline_schedule) { create(:ci_pipeline_schedule) } + + let!(:pipeline_schedule_variables) do + create_list(:ci_pipeline_schedule_variable, 2, pipeline_schedule: pipeline_schedule) + end + + subject { pipeline_schedule.job_variables } + + before do + pipeline_schedule.reload + end + + it { is_expected.to contain_exactly(*pipeline_schedule_variables.map(&:to_runner_variable)) } + end end diff --git a/spec/models/ci/pipeline_schedule_variable_spec.rb b/spec/models/ci/pipeline_schedule_variable_spec.rb new file mode 100644 index 00000000000..0de76a57b7f --- /dev/null +++ b/spec/models/ci/pipeline_schedule_variable_spec.rb @@ -0,0 +1,7 @@ +require 'spec_helper' + +describe Ci::PipelineScheduleVariable, models: true do + subject { build(:ci_pipeline_schedule_variable) } + + it { is_expected.to include_module(HasVariable) } +end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index ea405fabff0..1eadc28869f 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -803,7 +803,7 @@ describe MergeRequest, models: true do 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_commit_shas).to eq(all_commit_shas) + expect(subject.all_commit_shas).to match_array(all_commit_shas) end end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 45953023a36..2649d04bee3 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -6,9 +6,6 @@ describe Milestone, models: true do allow(subject).to receive(:set_iid).and_return(false) end - it { is_expected.to validate_presence_of(:title) } - it { is_expected.to validate_presence_of(:project) } - describe 'start_date' do it 'adds an error when start_date is greated then due_date' do milestone = build(:milestone, start_date: Date.tomorrow, due_date: Date.yesterday) @@ -37,17 +34,42 @@ describe Milestone, models: true do end end - describe "unique milestone title per project" do - it "does not accept the same title in a project twice" do - new_milestone = Milestone.new(project: milestone.project, title: milestone.title) - expect(new_milestone).not_to be_valid + describe "unique milestone title" do + context "per project" do + it "does not accept the same title in a project twice" do + new_milestone = Milestone.new(project: milestone.project, title: milestone.title) + expect(new_milestone).not_to be_valid + end + + it "accepts the same title in another project" do + project = create(:empty_project) + new_milestone = Milestone.new(project: project, title: milestone.title) + + expect(new_milestone).to be_valid + end end - it "accepts the same title in another project" do - project = build(:empty_project) - new_milestone = Milestone.new(project: project, title: milestone.title) + context "per group" do + let(:group) { create(:group) } + let(:milestone) { create(:milestone, group: group) } + + before do + project.update(group: group) + end + + it "does not accept the same title in a group twice" do + new_milestone = Milestone.new(group: group, title: milestone.title) + + expect(new_milestone).not_to be_valid + end - expect(new_milestone).to be_valid + it "does not accept the same title of a child project milestone" do + create(:milestone, project: group.projects.first) + + new_milestone = Milestone.new(group: group, title: milestone.title) + + expect(new_milestone).not_to be_valid + end end end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 79cac721202..9b53164b4a2 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -772,7 +772,7 @@ describe API::Issues do end end - context 'CE restrictions' do + context 'single assignee restrictions' do it 'creates a new project issue with no more than one assignee' do post api("/projects/#{project.id}/issues", user), title: 'new issue', assignee_ids: [user2.id, guest.id] @@ -1123,7 +1123,7 @@ describe API::Issues do expect(json_response['assignees'].first['name']).to eq(user2.name) end - context 'CE restrictions' do + context 'single assignee restrictions' do it 'updates an issue with several assignees but only one has been applied' do put api("/projects/#{project.id}/issues/#{issue.iid}", user), assignee_ids: [user2.id, guest.id] diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 85d11deb26f..b34555d2815 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -279,6 +279,8 @@ describe API::PipelineSchedules do end context 'authenticated user with invalid permissions' do + let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: master) } + it 'does not delete pipeline_schedule' do delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", developer) diff --git a/spec/services/boards/create_service_spec.rb b/spec/services/boards/create_service_spec.rb index effa4633d13..89615df1692 100644 --- a/spec/services/boards/create_service_spec.rb +++ b/spec/services/boards/create_service_spec.rb @@ -26,6 +26,8 @@ describe Boards::CreateService, services: true do end it 'does not create a new board' do + expect(service).to receive(:can_create_board?) { false } + expect { service.execute }.not_to change(project.boards, :count) end end diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index d1dd1466d95..36d5038fb95 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -37,9 +37,6 @@ describe Issues::MoveService, services: true do describe '#execute' do shared_context 'issue move executed' do - let!(:milestone2) do - create(:milestone, project_id: new_project.id, title: 'v9.0') - end let!(:award_emoji) { create(:award_emoji, awardable: old_issue) } let!(:new_issue) { move_service.execute(old_issue, new_project) } @@ -48,6 +45,63 @@ describe Issues::MoveService, services: true do context 'issue movable' do include_context 'user can move issue' + context 'move to new milestone' do + let(:new_issue) { move_service.execute(old_issue, new_project) } + + context 'project milestone' do + let!(:milestone2) do + create(:milestone, project_id: new_project.id, title: 'v9.0') + end + + it 'assigns milestone to new issue' do + expect(new_issue.reload.milestone.title).to eq 'v9.0' + expect(new_issue.reload.milestone).to eq(milestone2) + end + end + + context 'group milestones' do + let!(:group) { create(:group, :private) } + let!(:group_milestone_1) do + create(:milestone, group_id: group.id, title: 'v9.0_group') + end + + before do + old_issue.update(milestone: group_milestone_1) + old_project.update(namespace: group) + new_project.update(namespace: group) + + group.add_users([user], GroupMember::DEVELOPER) + end + + context 'when moving to a project of the same group' do + it 'keeps the same group milestone' do + expect(new_issue.reload.project).to eq(new_project) + expect(new_issue.reload.milestone).to eq(group_milestone_1) + end + end + + context 'when moving to a project of a different group' do + let!(:group_2) { create(:group, :private) } + + let!(:group_milestone_2) do + create(:milestone, group_id: group_2.id, title: 'v9.0_group') + end + + before do + old_issue.update(milestone: group_milestone_1) + new_project.update(namespace: group_2) + + group_2.add_users([user], GroupMember::DEVELOPER) + end + + it 'assigns to new group milestone of same title' do + expect(new_issue.reload.project).to eq(new_project) + expect(new_issue.reload.milestone).to eq(group_milestone_2) + end + end + end + end + context 'generic issue' do include_context 'issue move executed' @@ -55,11 +109,6 @@ describe Issues::MoveService, services: true do expect(new_issue.project).to eq new_project end - it 'assigns milestone to new issue' do - expect(new_issue.reload.milestone.title).to eq 'v9.0' - expect(new_issue.reload.milestone).to eq(milestone2) - end - it 'assign labels to new issue' do expected_label_titles = new_issue.reload.labels.map(&:title) expect(expected_label_titles).to include 'label1' diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index c26642f5015..d0b991f19ab 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -253,13 +253,13 @@ describe Issues::UpdateService, services: true do end context 'when the milestone change' do - before do + it 'marks todos as done' do update_issue(milestone: create(:milestone)) - end - it 'marks todos as done' do expect(todo.reload.done?).to eq true end + + it_behaves_like 'system notes for milestones' end context 'when the labels change' do diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index ec15b5cac14..be62584ec0e 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -296,13 +296,13 @@ describe MergeRequests::UpdateService, services: true do end context 'when the milestone change' do - before do + it 'marks pending todos as done' do update_merge_request({ milestone: create(:milestone) }) - end - it 'marks pending todos as done' do expect(pending_todo.reload).to be_done end + + it_behaves_like 'system notes for milestones' end context 'when the labels change' do diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index 35373675894..a2db3f68ff7 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -431,22 +431,6 @@ describe QuickActions::InterpretService, services: true do end end - context 'reassign command' do - let(:content) { '/reassign' } - - context 'Issue' do - it 'reassigns user if content contains /reassign @user' do - user = create(:user) - - issue.update(assignee_ids: [developer.id]) - - _, updates = service.execute("/reassign @#{user.username}", issue) - - expect(updates).to eq(assignee_ids: [user.id]) - end - end - end - it_behaves_like 'milestone command' do let(:content) { "/milestone %#{milestone.title}" } let(:issuable) { issue } diff --git a/spec/support/issuable_shared_examples.rb b/spec/support/issuable_shared_examples.rb index 03011535351..970fe10db2b 100644 --- a/spec/support/issuable_shared_examples.rb +++ b/spec/support/issuable_shared_examples.rb @@ -5,3 +5,34 @@ shared_examples 'cache counters invalidator' do described_class.new(project, user, {}).execute(merge_request) end end + +shared_examples 'system notes for milestones' do + def update_issuable(opts) + issuable = try(:issue) || try(:merge_request) + described_class.new(project, user, opts).execute(issuable) + end + + context 'group milestones' do + let(:group) { create(:group) } + let(:group_milestone) { create(:milestone, group: group) } + + before do + project.update(namespace: group) + create(:group_member, group: group, user: user) + end + + it 'does not create system note' do + expect do + update_issuable(milestone: group_milestone) + end.not_to change { Note.system.count } + end + end + + context 'project milestones' do + it 'creates system note' do + expect do + update_issuable(milestone: create(:milestone)) + end.to change { Note.system.count }.by(1) + end + end +end diff --git a/spec/support/matchers/access_matchers_for_controller.rb b/spec/support/matchers/access_matchers_for_controller.rb index fb43f51c70c..ff60bd0c0ae 100644 --- a/spec/support/matchers/access_matchers_for_controller.rb +++ b/spec/support/matchers/access_matchers_for_controller.rb @@ -50,9 +50,24 @@ module AccessMatchersForController "be #{type} for #{role}. Expected: #{expected.join(',')} Got: #{result}" end + def update_owner(objects, user) + return unless objects + + objects.each do |object| + if object.respond_to?(:owner) + object.update_attribute(:owner, user) + elsif object.respond_to?(:user) + object.update_attribute(:user, user) + else + raise ArgumentError, "cannot own this object #{object}" + end + end + end + matcher :be_allowed_for do |role| match do |action| - emulate_user(role, @membership) + user = emulate_user(role, @membership) + update_owner(@objects, user) action.call EXPECTED_STATUS_CODE_ALLOWED.include?(response.status) @@ -62,13 +77,18 @@ module AccessMatchersForController @membership = membership end + chain :own do |*objects| + @objects = objects + end + description { description_for(role, 'allowed', EXPECTED_STATUS_CODE_ALLOWED, response.status) } supports_block_expectations end matcher :be_denied_for do |role| match do |action| - emulate_user(role, @membership) + user = emulate_user(role, @membership) + update_owner(@objects, user) action.call EXPECTED_STATUS_CODE_DENIED.include?(response.status) @@ -78,6 +98,10 @@ module AccessMatchersForController @membership = membership end + chain :own do |*objects| + @objects = objects + end + description { description_for(role, 'denied', EXPECTED_STATUS_CODE_DENIED, response.status) } supports_block_expectations end |