diff options
171 files changed, 2872 insertions, 656 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index 88345373a5b..b093d4d25d4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -5,7 +5,7 @@ require: inherit_from: .rubocop_todo.yml AllCops: - TargetRubyVersion: 2.1 + TargetRubyVersion: 2.3 # Cop names are not d§splayed in offense messages by default. Change behavior # by overriding DisplayCopNames, or by giving the -D/--display-cop-names # option. @@ -339,6 +339,10 @@ Style/OpMethod: Style/ParenthesesAroundCondition: Enabled: true +# Checks for an obsolete RuntimeException argument in raise/fail. +Style/RedundantException: + Enabled: true + # Checks for parentheses that seem not to serve any purpose. Style/RedundantParentheses: Enabled: true @@ -568,6 +572,10 @@ Lint/ElseLayout: Lint/EmptyEnsure: Enabled: true +# Checks for the presence of `when` branches without a body. +Lint/EmptyWhen: + Enabled: true + # Align ends correctly. Lint/EndAlignment: Enabled: true @@ -769,6 +777,10 @@ Rails/ScopeArgs: RSpec/AnyInstance: Enabled: false +# Check for expectations where `be(...)` can replace `eql(...)`. +RSpec/BeEql: + Enabled: false + # Check that the first argument to the top level describe is the tested class or # module. RSpec/DescribeClass: @@ -797,6 +809,10 @@ RSpec/ExampleWording: not: does not IgnoredWords: [] +# Checks for `expect(...)` calls containing literal values. +RSpec/ExpectActual: + Enabled: true + # Checks the file and folder naming of the spec file. RSpec/FilePath: Enabled: false diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index d581610162f..648b3fc49d2 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -21,10 +21,6 @@ Lint/AmbiguousRegexpLiteral: Lint/AssignmentInCondition: Enabled: false -# Offense count: 1 -Lint/EmptyWhen: - Enabled: false - # Offense count: 20 Lint/HandleExceptions: Enabled: false @@ -80,19 +76,11 @@ Performance/RedundantMatch: Performance/RedundantMerge: Enabled: false -# Offense count: 7 -RSpec/BeEql: - Enabled: false - # Offense count: 15 # Configuration parameters: CustomIncludeMethods. RSpec/EmptyExampleGroup: Enabled: false -# Offense count: 24 -RSpec/ExpectActual: - Enabled: false - # Offense count: 58 # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: implicit, each, example @@ -424,11 +412,6 @@ Style/RaiseArgs: Style/RedundantBegin: Enabled: false -# Offense count: 1 -# Cop supports --auto-correct. -Style/RedundantException: - Enabled: false - # Offense count: 29 # Cop supports --auto-correct. Style/RedundantFreeze: diff --git a/app/assets/javascripts/api.js b/app/assets/javascripts/api.js index b4a8c827d7f..84bbe90f3b1 100644 --- a/app/assets/javascripts/api.js +++ b/app/assets/javascripts/api.js @@ -11,7 +11,7 @@ licensePath: "/api/:version/templates/licenses/:key", gitignorePath: "/api/:version/templates/gitignores/:key", gitlabCiYmlPath: "/api/:version/templates/gitlab_ci_ymls/:key", - dockerfilePath: "/api/:version/dockerfiles/:key", + dockerfilePath: "/api/:version/templates/dockerfiles/:key", issuableTemplatePath: "/:namespace_path/:project_path/templates/:type/:key", group: function(group_id, callback) { var url = Api.buildUrl(Api.groupPath) diff --git a/app/assets/javascripts/cycle_analytics/components/stage_plan_component.js.es6 b/app/assets/javascripts/cycle_analytics/components/stage_plan_component.js.es6 index 513298ba4e7..8652479e7bf 100644 --- a/app/assets/javascripts/cycle_analytics/components/stage_plan_component.js.es6 +++ b/app/assets/javascripts/cycle_analytics/components/stage_plan_component.js.es6 @@ -13,6 +13,12 @@ <div> <div class="events-description"> {{ stage.description }} + <span v-if="items.length === 50" class="events-info pull-right"> + <i class="fa fa-warning has-tooltip" + title="Limited to showing 50 events at most" + data-placement="top"></i> + Showing 50 events + </span> </div> <ul class="stage-event-list"> <li v-for="commit in items" class="stage-event-item"> diff --git a/app/assets/javascripts/profile/profile.js.es6 b/app/assets/javascripts/profile/profile.js.es6 index 5aec9c813fe..81374296522 100644 --- a/app/assets/javascripts/profile/profile.js.es6 +++ b/app/assets/javascripts/profile/profile.js.es6 @@ -25,6 +25,7 @@ bindEvents() { $('.js-preferences-form').on('change.preference', 'input[type=radio]', this.submitForm); $('#user_notification_email').on('change', this.submitForm); + $('#user_notified_of_own_activity').on('change', this.submitForm); $('.update-username').on('ajax:before', this.beforeUpdateUsername); $('.update-username').on('ajax:complete', this.afterUpdateUsername); $('.update-notifications').on('ajax:success', this.onUpdateNotifs); diff --git a/app/assets/stylesheets/framework/animations.scss b/app/assets/stylesheets/framework/animations.scss index 0a26b4c6a8c..0ca5a9343f7 100644 --- a/app/assets/stylesheets/framework/animations.scss +++ b/app/assets/stylesheets/framework/animations.scss @@ -128,8 +128,7 @@ .note-action-button .link-highlight, .toolbar-btn, -.dropdown-toggle-caret, -.fa:not(.fa-bell) { +.dropdown-toggle-caret { @include transition(color); } diff --git a/app/assets/stylesheets/framework/avatar.scss b/app/assets/stylesheets/framework/avatar.scss index 1d59700543c..3f5b78ed445 100644 --- a/app/assets/stylesheets/framework/avatar.scss +++ b/app/assets/stylesheets/framework/avatar.scss @@ -28,6 +28,8 @@ .avatar { @extend .avatar-circle; + @include transition-property(none); + width: 40px; height: 40px; padding: 0; diff --git a/app/assets/stylesheets/mailers/highlighted_diff_email.scss b/app/assets/stylesheets/mailers/highlighted_diff_email.scss index 60ff72c703e..ea40f449134 100644 --- a/app/assets/stylesheets/mailers/highlighted_diff_email.scss +++ b/app/assets/stylesheets/mailers/highlighted_diff_email.scss @@ -138,6 +138,13 @@ pre { margin: 0; } +blockquote { + color: $gl-grayish-blue; + padding: 0 0 0 15px; + margin: 0; + border-left: 3px solid $white-dark; +} + span.highlight_word { background-color: $highlighted-highlight-word !important; } diff --git a/app/assets/stylesheets/pages/cycle_analytics.scss b/app/assets/stylesheets/pages/cycle_analytics.scss index cda069e6c0e..5b777953fb0 100644 --- a/app/assets/stylesheets/pages/cycle_analytics.scss +++ b/app/assets/stylesheets/pages/cycle_analytics.scss @@ -284,7 +284,11 @@ .events-description { line-height: 65px; - padding-left: $gl-padding; + padding: 0 $gl-padding; + } + + .events-info { + color: $gl-text-color-secondary; } } diff --git a/app/assets/stylesheets/pages/events.scss b/app/assets/stylesheets/pages/events.scss index b989d72ce1c..5776d86983a 100644 --- a/app/assets/stylesheets/pages/events.scss +++ b/app/assets/stylesheets/pages/events.scss @@ -41,7 +41,6 @@ word-wrap: break-word; .md { - color: $gl-grayish-blue; font-size: $gl-font-size; .label { diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index c02a65b0903..0b0c4bc130d 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -85,14 +85,18 @@ -webkit-align-items: center; align-items: center; + i, + svg { + margin-right: 8px; + } + svg { - margin-right: 4px; position: relative; top: 1px; overflow: visible; } - &> span { + & > span { padding-right: 4px; } diff --git a/app/assets/stylesheets/pages/wiki.scss b/app/assets/stylesheets/pages/wiki.scss index d5783e14b21..9bc47bbe173 100644 --- a/app/assets/stylesheets/pages/wiki.scss +++ b/app/assets/stylesheets/pages/wiki.scss @@ -1,3 +1,11 @@ +.new-wiki-page { + .new-wiki-page-slug-tip { + display: inline-block; + max-width: 100%; + margin-top: 5px; + } +} + .title .edit-wiki-header { width: 780px; margin-left: auto; @@ -9,12 +17,18 @@ @extend .top-area; position: relative; + .wiki-breadcrumb { + border-bottom: 1px solid $white-normal; + padding: 11px 0; + } + .wiki-page-title { margin: 0; font-size: 22px; } .wiki-last-edit-by { + display: block; color: $gl-text-color-secondary; strong { @@ -121,6 +135,10 @@ margin: 5px 0 10px; } + ul.wiki-pages ul { + padding-left: 15px; + } + .wiki-sidebar-header { padding: 0 $gl-padding $gl-padding; @@ -129,3 +147,15 @@ } } } + +ul.wiki-pages-list.content-list { + & ul { + list-style: none; + margin-left: 0; + padding-left: 15px; + } + + & ul li { + padding: 5px 0; + } +} diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index 6247934f81e..a6e158ebae6 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -9,6 +9,28 @@ module IssuableCollections private + def issuable_meta_data(issuable_collection) + # map has to be used here since using pluck or select will + # throw an error when ordering issuables by priority which inserts + # a new order into the collection. + # We cannot use reorder to not mess up the paginated collection. + issuable_ids = issuable_collection.map(&:id) + issuable_note_count = Note.count_for_collection(issuable_ids, @collection_type) + issuable_votes_count = AwardEmoji.votes_for_collection(issuable_ids, @collection_type) + + issuable_ids.each_with_object({}) do |id, issuable_meta| + downvotes = issuable_votes_count.find { |votes| votes.awardable_id == id && votes.downvote? } + upvotes = issuable_votes_count.find { |votes| votes.awardable_id == id && votes.upvote? } + notes = issuable_note_count.find { |notes| notes.noteable_id == id } + + issuable_meta[id] = Issuable::IssuableMeta.new( + upvotes.try(:count).to_i, + downvotes.try(:count).to_i, + notes.try(:count).to_i + ) + end + end + def issues_collection issues_finder.execute.preload(:project, :author, :assignee, :labels, :milestone, project: :namespace) end diff --git a/app/controllers/concerns/issues_action.rb b/app/controllers/concerns/issues_action.rb index b46adcceb60..fb5edb34370 100644 --- a/app/controllers/concerns/issues_action.rb +++ b/app/controllers/concerns/issues_action.rb @@ -9,6 +9,9 @@ module IssuesAction .non_archived .page(params[:page]) + @collection_type = "Issue" + @issuable_meta_data = issuable_meta_data(@issues) + respond_to do |format| format.html format.atom { render layout: false } diff --git a/app/controllers/concerns/merge_requests_action.rb b/app/controllers/concerns/merge_requests_action.rb index fdb05bb3228..6229759dcf1 100644 --- a/app/controllers/concerns/merge_requests_action.rb +++ b/app/controllers/concerns/merge_requests_action.rb @@ -7,6 +7,9 @@ module MergeRequestsAction @merge_requests = merge_requests_collection .page(params[:page]) + + @collection_type = "MergeRequest" + @issuable_meta_data = issuable_meta_data(@merge_requests) end private diff --git a/app/controllers/dashboard/projects_controller.rb b/app/controllers/dashboard/projects_controller.rb index 3ba8c2f8bb9..325ae565537 100644 --- a/app/controllers/dashboard/projects_controller.rb +++ b/app/controllers/dashboard/projects_controller.rb @@ -1,19 +1,14 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController include FilterProjects - before_action :event_filter - def index - @projects = current_user.authorized_projects.sorted_by_activity - @projects = filter_projects(@projects) - @projects = @projects.includes(:namespace) + @projects = load_projects(current_user.authorized_projects) @projects = @projects.sort(@sort = params[:sort]) @projects = @projects.page(params[:page]) respond_to do |format| format.html { @last_push = current_user.recent_push } format.atom do - event_filter load_events render layout: false end @@ -26,9 +21,8 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController end def starred - @projects = current_user.viewable_starred_projects.sorted_by_activity - @projects = filter_projects(@projects) - @projects = @projects.includes(:namespace, :forked_from_project, :tags) + @projects = load_projects(current_user.viewable_starred_projects) + @projects = @projects.includes(:forked_from_project, :tags) @projects = @projects.sort(@sort = params[:sort]) @projects = @projects.page(params[:page]) @@ -37,7 +31,6 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController respond_to do |format| format.html - format.json do render json: { html: view_to_html_string("dashboard/projects/_projects", locals: { projects: @projects }) @@ -48,9 +41,15 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController private + def load_projects(base_scope) + projects = base_scope.sorted_by_activity.includes(:namespace) + + filter_projects(projects) + end + def load_events - @events = Event.in_projects(@projects) - @events = @event_filter.apply_filter(@events).with_associations + @events = Event.in_projects(load_projects(current_user.authorized_projects)) + @events = event_filter.apply_filter(@events).with_associations @events = @events.limit(20).offset(params[:offset] || 0) end end diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb index b8b71d295f6..a271e2dfc4b 100644 --- a/app/controllers/profiles/notifications_controller.rb +++ b/app/controllers/profiles/notifications_controller.rb @@ -17,6 +17,6 @@ class Profiles::NotificationsController < Profiles::ApplicationController end def user_params - params.require(:user).permit(:notification_email) + params.require(:user).permit(:notification_email, :notified_of_own_activity) end end diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 4c39fe98028..a1db856dcfb 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -61,10 +61,10 @@ class Projects::BlobController < Projects::ApplicationController end def destroy - create_commit(Files::DeleteService, success_notice: "The file has been successfully deleted.", - success_path: namespace_project_tree_path(@project.namespace, @project, @target_branch), - failure_view: :show, - failure_path: namespace_project_blob_path(@project.namespace, @project, @id)) + create_commit(Files::DestroyService, success_notice: "The file has been successfully deleted.", + success_path: namespace_project_tree_path(@project.namespace, @project, @target_branch), + failure_view: :show, + failure_path: namespace_project_blob_path(@project.namespace, @project, @id)) end def diff diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index c75b8987a4b..744a4af1c51 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -23,8 +23,11 @@ class Projects::IssuesController < Projects::ApplicationController respond_to :html def index - @issues = issues_collection - @issues = @issues.page(params[:page]) + @collection_type = "Issue" + @issues = issues_collection + @issues = @issues.page(params[:page]) + @issuable_meta_data = issuable_meta_data(@issues) + if @issues.out_of_range? && @issues.total_pages != 0 return redirect_to url_for(params.merge(page: @issues.total_pages)) end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index fbad66c5c40..c3e1760f168 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -36,8 +36,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController before_action :authorize_can_resolve_conflicts!, only: [:conflicts, :conflict_for_path, :resolve_conflicts] def index - @merge_requests = merge_requests_collection - @merge_requests = @merge_requests.page(params[:page]) + @collection_type = "MergeRequest" + @merge_requests = merge_requests_collection + @merge_requests = @merge_requests.page(params[:page]) + @issuable_meta_data = issuable_meta_data(@merge_requests) + if @merge_requests.out_of_range? && @merge_requests.total_pages != 0 return redirect_to url_for(params.merge(page: @merge_requests.total_pages)) end diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index c3353446fd1..0faa71c4d7d 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -8,6 +8,7 @@ class Projects::WikisController < Projects::ApplicationController def pages @wiki_pages = Kaminari.paginate_array(@project_wiki.pages).page(params[:page]) + @wiki_entries = WikiPage.group_by_directory(@wiki_pages) end def show @@ -83,7 +84,7 @@ class Projects::WikisController < Projects::ApplicationController def destroy @page = @project_wiki.find_page(params[:id]) - @page.delete if @page + @page&.delete redirect_to( namespace_project_wiki_path(@project.namespace, @project, :home), @@ -116,7 +117,7 @@ class Projects::WikisController < Projects::ApplicationController # Call #wiki to make sure the Wiki Repo is initialized @project_wiki.wiki - @sidebar_wiki_pages = @project_wiki.pages.first(15) + @sidebar_wiki_entries = WikiPage.group_by_directory(@project_wiki.pages.first(15)) rescue ProjectWiki::CouldNotCreateWikiError flash[:notice] = "Could not create Wiki Repository at this time. Please try again later." redirect_to project_path(@project) diff --git a/app/helpers/builds_helper.rb b/app/helpers/builds_helper.rb index 9fc69e12266..ff937b5ebd2 100644 --- a/app/helpers/builds_helper.rb +++ b/app/helpers/builds_helper.rb @@ -1,7 +1,7 @@ module BuildsHelper def sidebar_build_class(build, current_build) build_class = '' - build_class += ' active' if build == current_build + build_class += ' active' if build.id === current_build.id build_class += ' retried' if build.retried? build_class end diff --git a/app/helpers/preferences_helper.rb b/app/helpers/preferences_helper.rb index 6e68aad4cb7..dd0a4ea03f0 100644 --- a/app/helpers/preferences_helper.rb +++ b/app/helpers/preferences_helper.rb @@ -23,7 +23,7 @@ module PreferencesHelper if defined.size != DASHBOARD_CHOICES.size # Ensure that anyone adding new options updates this method too - raise RuntimeError, "`User` defines #{defined.size} dashboard choices," + + raise "`User` defines #{defined.size} dashboard choices," \ " but `DASHBOARD_CHOICES` defined #{DASHBOARD_CHOICES.size}." else defined.map do |key, _| diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index d7d51c99979..845f1a0e840 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -15,6 +15,7 @@ module TodosHelper when Todo::MARKED then 'added a todo for' when Todo::APPROVAL_REQUIRED then 'set you as an approver for' when Todo::UNMERGEABLE then 'Could not merge' + when Todo::DIRECTLY_ADDRESSED then 'directly addressed you on' end end @@ -88,7 +89,8 @@ module TodosHelper { id: Todo::ASSIGNED, text: 'Assigned' }, { id: Todo::MENTIONED, text: 'Mentioned' }, { id: Todo::MARKED, text: 'Added' }, - { id: Todo::BUILD_FAILED, text: 'Pipelines' } + { id: Todo::BUILD_FAILED, text: 'Pipelines' }, + { id: Todo::DIRECTLY_ADDRESSED, text: 'Directly addressed' } ] end diff --git a/app/helpers/wiki_helper.rb b/app/helpers/wiki_helper.rb new file mode 100644 index 00000000000..3e3f6246fc5 --- /dev/null +++ b/app/helpers/wiki_helper.rb @@ -0,0 +1,13 @@ +module WikiHelper + # Produces a pure text breadcrumb for a given page. + # + # page_slug - The slug of a WikiPage object. + # + # Returns a String composed of the capitalized name of each directory and the + # capitalized name of the page itself. + def breadcrumb(page_slug) + page_slug.split('/'). + map { |dir_or_page| WikiPage.unhyphenize(dir_or_page).capitalize }. + join(' / ') + end +end diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 0cd3456b4de..5b9226a6b81 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -151,7 +151,7 @@ class Notify < BaseMailer headers['In-Reply-To'] = message_id(model) headers['References'] = message_id(model) - headers[:subject].prepend('Re: ') if headers[:subject] + headers[:subject]&.prepend('Re: ') mail_thread(model, headers) end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 9a4557524c4..74b358d8c40 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -116,31 +116,25 @@ class ApplicationSetting < ActiveRecord::Base numericality: { only_integer: true, greater_than_or_equal_to: 0 } validates_each :restricted_visibility_levels do |record, attr, value| - unless value.nil? - value.each do |level| - unless Gitlab::VisibilityLevel.options.has_value?(level) - record.errors.add(attr, "'#{level}' is not a valid visibility level") - end + value&.each do |level| + unless Gitlab::VisibilityLevel.options.has_value?(level) + record.errors.add(attr, "'#{level}' is not a valid visibility level") end end end validates_each :import_sources do |record, attr, value| - unless value.nil? - value.each do |source| - unless Gitlab::ImportSources.options.has_value?(source) - record.errors.add(attr, "'#{source}' is not a import source") - end + value&.each do |source| + unless Gitlab::ImportSources.options.has_value?(source) + record.errors.add(attr, "'#{source}' is not a import source") end end end validates_each :disabled_oauth_sign_in_sources do |record, attr, value| - unless value.nil? - value.each do |source| - unless Devise.omniauth_providers.include?(source.to_sym) - record.errors.add(attr, "'#{source}' is not an OAuth sign-in source") - end + value&.each do |source| + unless Devise.omniauth_providers.include?(source.to_sym) + record.errors.add(attr, "'#{source}' is not an OAuth sign-in source") end end end @@ -230,11 +224,11 @@ class ApplicationSetting < ActiveRecord::Base end def domain_whitelist_raw - self.domain_whitelist.join("\n") unless self.domain_whitelist.nil? + self.domain_whitelist&.join("\n") end def domain_blacklist_raw - self.domain_blacklist.join("\n") unless self.domain_blacklist.nil? + self.domain_blacklist&.join("\n") end def domain_whitelist_raw=(values) diff --git a/app/models/award_emoji.rb b/app/models/award_emoji.rb index 46b17479d6d..6937ad3bdd9 100644 --- a/app/models/award_emoji.rb +++ b/app/models/award_emoji.rb @@ -16,6 +16,14 @@ class AwardEmoji < ActiveRecord::Base scope :downvotes, -> { where(name: DOWNVOTE_NAME) } scope :upvotes, -> { where(name: UPVOTE_NAME) } + class << self + def votes_for_collection(ids, type) + select('name', 'awardable_id', 'COUNT(*) as count'). + where('name IN (?) AND awardable_type = ? AND awardable_id IN (?)', [DOWNVOTE_NAME, UPVOTE_NAME], type, ids). + group('name', 'awardable_id') + end + end + def downvote? self.name == DOWNVOTE_NAME end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 3517969eabc..5f53c48fc88 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -15,6 +15,11 @@ module Issuable include Taskable include TimeTrackable + # This object is used to gather issuable meta data for displaying + # upvotes, downvotes and notes count for issues and merge requests + # lists avoiding n+1 queries and improving performance. + IssuableMeta = Struct.new(:upvotes, :downvotes, :notes_count) + included do cache_markdown_field :title, pipeline: :single_line cache_markdown_field :description @@ -95,8 +100,8 @@ module Issuable def update_assignee_cache_counts # make sure we flush the cache for both the old *and* new assignees(if they exist) previous_assignee = User.find_by_id(assignee_id_was) if assignee_id_was - previous_assignee.update_cache_counts if previous_assignee - assignee.update_cache_counts if assignee + previous_assignee&.update_cache_counts + assignee&.update_cache_counts end # We want to use optimistic lock for cases when only title or description are involved diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index ef2c1e5d414..7e56e371b27 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -44,8 +44,15 @@ module Mentionable end def all_references(current_user = nil, extractor: nil) - extractor ||= Gitlab::ReferenceExtractor. - new(project, current_user) + # Use custom extractor if it's passed in the function parameters. + if extractor + @extractor = extractor + else + @extractor ||= Gitlab::ReferenceExtractor. + new(project, current_user) + + @extractor.reset_memoized_values + end self.class.mentionable_attrs.each do |attr, options| text = __send__(attr) @@ -55,16 +62,20 @@ module Mentionable skip_project_check: skip_project_check? ) - extractor.analyze(text, options) + @extractor.analyze(text, options) end - extractor + @extractor end def mentioned_users(current_user = nil) all_references(current_user).users end + def directly_addressed_users(current_user = nil) + all_references(current_user).directly_addressed_users + end + # Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference. def referenced_mentionables(current_user = self.author) refs = all_references(current_user) diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb index e9450dd0c26..f449229864d 100644 --- a/app/models/concerns/milestoneish.rb +++ b/app/models/concerns/milestoneish.rb @@ -73,7 +73,7 @@ module Milestoneish def memoize_per_user(user, method_name) @memoized ||= {} @memoized[method_name] ||= {} - @memoized[method_name][user.try!(:id)] ||= yield + @memoized[method_name][user&.id] ||= yield end # override in a class that includes this module to get a faster query diff --git a/app/models/directly_addressed_user.rb b/app/models/directly_addressed_user.rb new file mode 100644 index 00000000000..0d519c6ac22 --- /dev/null +++ b/app/models/directly_addressed_user.rb @@ -0,0 +1,7 @@ +class DirectlyAddressedUser + class << self + def reference_pattern + User.reference_pattern + end + end +end diff --git a/app/models/environment.rb b/app/models/environment.rb index 803060b3979..1a21b5e52b5 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -126,7 +126,7 @@ class Environment < ActiveRecord::Base return unless available? stop! - stop_action.play(current_user) if stop_action + stop_action&.play(current_user) end def actions_for(environment) diff --git a/app/models/event.rb b/app/models/event.rb index 2662f170765..cf89ac5207f 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -36,10 +36,10 @@ class Event < ActiveRecord::Base scope :code_push, -> { where(action: PUSHED) } scope :in_projects, ->(projects) do - where(project_id: projects.map(&:id)).recent + where(project_id: projects).recent end - scope :with_associations, -> { includes(project: :namespace) } + scope :with_associations, -> { includes(:author, :project, project: :namespace).preload(:target) } scope :for_milestone_id, ->(milestone_id) { where(target_type: "Milestone", target_id: milestone_id) } class << self diff --git a/app/models/group_milestone.rb b/app/models/group_milestone.rb index 7b6db2634b7..86d38e5468b 100644 --- a/app/models/group_milestone.rb +++ b/app/models/group_milestone.rb @@ -9,7 +9,7 @@ class GroupMilestone < GlobalMilestone def self.build(group, projects, title) super(projects, title).tap do |milestone| - milestone.group = group if milestone + milestone&.group = group end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index c0d4dd0197f..38646eba3ac 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -561,7 +561,7 @@ class MergeRequest < ActiveRecord::Base # Return the set of issues that will be closed if this merge request is accepted. def closes_issues(current_user = self.author) if target_branch == project.default_branch - messages = [description] + messages = [title, description] messages.concat(commits.map(&:safe_message)) if merge_request_diff Gitlab::ClosingIssueExtractor.new(project, current_user). @@ -575,7 +575,7 @@ class MergeRequest < ActiveRecord::Base return [] unless target_branch == project.default_branch ext = Gitlab::ReferenceExtractor.new(project, current_user) - ext.analyze(description) + ext.analyze("#{title}\n#{description}") ext.issues - closes_issues(current_user) end diff --git a/app/models/note.rb b/app/models/note.rb index bf090a0438c..029fe667a45 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -108,6 +108,12 @@ class Note < ActiveRecord::Base Discussion.for_diff_notes(active_notes). map { |d| [d.line_code, d] }.to_h end + + def count_for_collection(ids, type) + user.select('noteable_id', 'COUNT(*) as count'). + group(:noteable_id). + where(noteable_type: type, noteable_id: ids) + end end def cross_reference? diff --git a/app/models/project.rb b/app/models/project.rb index c17bcedf7b2..aa408b4556e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -469,7 +469,7 @@ class Project < ActiveRecord::Base def reset_cache_and_import_attrs ProjectCacheWorker.perform_async(self.id) - self.import_data.destroy if self.import_data + self.import_data&.destroy end def import_url=(value) diff --git a/app/models/repository.rb b/app/models/repository.rb index d2d92a064a4..56c582cd9be 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1230,6 +1230,14 @@ class Repository action[:content] end + detect = CharlockHolmes::EncodingDetector.new.detect(content) if content + + unless detect && detect[:type] == :binary + # When writing to the repo directly as we are doing here, + # the `core.autocrlf` config isn't taken into account. + content.gsub!("\r\n", "\n") if self.autocrlf + end + oid = rugged.write(content, :blob) index.add(path: path, oid: oid, mode: mode) diff --git a/app/models/todo.rb b/app/models/todo.rb index 2adf494ce11..3dda7948d0b 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -1,12 +1,13 @@ class Todo < ActiveRecord::Base include Sortable - ASSIGNED = 1 - MENTIONED = 2 - BUILD_FAILED = 3 - MARKED = 4 - APPROVAL_REQUIRED = 5 # This is an EE-only feature - UNMERGEABLE = 6 + ASSIGNED = 1 + MENTIONED = 2 + BUILD_FAILED = 3 + MARKED = 4 + APPROVAL_REQUIRED = 5 # This is an EE-only feature + UNMERGEABLE = 6 + DIRECTLY_ADDRESSED = 7 ACTION_NAMES = { ASSIGNED => :assigned, @@ -14,7 +15,8 @@ class Todo < ActiveRecord::Base BUILD_FAILED => :build_failed, MARKED => :marked, APPROVAL_REQUIRED => :approval_required, - UNMERGEABLE => :unmergeable + UNMERGEABLE => :unmergeable, + DIRECTLY_ADDRESSED => :directly_addressed } belongs_to :author, class_name: "User" diff --git a/app/models/user.rb b/app/models/user.rb index f9245cdb82a..1649bf04eaa 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -51,7 +51,12 @@ class User < ActiveRecord::Base has_one :namespace, -> { where type: nil }, dependent: :destroy, foreign_key: :owner_id # Profile - has_many :keys, dependent: :destroy + has_many :keys, -> do + type = Key.arel_table[:type] + where(type.not_eq('DeployKey').or(type.eq(nil))) + end, dependent: :destroy + has_many :deploy_keys, -> { where(type: 'DeployKey') }, dependent: :destroy + has_many :emails, dependent: :destroy has_many :personal_access_tokens, dependent: :destroy has_many :identities, dependent: :destroy, autosave: true @@ -314,7 +319,7 @@ class User < ActiveRecord::Base def find_by_personal_access_token(token_string) personal_access_token = PersonalAccessToken.active.find_by_token(token_string) if token_string - personal_access_token.user if personal_access_token + personal_access_token&.user end # Returns a user for the given SSH key. diff --git a/app/models/wiki_directory.rb b/app/models/wiki_directory.rb new file mode 100644 index 00000000000..9340fc2dbbe --- /dev/null +++ b/app/models/wiki_directory.rb @@ -0,0 +1,18 @@ +class WikiDirectory + include ActiveModel::Validations + + attr_accessor :slug, :pages + + validates :slug, presence: true + + def initialize(slug, pages = []) + @slug = slug + @pages = pages + end + + # Relative path to the partial to be used when rendering collections + # of this object. + def to_partial_path + 'projects/wikis/wiki_directory' + end +end diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index c3de278f5b7..6347b274341 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -12,6 +12,32 @@ class WikiPage ActiveModel::Name.new(self, nil, 'wiki') end + # Sorts and groups pages by directory. + # + # pages - an array of WikiPage objects. + # + # Returns an array of WikiPage and WikiDirectory objects. The entries are + # sorted by alphabetical order (directories and pages inside each directory). + # Pages at the root level come before everything. + def self.group_by_directory(pages) + return [] if pages.blank? + + pages.sort_by { |page| [page.directory, page.slug] }. + group_by(&:directory). + map do |dir, pages| + if dir.present? + WikiDirectory.new(dir, pages) + else + pages + end + end. + flatten + end + + def self.unhyphenize(name) + name.gsub(/-+/, ' ') + end + def to_key [:slug] end @@ -56,7 +82,7 @@ class WikiPage # The formatted title of this page. def title if @attributes[:title] - @attributes[:title].gsub(/-+/, ' ') + self.class.unhyphenize(@attributes[:title]) else "" end @@ -69,16 +95,17 @@ class WikiPage # The raw content of this page. def content - @attributes[:content] ||= if @page - @page.text_data - end + @attributes[:content] ||= @page&.text_data + end + + # The hierarchy of the directory this page is contained in. + def directory + wiki.page_title_and_dir(slug).last end # The processed/formatted content of this page. def formatted_content - @attributes[:formatted_content] ||= if @page - @page.formatted_data - end + @attributes[:formatted_content] ||= @page&.formatted_data end # The markup format for the page. @@ -174,6 +201,12 @@ class WikiPage end end + # Relative path to the partial to be used when rendering collections + # of this object. + def to_partial_path + 'projects/wikis/wiki_page' + end + private def set_attributes diff --git a/app/services/create_tag_service.rb b/app/services/create_tag_service.rb index fe9353afeb8..6c75d1f04ff 100644 --- a/app/services/create_tag_service.rb +++ b/app/services/create_tag_service.rb @@ -4,7 +4,7 @@ class CreateTagService < BaseService return error('Tag name invalid') unless valid_tag repository = project.repository - message.strip! if message + message&.strip! new_tag = nil diff --git a/app/services/delete_tag_service.rb b/app/services/delete_tag_service.rb index 9d4bffb93e9..eb726cb04b1 100644 --- a/app/services/delete_tag_service.rb +++ b/app/services/delete_tag_service.rb @@ -9,7 +9,7 @@ class DeleteTagService < BaseService if repository.rm_tag(current_user, tag_name) release = project.releases.find_by(tag: tag_name) - release.destroy if release + release&.destroy push_data = build_push_data(tag) EventCreateService.new.push(project, current_user, push_data) diff --git a/app/services/files/delete_service.rb b/app/services/files/destroy_service.rb index 50f0ffcac9f..c3be806a42d 100644 --- a/app/services/files/delete_service.rb +++ b/app/services/files/destroy_service.rb @@ -1,5 +1,5 @@ module Files - class DeleteService < Files::BaseService + class DestroyService < Files::BaseService def commit repository.remove_file( current_user, diff --git a/app/services/issues/build_service.rb b/app/services/issues/build_service.rb index a63982f60c8..7cd927d8005 100644 --- a/app/services/issues/build_service.rb +++ b/app/services/issues/build_service.rb @@ -44,7 +44,15 @@ module Issues end def issue_params - @issue_params ||= issue_params_with_info_from_merge_request.merge(params.slice(:title, :description)) + @issue_params ||= issue_params_with_info_from_merge_request.merge(whitelisted_issue_params) + end + + def whitelisted_issue_params + if can?(current_user, :admin_issue, project) + params.slice(:title, :description, :milestone_id) + else + params.slice(:title, :description) + end end end end diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index c9168f74249..961605a1005 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -15,7 +15,7 @@ module Issues def before_create(issuable) if @recaptcha_verified spam_log = current_user.spam_logs.find_by(id: @spam_log_id, title: issuable.title) - spam_log.update!(recaptcha_verified: true) if spam_log + spam_log&.update!(recaptcha_verified: true) else issuable.spam = spam_service.check(@api) issuable.spam_log = spam_service.spam_log diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index b4bfb0e5e8c..581d18032e6 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -144,7 +144,11 @@ module MergeRequests return unless @commits.present? merge_requests_for_source_branch.each do |merge_request| - wip_commit = @commits.detect(&:work_in_progress?) + commit_shas = merge_request.commits_sha + + wip_commit = @commits.detect do |commit| + commit.work_in_progress? && commit_shas.include?(commit.sha) + end if wip_commit && !merge_request.work_in_progress? merge_request.update(title: merge_request.wip_title) diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index b2cc39763f3..3734e3c4253 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -217,7 +217,7 @@ class NotificationService recipients = reject_unsubscribed_users(recipients, note.noteable) recipients = reject_users_without_access(recipients, note.noteable) - recipients.delete(note.author) + recipients.delete(note.author) unless note.author.notified_of_own_activity? recipients = recipients.uniq notify_method = "note_#{note.to_ability_name}_email".to_sym @@ -327,8 +327,9 @@ class NotificationService recipients ||= build_recipients( pipeline, pipeline.project, - nil, # The acting user, who won't be added to recipients - action: pipeline.status).map(&:notification_email) + pipeline.user, + action: pipeline.status, + skip_current_user: false).map(&:notification_email) if recipients.any? mailer.public_send(email_template, pipeline, recipients).deliver_later @@ -627,7 +628,7 @@ class NotificationService recipients = reject_unsubscribed_users(recipients, target) recipients = reject_users_without_access(recipients, target) - recipients.delete(current_user) if skip_current_user + recipients.delete(current_user) if skip_current_user && !current_user.notified_of_own_activity? recipients.uniq end @@ -636,7 +637,7 @@ class NotificationService recipients = add_labels_subscribers([], project, target, labels: labels) recipients = reject_unsubscribed_users(recipients, target) recipients = reject_users_without_access(recipients, target) - recipients.delete(current_user) + recipients.delete(current_user) unless current_user.notified_of_own_activity? recipients.uniq end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index c7cce0c55b9..6dc3d8c2416 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -97,7 +97,7 @@ module Projects @project.team << [current_user, :master, current_user] end - @project.group.refresh_members_authorized_projects if @project.group + @project.group&.refresh_members_authorized_projects end def skip_wiki? diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 1bd6ce416ab..8ab943f4639 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -243,6 +243,12 @@ class TodoService end def create_mention_todos(project, target, author, note = nil) + # Create Todos for directly addressed users + directly_addressed_users = filter_directly_addressed_users(project, note || target, author) + attributes = attributes_for_todo(project, target, author, Todo::DIRECTLY_ADDRESSED, note) + create_todos(directly_addressed_users, attributes) + + # Create Todos for mentioned users mentioned_users = filter_mentioned_users(project, note || target, author) attributes = attributes_for_todo(project, target, author, Todo::MENTIONED, note) create_todos(mentioned_users, attributes) @@ -282,10 +288,18 @@ class TodoService ) end + def filter_todo_users(users, project, target) + reject_users_without_access(users, project, target).uniq + end + def filter_mentioned_users(project, target, author) mentioned_users = target.mentioned_users(author) - mentioned_users = reject_users_without_access(mentioned_users, project, target) - mentioned_users.uniq + filter_todo_users(mentioned_users, project, target) + end + + def filter_directly_addressed_users(project, target, author) + directly_addressed_users = target.directly_addressed_users(author) + filter_todo_users(directly_addressed_users, project, target) end def reject_users_without_access(users, project, target) diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml index 5c5e5940365..51c4e8e5a73 100644 --- a/app/views/profiles/notifications/show.html.haml +++ b/app/views/profiles/notifications/show.html.haml @@ -34,6 +34,11 @@ .clearfix + = form_for @user, url: profile_notifications_path, method: :put do |f| + %label{ for: 'user_notified_of_own_activity' } + = f.check_box :notified_of_own_activity + %span Receive notifications about your own activity + %hr %h5 Groups (#{@group_notifications.count}) diff --git a/app/views/projects/diffs/_file_header.html.haml b/app/views/projects/diffs/_file_header.html.haml index 5b09b6907ab..1dbfe830d52 100644 --- a/app/views/projects/diffs/_file_header.html.haml +++ b/app/views/projects/diffs/_file_header.html.haml @@ -10,13 +10,13 @@ - if diff_file.renamed_file - old_path, new_path = mark_inline_diffs(diff_file.old_path, diff_file.new_path) - %strong.file-title-name.has-tooltip{ data: { title: old_path } } + %strong.file-title-name.has-tooltip{ data: { title: old_path, container: 'body' } } = old_path → - %strong.file-title-name.has-tooltip{ data: { title: new_path } } + %strong.file-title-name.has-tooltip{ data: { title: new_path, container: 'body' } } = new_path - else - %strong.file-title-name.has-tooltip{ data: { title: diff_file.new_path } } + %strong.file-title-name.has-tooltip{ data: { title: diff_file.new_path, container: 'body' } } = diff_file.new_path - if diff_file.deleted_file deleted diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml index 5c9839cb330..0e3902c066a 100644 --- a/app/views/projects/issues/_issue.html.haml +++ b/app/views/projects/issues/_issue.html.haml @@ -17,22 +17,7 @@ %li = link_to_member(@project, issue.assignee, name: false, title: "Assigned to :name") - - upvotes, downvotes = issue.upvotes, issue.downvotes - - if upvotes > 0 - %li - = icon('thumbs-up') - = upvotes - - - if downvotes > 0 - %li - = icon('thumbs-down') - = downvotes - - - note_count = issue.notes.user.count - %li - = link_to issue_path(issue, anchor: 'notes'), class: ('no-comments' if note_count.zero?) do - = icon('comments') - = note_count + = render 'shared/issuable_meta_data', issuable: issue .issue-info #{issuable_reference(issue)} · diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index a5fbe9d6128..11b7aaec704 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -29,22 +29,7 @@ %li = link_to_member(merge_request.source_project, merge_request.assignee, name: false, title: "Assigned to :name") - - upvotes, downvotes = merge_request.upvotes, merge_request.downvotes - - if upvotes > 0 - %li - = icon('thumbs-up') - = upvotes - - - if downvotes > 0 - %li - = icon('thumbs-down') - = downvotes - - - note_count = merge_request.related_notes.user.count - %li - = link_to merge_request_path(merge_request, anchor: 'notes'), class: ('no-comments' if note_count.zero?) do - = icon('comments') - = note_count + = render 'shared/issuable_meta_data', issuable: merge_request .merge-request-info #{issuable_reference(merge_request)} · diff --git a/app/views/projects/wikis/_new.html.haml b/app/views/projects/wikis/_new.html.haml index c74f53b4c39..3d33679f07d 100644 --- a/app/views/projects/wikis/_new.html.haml +++ b/app/views/projects/wikis/_new.html.haml @@ -13,5 +13,9 @@ = label_tag :new_wiki_path do %span Page slug = text_field_tag :new_wiki_path, nil, placeholder: 'how-to-setup', class: 'form-control', required: true, :'data-wikis-path' => namespace_project_wikis_path(@project.namespace, @project), autofocus: true + %span.new-wiki-page-slug-tip + = icon('lightbulb-o') + Tip: You can specify the full path for the new file. + We will automatically create any missing directories. .form-actions = button_tag 'Create Page', class: 'build-new-wiki btn btn-create' diff --git a/app/views/projects/wikis/_pages_wiki_page.html.haml b/app/views/projects/wikis/_pages_wiki_page.html.haml new file mode 100644 index 00000000000..6298cf6c8da --- /dev/null +++ b/app/views/projects/wikis/_pages_wiki_page.html.haml @@ -0,0 +1,5 @@ +%li + = link_to wiki_page.title, namespace_project_wiki_path(@project.namespace, @project, wiki_page) + %small (#{wiki_page.format}) + .pull-right + %small Last edited #{time_ago_with_tooltip(wiki_page.commit.authored_date)} diff --git a/app/views/projects/wikis/_sidebar.html.haml b/app/views/projects/wikis/_sidebar.html.haml index f115f60088c..8c582f747b3 100644 --- a/app/views/projects/wikis/_sidebar.html.haml +++ b/app/views/projects/wikis/_sidebar.html.haml @@ -12,10 +12,8 @@ .blocks-container .block.block-first %ul.wiki-pages - - @sidebar_wiki_pages.each do |wiki_page| - %li{ class: params[:id] == wiki_page.slug ? 'active' : '' } - = link_to namespace_project_wiki_path(@project.namespace, @project, wiki_page) do - = wiki_page.title.capitalize + = render @sidebar_wiki_entries, context: 'sidebar' + .block = link_to namespace_project_wikis_pages_path(@project.namespace, @project), class: 'btn btn-block' do More Pages diff --git a/app/views/projects/wikis/_sidebar_wiki_page.html.haml b/app/views/projects/wikis/_sidebar_wiki_page.html.haml new file mode 100644 index 00000000000..eb9bd14920d --- /dev/null +++ b/app/views/projects/wikis/_sidebar_wiki_page.html.haml @@ -0,0 +1,3 @@ +%li{ class: params[:id] == wiki_page.slug ? 'active' : '' } + = link_to namespace_project_wiki_path(@project.namespace, @project, wiki_page) do + = wiki_page.title.capitalize diff --git a/app/views/projects/wikis/_wiki_directory.html.haml b/app/views/projects/wikis/_wiki_directory.html.haml new file mode 100644 index 00000000000..0e5f32ed859 --- /dev/null +++ b/app/views/projects/wikis/_wiki_directory.html.haml @@ -0,0 +1,4 @@ +%li + = wiki_directory.slug + %ul + = render wiki_directory.pages, context: context diff --git a/app/views/projects/wikis/_wiki_page.html.haml b/app/views/projects/wikis/_wiki_page.html.haml new file mode 100644 index 00000000000..c84d06dad02 --- /dev/null +++ b/app/views/projects/wikis/_wiki_page.html.haml @@ -0,0 +1 @@ += render "#{context}_wiki_page", wiki_page: wiki_page diff --git a/app/views/projects/wikis/pages.html.haml b/app/views/projects/wikis/pages.html.haml index e1eaffc6884..5fba2b1a5ae 100644 --- a/app/views/projects/wikis/pages.html.haml +++ b/app/views/projects/wikis/pages.html.haml @@ -13,11 +13,7 @@ = icon('cloud-download') Clone repository - %ul.content-list - - @wiki_pages.each do |wiki_page| - %li - = link_to wiki_page.title, namespace_project_wiki_path(@project.namespace, @project, wiki_page) - %small (#{wiki_page.format}) - .pull-right - %small Last edited #{time_ago_with_tooltip(wiki_page.commit.authored_date)} + %ul.wiki-pages-list.content-list + = render @wiki_entries, context: 'pages' + = paginate @wiki_pages, theme: 'gitlab' diff --git a/app/views/projects/wikis/show.html.haml b/app/views/projects/wikis/show.html.haml index 1b6dceee241..3609461b721 100644 --- a/app/views/projects/wikis/show.html.haml +++ b/app/views/projects/wikis/show.html.haml @@ -6,9 +6,11 @@ %button.btn.btn-default.sidebar-toggle.js-sidebar-wiki-toggle{ role: "button", type: "button" } = icon('angle-double-left') + .wiki-breadcrumb + %span= breadcrumb(@page.slug) + .nav-text %h2.wiki-page-title= @page.title.capitalize - %span.wiki-last-edit-by Last edited by %strong diff --git a/app/views/shared/_issuable_meta_data.html.haml b/app/views/shared/_issuable_meta_data.html.haml new file mode 100644 index 00000000000..1264e524d86 --- /dev/null +++ b/app/views/shared/_issuable_meta_data.html.haml @@ -0,0 +1,19 @@ +- note_count = @issuable_meta_data[issuable.id].notes_count +- issue_votes = @issuable_meta_data[issuable.id] +- upvotes, downvotes = issue_votes.upvotes, issue_votes.downvotes +- issuable_url = @collection_type == "Issue" ? issue_path(issuable, anchor: 'notes') : merge_request_path(issuable, anchor: 'notes') + +- if upvotes > 0 + %li + = icon('thumbs-up') + = upvotes + +- if downvotes > 0 + %li + = icon('thumbs-down') + = downvotes + +%li + = link_to issuable_url, class: ('no-comments' if note_count.zero?) do + = icon('comments') + = note_count diff --git a/app/workers/authorized_projects_worker.rb b/app/workers/authorized_projects_worker.rb index 6abbb5a5250..0e20df506a3 100644 --- a/app/workers/authorized_projects_worker.rb +++ b/app/workers/authorized_projects_worker.rb @@ -16,6 +16,6 @@ class AuthorizedProjectsWorker def perform(user_id) user = User.find_by(id: user_id) - user.refresh_authorized_projects if user + user&.refresh_authorized_projects end end diff --git a/changelogs/unreleased/20732_member_exists_409.yml b/changelogs/unreleased/20732_member_exists_409.yml new file mode 100644 index 00000000000..135647c7ac3 --- /dev/null +++ b/changelogs/unreleased/20732_member_exists_409.yml @@ -0,0 +1,4 @@ +--- +title: 'Add member: Always return 409 when a member exists' +merge_request: +author: diff --git a/changelogs/unreleased/22818-licence-gitignore-and-yml-endpoints-removal.yml b/changelogs/unreleased/22818-licence-gitignore-and-yml-endpoints-removal.yml new file mode 100644 index 00000000000..05d5993ddf3 --- /dev/null +++ b/changelogs/unreleased/22818-licence-gitignore-and-yml-endpoints-removal.yml @@ -0,0 +1,4 @@ +--- +title: V3 deprecated templates endpoints removal +merge_request: 8853 +author: diff --git a/changelogs/unreleased/23535-folders-in-wiki-repository.yml b/changelogs/unreleased/23535-folders-in-wiki-repository.yml new file mode 100644 index 00000000000..05212b608d4 --- /dev/null +++ b/changelogs/unreleased/23535-folders-in-wiki-repository.yml @@ -0,0 +1,4 @@ +--- +title: Show directory hierarchy when listing wiki pages +merge_request: 8133 +author: Alex Braha Stoll diff --git a/changelogs/unreleased/24333-close-issues-with-merge-request-title-ui.yml b/changelogs/unreleased/24333-close-issues-with-merge-request-title-ui.yml new file mode 100644 index 00000000000..fa137a29cb4 --- /dev/null +++ b/changelogs/unreleased/24333-close-issues-with-merge-request-title-ui.yml @@ -0,0 +1,5 @@ +--- +title: Show Issues mentioned / being closed from a Merge Requests title below the + 'Accept Merge Request' button +merge_request: 9194 +author: Jan Christophersen diff --git a/changelogs/unreleased/24976-start-of-line-mention.yml b/changelogs/unreleased/24976-start-of-line-mention.yml new file mode 100644 index 00000000000..99208aac87c --- /dev/null +++ b/changelogs/unreleased/24976-start-of-line-mention.yml @@ -0,0 +1,4 @@ +--- +title: Added a feature to create a 'directly addressed' Todo when mentioned in the beginning of a line. +merge_request: 7926 +author: Ershad Kunnakkadan diff --git a/changelogs/unreleased/27395-reduce-group-activity-sql-queries-2.yml b/changelogs/unreleased/27395-reduce-group-activity-sql-queries-2.yml new file mode 100644 index 00000000000..f3ce1709518 --- /dev/null +++ b/changelogs/unreleased/27395-reduce-group-activity-sql-queries-2.yml @@ -0,0 +1,4 @@ +--- +title: Include :author, :project, and :target in Event.with_associations +merge_request: +author: diff --git a/changelogs/unreleased/27395-reduce-group-activity-sql-queries.yml b/changelogs/unreleased/27395-reduce-group-activity-sql-queries.yml new file mode 100644 index 00000000000..3f6d922f2a0 --- /dev/null +++ b/changelogs/unreleased/27395-reduce-group-activity-sql-queries.yml @@ -0,0 +1,4 @@ +--- +title: Don't instantiate AR objects in Event.in_projects +merge_request: +author: diff --git a/changelogs/unreleased/27480-deploy_keys_should_not_show_up_in_users_keys_list.yml b/changelogs/unreleased/27480-deploy_keys_should_not_show_up_in_users_keys_list.yml new file mode 100644 index 00000000000..6e9192cb632 --- /dev/null +++ b/changelogs/unreleased/27480-deploy_keys_should_not_show_up_in_users_keys_list.yml @@ -0,0 +1,4 @@ +--- +title: Do not display deploy keys in user's own ssh keys list +merge_request: 9024 +author: diff --git a/changelogs/unreleased/27608-fixes-markdown-in-activity-feed-is-light-gray.yml b/changelogs/unreleased/27608-fixes-markdown-in-activity-feed-is-light-gray.yml new file mode 100644 index 00000000000..8f297620e23 --- /dev/null +++ b/changelogs/unreleased/27608-fixes-markdown-in-activity-feed-is-light-gray.yml @@ -0,0 +1,4 @@ +--- +title: Fixes markdown in activity-feed is gray +merge_request: 9179 +author: diff --git a/changelogs/unreleased/27939-fix-current-build-arrow.yml b/changelogs/unreleased/27939-fix-current-build-arrow.yml new file mode 100644 index 00000000000..280ab090f2c --- /dev/null +++ b/changelogs/unreleased/27939-fix-current-build-arrow.yml @@ -0,0 +1,4 @@ +--- +title: Fix current build arrow indicator +merge_request: +author: diff --git a/changelogs/unreleased/27947-missing-margin-between-loading-icon-and-text-in-merge-request-widget.yml b/changelogs/unreleased/27947-missing-margin-between-loading-icon-and-text-in-merge-request-widget.yml new file mode 100644 index 00000000000..1dfabd3813b --- /dev/null +++ b/changelogs/unreleased/27947-missing-margin-between-loading-icon-and-text-in-merge-request-widget.yml @@ -0,0 +1,4 @@ +--- +title: Add space between text and loading icon in Megre Request Widget +merge_request: 9119 +author: diff --git a/changelogs/unreleased/28029-improve-blockquote-formatting-on-emails.yml b/changelogs/unreleased/28029-improve-blockquote-formatting-on-emails.yml new file mode 100644 index 00000000000..be2a0afbc52 --- /dev/null +++ b/changelogs/unreleased/28029-improve-blockquote-formatting-on-emails.yml @@ -0,0 +1,4 @@ +--- +title: Improve blockquote formatting in notification emails +merge_request: +author: diff --git a/changelogs/unreleased/28032-tooltips-file-name.yml b/changelogs/unreleased/28032-tooltips-file-name.yml new file mode 100644 index 00000000000..9fe11e7c2b6 --- /dev/null +++ b/changelogs/unreleased/28032-tooltips-file-name.yml @@ -0,0 +1,5 @@ +--- +title: Adds container to tooltip in order to make it work with overflow:hidden in + parent element +merge_request: +author: diff --git a/changelogs/unreleased/dynamic-header-fixture.yml b/changelogs/unreleased/dynamic-header-fixture.yml new file mode 100644 index 00000000000..9789a1999c8 --- /dev/null +++ b/changelogs/unreleased/dynamic-header-fixture.yml @@ -0,0 +1,4 @@ +--- +title: Replace static fixture for header_spec.js +merge_request: 9174 +author: winniehell diff --git a/changelogs/unreleased/fix-cycle-analytics-events-limit.yml b/changelogs/unreleased/fix-cycle-analytics-events-limit.yml new file mode 100644 index 00000000000..152b37ca430 --- /dev/null +++ b/changelogs/unreleased/fix-cycle-analytics-events-limit.yml @@ -0,0 +1,4 @@ +--- +title: Add limit to the number of events showed in cycle analytics +merge_request: +author: diff --git a/changelogs/unreleased/fix_issue_from_milestone.yml b/changelogs/unreleased/fix_issue_from_milestone.yml new file mode 100644 index 00000000000..02581e3ea09 --- /dev/null +++ b/changelogs/unreleased/fix_issue_from_milestone.yml @@ -0,0 +1,4 @@ +--- +title: fix milestone does not automatically assign when create issue from milestone +merge_request: +author: diff --git a/changelogs/unreleased/issue_25900.yml b/changelogs/unreleased/issue_25900.yml new file mode 100644 index 00000000000..b4b72b8a20c --- /dev/null +++ b/changelogs/unreleased/issue_25900.yml @@ -0,0 +1,4 @@ +--- +title: Gather issuable metadata to avoid n+1 queries on index view +merge_request: +author: diff --git a/changelogs/unreleased/merge-request-tabs-fixture.yml b/changelogs/unreleased/merge-request-tabs-fixture.yml new file mode 100644 index 00000000000..289cd7b604a --- /dev/null +++ b/changelogs/unreleased/merge-request-tabs-fixture.yml @@ -0,0 +1,4 @@ +--- +title: Replace static fixture for merge_request_tabs_spec.js +merge_request: 9172 +author: winniehell diff --git a/changelogs/unreleased/new-branch-fixture.yml b/changelogs/unreleased/new-branch-fixture.yml new file mode 100644 index 00000000000..ce5ed816102 --- /dev/null +++ b/changelogs/unreleased/new-branch-fixture.yml @@ -0,0 +1,4 @@ +--- +title: Replace static fixture for new_branch_spec.js +merge_request: 9131 +author: winniehell diff --git a/changelogs/unreleased/option-to-be-notified-of-own-activity.yml b/changelogs/unreleased/option-to-be-notified-of-own-activity.yml new file mode 100644 index 00000000000..c2e0410cc33 --- /dev/null +++ b/changelogs/unreleased/option-to-be-notified-of-own-activity.yml @@ -0,0 +1,4 @@ +--- +title: Add option to receive email notifications about your own activity +merge_request: 8836 +author: Richard Macklin diff --git a/changelogs/unreleased/rename_files_delete_service.yml b/changelogs/unreleased/rename_files_delete_service.yml new file mode 100644 index 00000000000..4de1c5b0d63 --- /dev/null +++ b/changelogs/unreleased/rename_files_delete_service.yml @@ -0,0 +1,4 @@ +--- +title: Rename Files::DeleteService to Files::DestroyService +merge_request: 9110 +author: dixpac diff --git a/changelogs/unreleased/sh-add-labels-index.yml b/changelogs/unreleased/sh-add-labels-index.yml new file mode 100644 index 00000000000..b948a75081c --- /dev/null +++ b/changelogs/unreleased/sh-add-labels-index.yml @@ -0,0 +1,4 @@ +--- +title: Add indices to improve loading of labels page +merge_request: +author: diff --git a/config/webpack.config.js b/config/webpack.config.js index 00f448c1fbb..2ac779c8511 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -117,7 +117,8 @@ if (IS_PRODUCTION) { if (IS_DEV_SERVER) { config.devServer = { port: DEV_SERVER_PORT, - headers: { 'Access-Control-Allow-Origin': '*' } + headers: { 'Access-Control-Allow-Origin': '*' }, + stats: 'errors-only', }; config.output.publicPath = '//localhost:' + DEV_SERVER_PORT + config.output.publicPath; } diff --git a/db/migrate/20160919145149_add_group_id_to_labels.rb b/db/migrate/20160919145149_add_group_id_to_labels.rb index 05e21af0584..d10f3a6d104 100644 --- a/db/migrate/20160919145149_add_group_id_to_labels.rb +++ b/db/migrate/20160919145149_add_group_id_to_labels.rb @@ -7,7 +7,7 @@ class AddGroupIdToLabels < ActiveRecord::Migration def change add_column :labels, :group_id, :integer - add_foreign_key :labels, :namespaces, column: :group_id, on_delete: :cascade + add_foreign_key :labels, :namespaces, column: :group_id, on_delete: :cascade # rubocop: disable Migration/AddConcurrentForeignKey add_concurrent_index :labels, :group_id end end diff --git a/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb b/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb index f49df6802a7..2abfe47b776 100644 --- a/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb +++ b/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb @@ -28,6 +28,6 @@ class AddPipelineIdToMergeRequestMetrics < ActiveRecord::Migration def change add_column :merge_request_metrics, :pipeline_id, :integer add_concurrent_index :merge_request_metrics, :pipeline_id - add_foreign_key :merge_request_metrics, :ci_commits, column: :pipeline_id, on_delete: :cascade + add_foreign_key :merge_request_metrics, :ci_commits, column: :pipeline_id, on_delete: :cascade # rubocop: disable Migration/AddConcurrentForeignKey end end diff --git a/db/migrate/20161031171301_add_project_id_to_subscriptions.rb b/db/migrate/20161031171301_add_project_id_to_subscriptions.rb index 97534679b59..d5c343dc527 100644 --- a/db/migrate/20161031171301_add_project_id_to_subscriptions.rb +++ b/db/migrate/20161031171301_add_project_id_to_subscriptions.rb @@ -5,7 +5,7 @@ class AddProjectIdToSubscriptions < ActiveRecord::Migration def up add_column :subscriptions, :project_id, :integer - add_foreign_key :subscriptions, :projects, column: :project_id, on_delete: :cascade + add_foreign_key :subscriptions, :projects, column: :project_id, on_delete: :cascade # rubocop: disable Migration/AddConcurrentForeignKey end def down diff --git a/db/migrate/20170123061730_add_notified_of_own_activity_to_users.rb b/db/migrate/20170123061730_add_notified_of_own_activity_to_users.rb new file mode 100644 index 00000000000..f90637e1e35 --- /dev/null +++ b/db/migrate/20170123061730_add_notified_of_own_activity_to_users.rb @@ -0,0 +1,14 @@ +class AddNotifiedOfOwnActivityToUsers < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + + DOWNTIME = false + + def up + add_column_with_default :users, :notified_of_own_activity, :boolean, default: false + end + + def down + remove_column :users, :notified_of_own_activity + end +end diff --git a/db/migrate/20170210062829_add_index_to_labels_for_title_and_project.rb b/db/migrate/20170210062829_add_index_to_labels_for_title_and_project.rb new file mode 100644 index 00000000000..f922ed209aa --- /dev/null +++ b/db/migrate/20170210062829_add_index_to_labels_for_title_and_project.rb @@ -0,0 +1,12 @@ +class AddIndexToLabelsForTitleAndProject < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def change + add_concurrent_index :labels, :title + add_concurrent_index :labels, :project_id + end +end diff --git a/db/schema.rb b/db/schema.rb index d71911eaf14..d421d5c6774 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -581,6 +581,8 @@ ActiveRecord::Schema.define(version: 20170210075922) do add_index "labels", ["group_id", "project_id", "title"], name: "index_labels_on_group_id_and_project_id_and_title", unique: true, using: :btree add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree + add_index "labels", ["project_id"], name: "index_labels_on_project_id", using: :btree + add_index "labels", ["title"], name: "index_labels_on_title", using: :btree create_table "lfs_objects", force: :cascade do |t| t.string "oid", null: false @@ -1277,6 +1279,7 @@ ActiveRecord::Schema.define(version: 20170210075922) do t.string "organization" t.string "incoming_email_token" t.boolean "authorized_projects_populated" + t.boolean "notified_of_own_activity", default: false, null: false end add_index "users", ["admin"], name: "index_users_on_admin", using: :btree diff --git a/doc/api/v3_to_v4.md b/doc/api/v3_to_v4.md index 707f0437b7e..0ae07b5d3de 100644 --- a/doc/api/v3_to_v4.md +++ b/doc/api/v3_to_v4.md @@ -12,3 +12,14 @@ changes are in V4: - Endpoints under `projects/merge_request/:id` have been removed (use: `projects/merge_requests/:id`) - Project snippets do not return deprecated field `expires_at` - Endpoints under `projects/:id/keys` have been removed (use `projects/:id/deploy_keys`) +- Status 409 returned for POST `project/:id/members` when a member already exists +- Removed the following deprecated Templates endpoints (these are still accessible with `/templates` prefix) + - `/licences` + - `/licences/:key` + - `/gitignores` + - `/gitlab_ci_ymls` + - `/dockerfiles` + - `/gitignores/:key` + - `/gitlab_ci_ymls/:key` + - `/dockerfiles/:key` + diff --git a/doc/user/project/img/issue_board.png b/doc/user/project/img/issue_board.png Binary files differindex 95e8532e908..b636cb294b8 100644 --- a/doc/user/project/img/issue_board.png +++ b/doc/user/project/img/issue_board.png diff --git a/doc/user/project/img/issue_board_search_backlog.png b/doc/user/project/img/issue_board_search_backlog.png Binary files differdeleted file mode 100644 index fbb67b9c18f..00000000000 --- a/doc/user/project/img/issue_board_search_backlog.png +++ /dev/null diff --git a/doc/user/project/img/issue_board_welcome_message.png b/doc/user/project/img/issue_board_welcome_message.png Binary files differindex 5bfdac88dde..5318e6ea4a9 100644 --- a/doc/user/project/img/issue_board_welcome_message.png +++ b/doc/user/project/img/issue_board_welcome_message.png diff --git a/doc/user/project/img/issue_boards_add_issues_modal.png b/doc/user/project/img/issue_boards_add_issues_modal.png Binary files differnew file mode 100644 index 00000000000..33049dce74f --- /dev/null +++ b/doc/user/project/img/issue_boards_add_issues_modal.png diff --git a/doc/user/project/img/issue_boards_remove_issue.png b/doc/user/project/img/issue_boards_remove_issue.png Binary files differnew file mode 100644 index 00000000000..8b3beca97cf --- /dev/null +++ b/doc/user/project/img/issue_boards_remove_issue.png diff --git a/doc/user/project/issue_board.md b/doc/user/project/issue_board.md index d1ae57c00d7..3199d370a58 100644 --- a/doc/user/project/issue_board.md +++ b/doc/user/project/issue_board.md @@ -1,6 +1,8 @@ # Issue board -> [Introduced][ce-5554] in GitLab 8.11. +>**Notes:** +- [Introduced][ce-5554] in GitLab 8.11. +- The Backlog column was replaced by the **Add issues** button in GitLab 8.17. The GitLab Issue Board is a software project management tool used to plan, organize, and visualize a workflow for a feature or product release. @@ -28,13 +30,11 @@ Below is a table of the definitions used for GitLab's Issue Board. | **List** | Each label that exists in the issue tracker can have its own dedicated list. Every list is named after the label it is based on and is represented by a column which contains all the issues associated with that label. You can think of a list like the results you get when you filter the issues by a label in your issue tracker. | | **Card** | Every card represents an issue and it is shown under the list for which it has a label. The information you can see on a card consists of the issue number, the issue title, the assignee and the labels associated with it. You can drag cards around from one list to another. Issues inside lists are [ordered by priority](labels.md#prioritize-labels). | -There are three types of lists, the ones you create based on your labels, and -two default: +There are two types of lists, the ones you create based on your labels, and +one default: -- **Backlog** (default): shows all issues that do not fall in one of the other lists. Always appears on the very left. -- **Done** (default): shows all closed issues. Always appears on the very right. -Label list: a list based on a label. It shows all issues with that label. - Label list: a list based on a label. It shows all opened issues with that label. +- **Done** (default): shows all closed issues. Always appears on the very right.  @@ -55,10 +55,10 @@ In short, here's a list of actions you can take in an Issue Board: If you are not able to perform one or more of the things above, make sure you have the right [permissions](#permissions). -## First time using the Issue Board +## First time using the issue board -The first time you navigate to your Issue Board, you will be presented with the -two default lists (**Backlog** and **Done**) and a welcoming message that gives +The first time you navigate to your Issue Board, you will be presented with +a default list (**Done**) and a welcoming message that gives you two options. You can either create a predefined set of labels and create their corresponding lists to the Issue Board or opt-out and use your own lists. @@ -93,23 +93,26 @@ in the list's heading. A confirmation dialog will appear for you to confirm. Deleting a list doesn't have any effect in issues and labels, it's just the list view that is removed. You can always add it back later if you need. -## Searching issues in the Backlog list +## Adding issues to a list + +You can add issues to a list by clicking the **Add issues** button that is +present in the upper right corner of the issue board. This will open up a modal +window where you can see all the issues that do not belong to any list. + +Select one or more issues by clicking on the cards and then click **Add issues** +to add them to the selected list. You can limit the issues you want to add to +the list by filtering by author, assignee, milestone and label. -The very first time you start using the Issue Board, it is very likely your -issue tracker is already populated with labels and issues. In that case, -**Backlog** will have all the issues that don't belong to another list, and -**Done** will have all the closed ones. + -For performance and visibility reasons, each list shows the first 20 issues -by default. If you have more than 20, you have to start scrolling down for the -next 20 issues to appear. This can be cumbersome if your issue tracker hosts -hundreds of issues, and for that reason it is easier to search for issues to -move from **Backlog** to another list. +## Removing an issue from a list -Start typing in the search bar under the **Backlog** list and the relevant -issues will appear. +Removing an issue from a list can be done by clicking on the issue card and then +clicking the **Remove from board** button in the sidebar. Under the hood, the +respective label is removed, and as such it's also removed from the list and the +board itself. - + ## Filtering issues @@ -142,8 +145,8 @@ A typical workflow of using the Issue Board would be: and gets automatically closed. For instance you can create a list based on the label of 'Frontend' and one for -'Backend'. A designer can start working on an issue by dragging it from -**Backlog** to 'Frontend'. That way, everyone knows that this issue is now being +'Backend'. A designer can start working on an issue by adding it to the +'Frontend' list. That way, everyone knows that this issue is now being worked on by the designers. Then, once they're done, all they have to do is drag it over to the next list, 'Backend', where a backend developer can eventually pick it up. Once they’re done, they move it to **Done**, to close the diff --git a/doc/user/project/merge_requests.md b/doc/user/project/merge_requests.md index be09337319f..84a79f04094 100644 --- a/doc/user/project/merge_requests.md +++ b/doc/user/project/merge_requests.md @@ -1,169 +1 @@ -# Merge Requests - -Merge requests allow you to exchange changes you made to source code and -collaborate with other people on the same project. - -## Authorization for merge requests - -There are two main ways to have a merge request flow with GitLab: - -1. Working with [protected branches][] in a single repository -1. Working with forks of an authoritative project - -[Learn more about the authorization for merge requests.](merge_requests/authorization_for_merge_requests.md) - -## Cherry-pick changes - -Cherry-pick any commit in the UI by simply clicking the **Cherry-pick** button -in a merged merge requests or a commit. - -[Learn more about cherry-picking changes.](merge_requests/cherry_pick_changes.md) - -## Merge when pipeline succeeds - -When reviewing a merge request that looks ready to merge but still has one or -more CI builds running, you can set it to be merged automatically when CI -pipeline succeeds. This way, you don't have to wait for the pipeline to finish -and remember to merge the request manually. - -[Learn more about merging when pipeline succeeds.](merge_requests/merge_when_pipeline_succeeds.md) - -## Resolve discussion comments in merge requests reviews - -Keep track of the progress during a code review with resolving comments. -Resolving comments prevents you from forgetting to address feedback and lets -you hide discussions that are no longer relevant. - -[Read more about resolving discussion comments in merge requests reviews.](merge_requests/merge_request_discussion_resolution.md) - -## Resolve conflicts - -When a merge request has conflicts, GitLab may provide the option to resolve -those conflicts in the GitLab UI. - -[Learn more about resolving merge conflicts in the UI.](merge_requests/resolve_conflicts.md) - -## Revert changes - -GitLab implements Git's powerful feature to revert any commit with introducing -a **Revert** button in merge requests and commit details. - -[Learn more about reverting changes in the UI](merge_requests/revert_changes.md) - -## Merge requests versions - -Every time you push to a branch that is tied to a merge request, a new version -of merge request diff is created. When you visit a merge request that contains -more than one pushes, you can select and compare the versions of those merge -request diffs. - -[Read more about the merge requests versions.](merge_requests/versions.md) - -## Work In Progress merge requests - -To prevent merge requests from accidentally being accepted before they're -completely ready, GitLab blocks the "Accept" button for merge requests that -have been marked as a **Work In Progress**. - -[Learn more about settings a merge request as "Work In Progress".](merge_requests/work_in_progress_merge_requests.md) - -## Ignore whitespace changes in Merge Request diff view - -If you click the **Hide whitespace changes** button, you can see the diff -without whitespace changes (if there are any). This is also working when on a -specific commit page. - - - ->**Tip:** -You can append `?w=1` while on the diffs page of a merge request to ignore any -whitespace changes. - -## Tips - -Here are some tips that will help you be more efficient with merge requests in -the command line. - -> **Note:** -This section might move in its own document in the future. - -### Checkout merge requests locally - -A merge request contains all the history from a repository, plus the additional -commits added to the branch associated with the merge request. Here's a few -tricks to checkout a merge request locally. - -Please note that you can checkout a merge request locally even if the source -project is a fork (even a private fork) of the target project. - -#### Checkout locally by adding a git alias - -Add the following alias to your `~/.gitconfig`: - -``` -[alias] - mr = !sh -c 'git fetch $1 merge-requests/$2/head:mr-$1-$2 && git checkout mr-$1-$2' - -``` - -Now you can check out a particular merge request from any repository and any -remote. For example, to check out the merge request with ID 5 as shown in GitLab -from the `upstream` remote, do: - -``` -git mr upstream 5 -``` - -This will fetch the merge request into a local `mr-upstream-5` branch and check -it out. - -#### Checkout locally by modifying `.git/config` for a given repository - -Locate the section for your GitLab remote in the `.git/config` file. It looks -like this: - -``` -[remote "origin"] - url = https://gitlab.com/gitlab-org/gitlab-ce.git - fetch = +refs/heads/*:refs/remotes/origin/* -``` - -You can open the file with: - -``` -git config -e -``` - -Now add the following line to the above section: - -``` -fetch = +refs/merge-requests/*/head:refs/remotes/origin/merge-requests/* -``` - -In the end, it should look like this: - -``` -[remote "origin"] - url = https://gitlab.com/gitlab-org/gitlab-ce.git - fetch = +refs/heads/*:refs/remotes/origin/* - fetch = +refs/merge-requests/*/head:refs/remotes/origin/merge-requests/* -``` - -Now you can fetch all the merge requests: - -``` -git fetch origin - -... -From https://gitlab.com/gitlab-org/gitlab-ce.git - * [new ref] refs/merge-requests/1/head -> origin/merge-requests/1 - * [new ref] refs/merge-requests/2/head -> origin/merge-requests/2 -... -``` - -And to check out a particular merge request: - -``` -git checkout origin/merge-requests/1 -``` - -[protected branches]: protected_branches.md +This document was moved to [merge_requests/index.md](merge_requests/index.md). diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md new file mode 100644 index 00000000000..a27e683143d --- /dev/null +++ b/doc/user/project/merge_requests/index.md @@ -0,0 +1,169 @@ +# Merge requests + +Merge requests allow you to exchange changes you made to source code and +collaborate with other people on the same project. + +## Authorization for merge requests + +There are two main ways to have a merge request flow with GitLab: + +1. Working with [protected branches][] in a single repository +1. Working with forks of an authoritative project + +[Learn more about the authorization for merge requests.](authorization_for_merge_requests.md) + +## Cherry-pick changes + +Cherry-pick any commit in the UI by simply clicking the **Cherry-pick** button +in a merged merge requests or a commit. + +[Learn more about cherry-picking changes.](cherry_pick_changes.md) + +## Merge when pipeline succeeds + +When reviewing a merge request that looks ready to merge but still has one or +more CI builds running, you can set it to be merged automatically when CI +pipeline succeeds. This way, you don't have to wait for the pipeline to finish +and remember to merge the request manually. + +[Learn more about merging when pipeline succeeds.](merge_when_pipeline_succeeds.md) + +## Resolve discussion comments in merge requests reviews + +Keep track of the progress during a code review with resolving comments. +Resolving comments prevents you from forgetting to address feedback and lets +you hide discussions that are no longer relevant. + +[Read more about resolving discussion comments in merge requests reviews.](merge_request_discussion_resolution.md) + +## Resolve conflicts + +When a merge request has conflicts, GitLab may provide the option to resolve +those conflicts in the GitLab UI. + +[Learn more about resolving merge conflicts in the UI.](resolve_conflicts.md) + +## Revert changes + +GitLab implements Git's powerful feature to revert any commit with introducing +a **Revert** button in merge requests and commit details. + +[Learn more about reverting changes in the UI](revert_changes.md) + +## Merge requests versions + +Every time you push to a branch that is tied to a merge request, a new version +of merge request diff is created. When you visit a merge request that contains +more than one pushes, you can select and compare the versions of those merge +request diffs. + +[Read more about the merge requests versions.](versions.md) + +## Work In Progress merge requests + +To prevent merge requests from accidentally being accepted before they're +completely ready, GitLab blocks the "Accept" button for merge requests that +have been marked as a **Work In Progress**. + +[Learn more about settings a merge request as "Work In Progress".](work_in_progress_merge_requests.md) + +## Ignore whitespace changes in Merge Request diff view + +If you click the **Hide whitespace changes** button, you can see the diff +without whitespace changes (if there are any). This is also working when on a +specific commit page. + + + +>**Tip:** +You can append `?w=1` while on the diffs page of a merge request to ignore any +whitespace changes. + +## Tips + +Here are some tips that will help you be more efficient with merge requests in +the command line. + +> **Note:** +This section might move in its own document in the future. + +### Checkout merge requests locally + +A merge request contains all the history from a repository, plus the additional +commits added to the branch associated with the merge request. Here's a few +tricks to checkout a merge request locally. + +Please note that you can checkout a merge request locally even if the source +project is a fork (even a private fork) of the target project. + +#### Checkout locally by adding a git alias + +Add the following alias to your `~/.gitconfig`: + +``` +[alias] + mr = !sh -c 'git fetch $1 merge-requests/$2/head:mr-$1-$2 && git checkout mr-$1-$2' - +``` + +Now you can check out a particular merge request from any repository and any +remote. For example, to check out the merge request with ID 5 as shown in GitLab +from the `upstream` remote, do: + +``` +git mr upstream 5 +``` + +This will fetch the merge request into a local `mr-upstream-5` branch and check +it out. + +#### Checkout locally by modifying `.git/config` for a given repository + +Locate the section for your GitLab remote in the `.git/config` file. It looks +like this: + +``` +[remote "origin"] + url = https://gitlab.com/gitlab-org/gitlab-ce.git + fetch = +refs/heads/*:refs/remotes/origin/* +``` + +You can open the file with: + +``` +git config -e +``` + +Now add the following line to the above section: + +``` +fetch = +refs/merge-requests/*/head:refs/remotes/origin/merge-requests/* +``` + +In the end, it should look like this: + +``` +[remote "origin"] + url = https://gitlab.com/gitlab-org/gitlab-ce.git + fetch = +refs/heads/*:refs/remotes/origin/* + fetch = +refs/merge-requests/*/head:refs/remotes/origin/merge-requests/* +``` + +Now you can fetch all the merge requests: + +``` +git fetch origin + +... +From https://gitlab.com/gitlab-org/gitlab-ce.git + * [new ref] refs/merge-requests/1/head -> origin/merge-requests/1 + * [new ref] refs/merge-requests/2/head -> origin/merge-requests/2 +... +``` + +And to check out a particular merge request: + +``` +git checkout origin/merge-requests/1 +``` + +[protected branches]: protected_branches.md diff --git a/doc/user/project/merge_requests/versions.md b/doc/user/project/merge_requests/versions.md index 77eab7ba5e3..610250ccf12 100644 --- a/doc/user/project/merge_requests/versions.md +++ b/doc/user/project/merge_requests/versions.md @@ -1,6 +1,12 @@ # Merge requests versions -> Will be [introduced][ce-5467] in GitLab 8.12. +>**Notes:** +- [Introduced][ce-5467] in GitLab 8.12. +- Comments are disabled while viewing outdated merge versions or comparing to + versions other than base. +- Merge request versions are based on push not on commit. So, if you pushed 5 + commits in a single push, it will be a single option in the dropdown. If you + pushed 5 times, that will count for 5 options. Every time you push to a branch that is tied to a merge request, a new version of merge request diff is created. When you visit a merge request that contains @@ -30,13 +36,4 @@ changes appears as a system note.  ---- - ->**Notes:** -- Comments are disabled while viewing outdated merge versions or comparing to - versions other than base. -- Merge request versions are based on push not on commit. So, if you pushed 5 - commits in a single push, it will be a single option in the dropdown. If you - pushed 5 times, that will count for 5 options. - [ce-5467]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5467 diff --git a/doc/workflow/README.md b/doc/workflow/README.md index 0b6f00c6aa4..7a97b87f1c5 100644 --- a/doc/workflow/README.md +++ b/doc/workflow/README.md @@ -27,7 +27,7 @@ - [Web Editor](../user/project/repository/web_editor.md) - [Releases](releases.md) - [Milestones](milestones.md) -- [Merge Requests](../user/project/merge_requests.md) +- [Merge Requests](../user/project/merge_requests/index.md) - [Authorization for merge requests](../user/project/merge_requests/authorization_for_merge_requests.md) - [Cherry-pick changes](../user/project/merge_requests/cherry_pick_changes.md) - [Merge when pipeline succeeds](../user/project/merge_requests/merge_when_pipeline_succeeds.md) diff --git a/lib/api/api.rb b/lib/api/api.rb index eb9792680ff..06346ae822a 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -7,9 +7,11 @@ module API version 'v3', using: :path do mount ::API::V3::DeployKeys mount ::API::V3::Issues + mount ::API::V3::Members mount ::API::V3::MergeRequests mount ::API::V3::Projects mount ::API::V3::ProjectSnippets + mount ::API::V3::Templates end before { allow_access_with_scope :api } diff --git a/lib/api/branches.rb b/lib/api/branches.rb index be659fa4a6a..9331be1f7de 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -84,7 +84,7 @@ module API branch = user_project.repository.find_branch(params[:branch]) not_found!("Branch") unless branch protected_branch = user_project.protected_branches.find_by(name: branch.name) - protected_branch.destroy if protected_branch + protected_branch&.destroy present branch, with: Entities::RepoBranch, project: user_project end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 3a5819d1bab..2a071e649fa 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -380,9 +380,7 @@ module API expose :author, using: Entities::UserBasic, if: ->(event, options) { event.author } expose :author_username do |event, options| - if event.author - event.author.username - end + event.author&.username end end diff --git a/lib/api/files.rb b/lib/api/files.rb index c58472de578..2ecdd747c8e 100644 --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -117,7 +117,7 @@ module API authorize! :push_code, user_project file_params = declared_params(include_missing: false) - result = ::Files::DeleteService.new(user_project, current_user, commit_params(file_params)).execute + result = ::Files::DestroyService.new(user_project, current_user, commit_params(file_params)).execute if result[:status] == :success status(200) diff --git a/lib/api/members.rb b/lib/api/members.rb index d85f1f78cd6..d1d78775c6d 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -56,16 +56,9 @@ module API member = source.members.find_by(user_id: params[:user_id]) - # We need this explicit check because `source.add_user` doesn't - # currently return the member created so it would return 201 even if - # the member already existed... - # The `source_type == 'group'` check is to ensure back-compatibility - # but 409 behavior should be used for both project and group members in 9.0! - conflict!('Member already exists') if source_type == 'group' && member - - unless member - member = source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at]) - end + conflict!('Member already exists') if member + + member = source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at]) if member.persisted? && member.valid? present member.user, with: Entities::Member, member: member diff --git a/lib/api/templates.rb b/lib/api/templates.rb index e23f99256a5..8a2d66efd89 100644 --- a/lib/api/templates.rb +++ b/lib/api/templates.rb @@ -24,7 +24,6 @@ module API /[\<\{\[] (fullname|name\sof\s(author|copyright\sowner)) [\>\}\]]/xi.freeze - DEPRECATION_MESSAGE = ' This endpoint is deprecated and will be removed in GitLab 9.0.'.freeze helpers do def parsed_license_template @@ -46,74 +45,58 @@ module API end end - { "licenses" => :deprecated, "templates/licenses" => :ok }.each do |route, status| - desc 'Get the list of the available license template' do - detailed_desc = 'This feature was introduced in GitLab 8.7.' - detailed_desc << DEPRECATION_MESSAGE unless status == :ok - detail detailed_desc - success Entities::RepoLicense - end - params do - optional :popular, type: Boolean, desc: 'If passed, returns only popular licenses' - end - get route do - options = { - featured: declared(params).popular.present? ? true : nil - } - present Licensee::License.all(options), with: Entities::RepoLicense - end + desc 'Get the list of the available license template' do + detail 'This feature was introduced in GitLab 8.7.' + success ::API::Entities::RepoLicense + end + params do + optional :popular, type: Boolean, desc: 'If passed, returns only popular licenses' + end + get "templates/licenses" do + options = { + featured: declared(params).popular.present? ? true : nil + } + present Licensee::License.all(options), with: ::API::Entities::RepoLicense end - { "licenses/:name" => :deprecated, "templates/licenses/:name" => :ok }.each do |route, status| - desc 'Get the text for a specific license' do - detailed_desc = 'This feature was introduced in GitLab 8.7.' - detailed_desc << DEPRECATION_MESSAGE unless status == :ok - detail detailed_desc - success Entities::RepoLicense - end - params do - requires :name, type: String, desc: 'The name of the template' - end - get route, requirements: { name: /[\w\.-]+/ } do - not_found!('License') unless Licensee::License.find(declared(params).name) + desc 'Get the text for a specific license' do + detail 'This feature was introduced in GitLab 8.7.' + success ::API::Entities::RepoLicense + end + params do + requires :name, type: String, desc: 'The name of the template' + end + get "templates/licenses/:name", requirements: { name: /[\w\.-]+/ } do + not_found!('License') unless Licensee::License.find(declared(params).name) - template = parsed_license_template + template = parsed_license_template - present template, with: Entities::RepoLicense - end + present template, with: ::API::Entities::RepoLicense end GLOBAL_TEMPLATE_TYPES.each do |template_type, properties| klass = properties[:klass] gitlab_version = properties[:gitlab_version] - { template_type => :deprecated, "templates/#{template_type}" => :ok }.each do |route, status| - desc 'Get the list of the available template' do - detailed_desc = "This feature was introduced in GitLab #{gitlab_version}." - detailed_desc << DEPRECATION_MESSAGE unless status == :ok - detail detailed_desc - success Entities::TemplatesList - end - get route do - present klass.all, with: Entities::TemplatesList - end + desc 'Get the list of the available template' do + detail "This feature was introduced in GitLab #{gitlab_version}." + success Entities::TemplatesList + end + get "templates/#{template_type}" do + present klass.all, with: Entities::TemplatesList end - { "#{template_type}/:name" => :deprecated, "templates/#{template_type}/:name" => :ok }.each do |route, status| - desc 'Get the text for a specific template present in local filesystem' do - detailed_desc = "This feature was introduced in GitLab #{gitlab_version}." - detailed_desc << DEPRECATION_MESSAGE unless status == :ok - detail detailed_desc - success Entities::Template - end - params do - requires :name, type: String, desc: 'The name of the template' - end - get route do - new_template = klass.find(declared(params).name) + desc 'Get the text for a specific template present in local filesystem' do + detail "This feature was introduced in GitLab #{gitlab_version}." + success Entities::Template + end + params do + requires :name, type: String, desc: 'The name of the template' + end + get "templates/#{template_type}/:name" do + new_template = klass.find(declared(params).name) - render_response(template_type, new_template) - end + render_response(template_type, new_template) end end end diff --git a/lib/api/v3/members.rb b/lib/api/v3/members.rb new file mode 100644 index 00000000000..4e6cb2e3c52 --- /dev/null +++ b/lib/api/v3/members.rb @@ -0,0 +1,134 @@ +module API + module V3 + class Members < Grape::API + include PaginationParams + + before { authenticate! } + + helpers ::API::Helpers::MembersHelpers + + %w[group project].each do |source_type| + params do + requires :id, type: String, desc: "The #{source_type} ID" + end + resource source_type.pluralize do + desc 'Gets a list of group or project members viewable by the authenticated user.' do + success ::API::Entities::Member + end + params do + optional :query, type: String, desc: 'A query string to search for members' + use :pagination + end + get ":id/members" do + source = find_source(source_type, params[:id]) + + users = source.users + users = users.merge(User.search(params[:query])) if params[:query] + + present paginate(users), with: ::API::Entities::Member, source: source + end + + desc 'Gets a member of a group or project.' do + success ::API::Entities::Member + end + params do + requires :user_id, type: Integer, desc: 'The user ID of the member' + end + get ":id/members/:user_id" do + source = find_source(source_type, params[:id]) + + members = source.members + member = members.find_by!(user_id: params[:user_id]) + + present member.user, with: ::API::Entities::Member, member: member + end + + desc 'Adds a member to a group or project.' do + success ::API::Entities::Member + end + params do + requires :user_id, type: Integer, desc: 'The user ID of the new member' + requires :access_level, type: Integer, desc: 'A valid access level (defaults: `30`, developer access level)' + optional :expires_at, type: DateTime, desc: 'Date string in the format YEAR-MONTH-DAY' + end + post ":id/members" do + source = find_source(source_type, params[:id]) + authorize_admin_source!(source_type, source) + + member = source.members.find_by(user_id: params[:user_id]) + + # We need this explicit check because `source.add_user` doesn't + # currently return the member created so it would return 201 even if + # the member already existed... + # The `source_type == 'group'` check is to ensure back-compatibility + # but 409 behavior should be used for both project and group members in 9.0! + conflict!('Member already exists') if source_type == 'group' && member + + unless member + member = source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at]) + end + if member.persisted? && member.valid? + present member.user, with: ::API::Entities::Member, member: member + else + # This is to ensure back-compatibility but 400 behavior should be used + # for all validation errors in 9.0! + render_api_error!('Access level is not known', 422) if member.errors.key?(:access_level) + render_validation_error!(member) + end + end + + desc 'Updates a member of a group or project.' do + success ::API::Entities::Member + end + params do + requires :user_id, type: Integer, desc: 'The user ID of the new member' + requires :access_level, type: Integer, desc: 'A valid access level' + optional :expires_at, type: DateTime, desc: 'Date string in the format YEAR-MONTH-DAY' + end + put ":id/members/:user_id" do + source = find_source(source_type, params[:id]) + authorize_admin_source!(source_type, source) + + member = source.members.find_by!(user_id: params[:user_id]) + attrs = attributes_for_keys [:access_level, :expires_at] + + if member.update_attributes(attrs) + present member.user, with: ::API::Entities::Member, member: member + else + # This is to ensure back-compatibility but 400 behavior should be used + # for all validation errors in 9.0! + render_api_error!('Access level is not known', 422) if member.errors.key?(:access_level) + render_validation_error!(member) + end + end + + desc 'Removes a user from a group or project.' + params do + requires :user_id, type: Integer, desc: 'The user ID of the member' + end + delete ":id/members/:user_id" do + source = find_source(source_type, params[:id]) + + # This is to ensure back-compatibility but find_by! should be used + # in that casse in 9.0! + member = source.members.find_by(user_id: params[:user_id]) + + # This is to ensure back-compatibility but this should be removed in + # favor of find_by! in 9.0! + not_found!("Member: user_id:#{params[:user_id]}") if source_type == 'group' && member.nil? + + # This is to ensure back-compatibility but 204 behavior should be used + # for all DELETE endpoints in 9.0! + if member.nil? + { message: "Access revoked", id: params[:user_id].to_i } + else + ::Members::DestroyService.new(source, current_user, declared_params).execute + + present member.user, with: ::API::Entities::Member, member: member + end + end + end + end + end + end +end diff --git a/lib/api/v3/templates.rb b/lib/api/v3/templates.rb new file mode 100644 index 00000000000..4c577a8d2b7 --- /dev/null +++ b/lib/api/v3/templates.rb @@ -0,0 +1,122 @@ +module API + module V3 + class Templates < Grape::API + GLOBAL_TEMPLATE_TYPES = { + gitignores: { + klass: Gitlab::Template::GitignoreTemplate, + gitlab_version: 8.8 + }, + gitlab_ci_ymls: { + klass: Gitlab::Template::GitlabCiYmlTemplate, + gitlab_version: 8.9 + }, + dockerfiles: { + klass: Gitlab::Template::DockerfileTemplate, + gitlab_version: 8.15 + } + }.freeze + PROJECT_TEMPLATE_REGEX = + /[\<\{\[] + (project|description| + one\sline\s.+\swhat\sit\sdoes\.) # matching the start and end is enough here + [\>\}\]]/xi.freeze + YEAR_TEMPLATE_REGEX = /[<{\[](year|yyyy)[>}\]]/i.freeze + FULLNAME_TEMPLATE_REGEX = + /[\<\{\[] + (fullname|name\sof\s(author|copyright\sowner)) + [\>\}\]]/xi.freeze + DEPRECATION_MESSAGE = ' This endpoint is deprecated and has been removed in V4.'.freeze + + helpers do + def parsed_license_template + # We create a fresh Licensee::License object since we'll modify its + # content in place below. + template = Licensee::License.new(params[:name]) + + template.content.gsub!(YEAR_TEMPLATE_REGEX, Time.now.year.to_s) + template.content.gsub!(PROJECT_TEMPLATE_REGEX, params[:project]) if params[:project].present? + + fullname = params[:fullname].presence || current_user.try(:name) + template.content.gsub!(FULLNAME_TEMPLATE_REGEX, fullname) if fullname + template + end + + def render_response(template_type, template) + not_found!(template_type.to_s.singularize) unless template + present template, with: ::API::Entities::Template + end + end + + { "licenses" => :deprecated, "templates/licenses" => :ok }.each do |route, status| + desc 'Get the list of the available license template' do + detailed_desc = 'This feature was introduced in GitLab 8.7.' + detailed_desc << DEPRECATION_MESSAGE unless status == :ok + detail detailed_desc + success ::API::Entities::RepoLicense + end + params do + optional :popular, type: Boolean, desc: 'If passed, returns only popular licenses' + end + get route do + options = { + featured: declared(params).popular.present? ? true : nil + } + present Licensee::License.all(options), with: ::API::Entities::RepoLicense + end + end + + { "licenses/:name" => :deprecated, "templates/licenses/:name" => :ok }.each do |route, status| + desc 'Get the text for a specific license' do + detailed_desc = 'This feature was introduced in GitLab 8.7.' + detailed_desc << DEPRECATION_MESSAGE unless status == :ok + detail detailed_desc + success ::API::Entities::RepoLicense + end + params do + requires :name, type: String, desc: 'The name of the template' + end + get route, requirements: { name: /[\w\.-]+/ } do + not_found!('License') unless Licensee::License.find(declared(params).name) + + template = parsed_license_template + + present template, with: ::API::Entities::RepoLicense + end + end + + GLOBAL_TEMPLATE_TYPES.each do |template_type, properties| + klass = properties[:klass] + gitlab_version = properties[:gitlab_version] + + { template_type => :deprecated, "templates/#{template_type}" => :ok }.each do |route, status| + desc 'Get the list of the available template' do + detailed_desc = "This feature was introduced in GitLab #{gitlab_version}." + detailed_desc << DEPRECATION_MESSAGE unless status == :ok + detail detailed_desc + success ::API::Entities::TemplatesList + end + get route do + present klass.all, with: ::API::Entities::TemplatesList + end + end + + { "#{template_type}/:name" => :deprecated, "templates/#{template_type}/:name" => :ok }.each do |route, status| + desc 'Get the text for a specific template present in local filesystem' do + detailed_desc = "This feature was introduced in GitLab #{gitlab_version}." + detailed_desc << DEPRECATION_MESSAGE unless status == :ok + detail detailed_desc + success ::API::Entities::Template + end + params do + requires :name, type: String, desc: 'The name of the template' + end + get route do + new_template = klass.find(declared(params).name) + + render_response(template_type, new_template) + end + end + end + end + end +end diff --git a/lib/banzai/querying.rb b/lib/banzai/querying.rb index 1e1b51e683e..fb2faae02bc 100644 --- a/lib/banzai/querying.rb +++ b/lib/banzai/querying.rb @@ -1,18 +1,64 @@ module Banzai module Querying + module_function + # Searches a Nokogiri document using a CSS query, optionally optimizing it # whenever possible. # - # document - A document/element to search. - # query - The CSS query to use. + # document - A document/element to search. + # query - The CSS query to use. + # reference_options - A hash with nodes filter options # - # Returns a Nokogiri::XML::NodeSet. - def self.css(document, query) + # Returns an array of Nokogiri::XML::Element objects if location is specified + # in reference_options. Otherwise it would a Nokogiri::XML::NodeSet. + def css(document, query, reference_options = {}) # When using "a.foo" Nokogiri compiles this to "//a[...]" but # "descendant::a[...]" is quite a bit faster and achieves the same result. xpath = Nokogiri::CSS.xpath_for(query)[0].gsub(%r{^//}, 'descendant::') + xpath = restrict_to_p_nodes_at_root(xpath) if filter_nodes_at_beginning?(reference_options) + nodes = document.xpath(xpath) + + filter_nodes(nodes, reference_options) + end + + def restrict_to_p_nodes_at_root(xpath) + xpath.gsub('descendant::', './p/') + end + + def filter_nodes(nodes, reference_options) + if filter_nodes_at_beginning?(reference_options) + filter_nodes_at_beginning(nodes) + else + nodes + end + end + + def filter_nodes_at_beginning?(reference_options) + reference_options && reference_options[:location] == :beginning + end + + # Selects child nodes if they are present in the beginning among other siblings. + # + # nodes - A Nokogiri::XML::NodeSet. + # + # Returns an array of Nokogiri::XML::Element objects. + def filter_nodes_at_beginning(nodes) + parents_and_nodes = nodes.group_by(&:parent) + filtered_nodes = [] + + parents_and_nodes.each do |parent, nodes| + children = parent.children + nodes = nodes.to_a + + children.each do |child| + next if child.text.blank? + node = nodes.shift + break unless node == child + filtered_nodes << node + end + end - document.xpath(xpath) + filtered_nodes end end end diff --git a/lib/banzai/reference_extractor.rb b/lib/banzai/reference_extractor.rb index b26a41a1f3b..8e3b0c4db79 100644 --- a/lib/banzai/reference_extractor.rb +++ b/lib/banzai/reference_extractor.rb @@ -16,6 +16,11 @@ module Banzai processor.process(html_documents) end + def reset_memoized_values + @html_documents = nil + @texts_and_contexts = [] + end + private def html_documents diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb index d8a855ec1fe..2058a58d0ae 100644 --- a/lib/banzai/reference_parser/base_parser.rb +++ b/lib/banzai/reference_parser/base_parser.rb @@ -33,7 +33,7 @@ module Banzai # they have access to. class BaseParser class << self - attr_accessor :reference_type + attr_accessor :reference_type, :reference_options end # Returns the attribute name containing the value for every object to be @@ -182,9 +182,10 @@ module Banzai # the references. def process(documents) type = self.class.reference_type + reference_options = self.class.reference_options nodes = documents.flat_map do |document| - Querying.css(document, "a[data-reference-type='#{type}'].gfm").to_a + Querying.css(document, "a[data-reference-type='#{type}'].gfm", reference_options).to_a end gather_references(nodes) diff --git a/lib/banzai/reference_parser/directly_addressed_user_parser.rb b/lib/banzai/reference_parser/directly_addressed_user_parser.rb new file mode 100644 index 00000000000..77df9bbd024 --- /dev/null +++ b/lib/banzai/reference_parser/directly_addressed_user_parser.rb @@ -0,0 +1,8 @@ +module Banzai + module ReferenceParser + class DirectlyAddressedUserParser < UserParser + self.reference_type = :user + self.reference_options = { location: :beginning } + end + end +end diff --git a/lib/gitlab/ci/config/entry/configurable.rb b/lib/gitlab/ci/config/entry/configurable.rb index 833ae4a0ff3..e05aca9881b 100644 --- a/lib/gitlab/ci/config/entry/configurable.rb +++ b/lib/gitlab/ci/config/entry/configurable.rb @@ -58,7 +58,7 @@ module Gitlab def helpers(*nodes) nodes.each do |symbol| define_method("#{symbol}_defined?") do - @entries[symbol].specified? if @entries[symbol] + @entries[symbol]&.specified? end define_method("#{symbol}_value") do diff --git a/lib/gitlab/cycle_analytics/base_event_fetcher.rb b/lib/gitlab/cycle_analytics/base_event_fetcher.rb index 0d8791d396b..ab115afcaa5 100644 --- a/lib/gitlab/cycle_analytics/base_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/base_event_fetcher.rb @@ -5,6 +5,8 @@ module Gitlab attr_reader :projections, :query, :stage, :order + MAX_EVENTS = 50 + def initialize(project:, stage:, options:) @project = project @stage = stage @@ -38,7 +40,7 @@ module Gitlab def events_query diff_fn = subtract_datetimes_diff(base_query, @options[:start_time_attrs], @options[:end_time_attrs]) - base_query.project(extract_diff_epoch(diff_fn).as('total_time'), *projections).order(order.desc) + base_query.project(extract_diff_epoch(diff_fn).as('total_time'), *projections).order(order.desc).take(MAX_EVENTS) end def default_order diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 0bd6e148ba8..4800a509b37 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -26,11 +26,59 @@ module Gitlab add_index(table_name, column_name, options) end + # Adds a foreign key with only minimal locking on the tables involved. + # + # This method only requires minimal locking when using PostgreSQL. When + # using MySQL this method will use Rails' default `add_foreign_key`. + # + # source - The source table containing the foreign key. + # target - The target table the key points to. + # column - The name of the column to create the foreign key on. + # on_delete - The action to perform when associated data is removed, + # defaults to "CASCADE". + def add_concurrent_foreign_key(source, target, column:, on_delete: :cascade) + # Transactions would result in ALTER TABLE locks being held for the + # duration of the transaction, defeating the purpose of this method. + if transaction_open? + raise 'add_concurrent_foreign_key can not be run inside a transaction' + end + + # While MySQL does allow disabling of foreign keys it has no equivalent + # of PostgreSQL's "VALIDATE CONSTRAINT". As a result we'll just fall + # back to the normal foreign key procedure. + if Database.mysql? + return add_foreign_key(source, target, + column: column, + on_delete: on_delete) + end + + disable_statement_timeout + + key_name = "fk_#{source}_#{target}_#{column}" + + # Using NOT VALID allows us to create a key without immediately + # validating it. This means we keep the ALTER TABLE lock only for a + # short period of time. The key _is_ enforced for any newly created + # data. + execute <<-EOF.strip_heredoc + ALTER TABLE #{source} + ADD CONSTRAINT #{key_name} + FOREIGN KEY (#{column}) + REFERENCES #{target} (id) + ON DELETE #{on_delete} NOT VALID; + EOF + + # Validate the existing constraint. This can potentially take a very + # long time to complete, but fortunately does not lock the source table + # while running. + execute("ALTER TABLE #{source} VALIDATE CONSTRAINT #{key_name};") + end + # Long-running migrations may take more than the timeout allowed by # the database. Disable the session's statement timeout to ensure # migrations don't get killed prematurely. (PostgreSQL only) def disable_statement_timeout - ActiveRecord::Base.connection.execute('SET statement_timeout TO 0') if Database.postgresql? + execute('SET statement_timeout TO 0') if Database.postgresql? end # Updates the value of a column in batches. diff --git a/lib/gitlab/diff/parser.rb b/lib/gitlab/diff/parser.rb index 59a2367b65d..89320f5d9dc 100644 --- a/lib/gitlab/diff/parser.rb +++ b/lib/gitlab/diff/parser.rb @@ -45,7 +45,7 @@ module Gitlab line_new += 1 when "-" line_old += 1 - when "\\" + when "\\" # rubocop:disable Lint/EmptyWhen # No increment else line_new += 1 diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index 0e3b65fceb4..6c69cd9e6a9 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -46,7 +46,7 @@ module Gitlab end def diffs_count - diffs.size if diffs + diffs&.size end def compare @@ -58,7 +58,7 @@ module Gitlab end def compare_timeout - diffs.overflow? if diffs + diffs&.overflow? end def reverse_compare? diff --git a/lib/gitlab/git/blob_snippet.rb b/lib/gitlab/git/blob_snippet.rb index e98de57fc22..d7975f88aaa 100644 --- a/lib/gitlab/git/blob_snippet.rb +++ b/lib/gitlab/git/blob_snippet.rb @@ -13,7 +13,7 @@ module Gitlab end def data - lines.join("\n") if lines + lines&.join("\n") end def name diff --git a/lib/gitlab/metrics.rb b/lib/gitlab/metrics.rb index 3d1ba33ec68..857e0abf710 100644 --- a/lib/gitlab/metrics.rb +++ b/lib/gitlab/metrics.rb @@ -112,7 +112,7 @@ module Gitlab def self.tag_transaction(name, value) trans = current_transaction - trans.add_tag(name, value) if trans + trans&.add_tag(name, value) end # Sets the action of the current transaction (if any) @@ -121,7 +121,7 @@ module Gitlab def self.action=(action) trans = current_transaction - trans.action = action if trans + trans&.action = action end # Tracks an event. @@ -130,7 +130,7 @@ module Gitlab def self.add_event(*args) trans = current_transaction - trans.add_event(*args) if trans + trans&.add_event(*args) end # Returns the prefix to use for the name of a series. diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index 11c0b01f0dc..437a339dd2b 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -1,13 +1,12 @@ module Gitlab # Extract possible GFM references from an arbitrary String for further processing. class ReferenceExtractor < Banzai::ReferenceExtractor - REFERABLES = %i(user issue label milestone merge_request snippet commit commit_range) + REFERABLES = %i(user issue label milestone merge_request snippet commit commit_range directly_addressed_user) attr_accessor :project, :current_user, :author def initialize(project, current_user = nil) @project = project @current_user = current_user - @references = {} super() @@ -21,6 +20,11 @@ module Gitlab super(type, project, current_user) end + def reset_memoized_values + @references = {} + super() + end + REFERABLES.each do |type| define_method("#{type}s") do @references[type] ||= references(type) diff --git a/rubocop/cop/migration/add_concurrent_foreign_key.rb b/rubocop/cop/migration/add_concurrent_foreign_key.rb new file mode 100644 index 00000000000..e40a7087a47 --- /dev/null +++ b/rubocop/cop/migration/add_concurrent_foreign_key.rb @@ -0,0 +1,27 @@ +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks if `add_concurrent_foreign_key` is used instead of + # `add_foreign_key`. + class AddConcurrentForeignKey < RuboCop::Cop::Cop + include MigrationHelpers + + MSG = '`add_foreign_key` requires downtime, use `add_concurrent_foreign_key` instead' + + def on_send(node) + return unless in_migration?(node) + + name = node.children[1] + + add_offense(node, :selector) if name == :add_foreign_key + end + + def method_name(node) + node.children.first + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index aa35fb1701c..d4266d0deae 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -1,4 +1,5 @@ require_relative 'cop/gem_fetcher' require_relative 'cop/migration/add_column' require_relative 'cop/migration/add_column_with_default' +require_relative 'cop/migration/add_concurrent_foreign_key' require_relative 'cop/migration/add_index' diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb new file mode 100644 index 00000000000..566d8515198 --- /dev/null +++ b/spec/controllers/dashboard_controller_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe DashboardController do + let(:user) { create(:user) } + let(:project) { create(:project) } + + before do + project.team << [user, :master] + sign_in(user) + end + + describe 'GET issues' do + it_behaves_like 'issuables list meta-data', :issue, :issues + end + + describe 'GET merge requests' do + it_behaves_like 'issuables list meta-data', :merge_request, :merge_requests + end +end diff --git a/spec/controllers/profiles/keys_controller_spec.rb b/spec/controllers/profiles/keys_controller_spec.rb index 6bcfae0fc13..f7219690722 100644 --- a/spec/controllers/profiles/keys_controller_spec.rb +++ b/spec/controllers/profiles/keys_controller_spec.rb @@ -42,10 +42,9 @@ describe Profiles::KeysController do end describe "user with keys" do - before do - user.keys << create(:key) - user.keys << create(:another_key) - end + let!(:key) { create(:key, user: user) } + let!(:another_key) { create(:another_key, user: user) } + let!(:deploy_key) { create(:deploy_key, user: user) } it "does generally work" do get :get_keys, username: user.username @@ -53,16 +52,16 @@ describe Profiles::KeysController do expect(response).to be_success end - it "renders all keys separated with a new line" do + it "renders all non deploy keys separated with a new line" do get :get_keys, username: user.username - expect(response.body).not_to eq("") + expect(response.body).not_to eq('') expect(response.body).to eq(user.all_ssh_keys.join("\n")) - # Unique part of key 1 - expect(response.body).to match(/PWx6WM4lhHNedGfBpPJNPpZ/) - # Key 2 - expect(response.body).to match(/AQDmTillFzNTrrGgwaCKaSj/) + expect(response.body).to include(key.key.sub(' dummy@gitlab.com', '')) + expect(response.body).to include(another_key.key) + + expect(response.body).not_to include(deploy_key.key) end it "does not render the comment of the key" do diff --git a/spec/controllers/profiles/notifications_controller_spec.rb b/spec/controllers/profiles/notifications_controller_spec.rb new file mode 100644 index 00000000000..58caf7999cf --- /dev/null +++ b/spec/controllers/profiles/notifications_controller_spec.rb @@ -0,0 +1,45 @@ +require 'spec_helper' + +describe Profiles::NotificationsController do + let(:user) do + create(:user) do |user| + user.emails.create(email: 'original@example.com') + user.emails.create(email: 'new@example.com') + user.update(notification_email: 'original@example.com') + user.save! + end + end + + describe 'GET show' do + it 'renders' do + sign_in(user) + + get :show + + expect(response).to render_template :show + end + end + + describe 'POST update' do + it 'updates only permitted attributes' do + sign_in(user) + + put :update, user: { notification_email: 'new@example.com', notified_of_own_activity: true, admin: true } + + user.reload + expect(user.notification_email).to eq('new@example.com') + expect(user.notified_of_own_activity).to eq(true) + expect(user.admin).to eq(false) + expect(controller).to set_flash[:notice].to('Notification settings saved') + end + + it 'shows an error message if the params are invalid' do + sign_in(user) + + put :update, user: { notification_email: '' } + + expect(user.reload.notification_email).to eq('original@example.com') + expect(controller).to set_flash[:alert].to('Failed to save new settings') + end + end +end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 4b89381eb96..e576bf9ef79 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -24,6 +24,8 @@ describe Projects::IssuesController do project.team << [user, :developer] end + it_behaves_like "issuables list meta-data", :issue + it "returns index" do get :index, namespace_id: project.namespace.path, project_id: project.path diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 63780802cfa..bfd134e406e 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -147,6 +147,8 @@ describe Projects::MergeRequestsController do end describe 'GET index' do + let!(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } + def get_merge_requests(page = nil) get :index, namespace_id: project.namespace.to_param, @@ -154,6 +156,8 @@ describe Projects::MergeRequestsController do state: 'opened', page: page.to_param end + it_behaves_like "issuables list meta-data", :merge_request + context 'when page param' do let(:last_page) { project.merge_requests.page().total_pages } let!(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } diff --git a/spec/factories/keys.rb b/spec/factories/keys.rb index d69c5b38d0a..dd93b439b2b 100644 --- a/spec/factories/keys.rb +++ b/spec/factories/keys.rb @@ -2,10 +2,13 @@ FactoryGirl.define do factory :key do title key do - "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0= dummy@gitlab.com" + 'ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0= dummy@gitlab.com' end factory :deploy_key, class: 'DeployKey' do + key do + 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDFf6RYK3qu/RKF/3ndJmL5xgMLp3O96x8lTay+QGZ0+9FnnAXMdUqBq/ZU6d/gyMB4IaW3nHzM1w049++yAB6UPCzMB8Uo27K5/jyZCtj7Vm9PFNjF/8am1kp46c/SeYicQgQaSBdzIW3UDEa1Ef68qroOlvpi9PYZ/tA7M0YP0K5PXX+E36zaIRnJVMPT3f2k+GnrxtjafZrwFdpOP/Fol5BQLBgcsyiU+LM1SuaCrzd8c9vyaTA1CxrkxaZh+buAi0PmdDtaDrHd42gqZkXCKavyvgM5o2CkQ5LJHCgzpXy05qNFzmThBSkb+XtoxbyagBiGbVZtSVow6Xa7qewz' + end end factory :personal_key do @@ -14,7 +17,7 @@ FactoryGirl.define do factory :another_key do key do - "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDmTillFzNTrrGgwaCKaSj+QCz81E6jBc/s9av0+3b1Hwfxgkqjl4nAK/OD2NjgyrONDTDfR8cRN4eAAy6nY8GLkOyYBDyuc5nTMqs5z3yVuTwf3koGm/YQQCmo91psZ2BgDFTor8SVEE5Mm1D1k3JDMhDFxzzrOtRYFPci9lskTJaBjpqWZ4E9rDTD2q/QZntCqbC3wE9uSemRQB5f8kik7vD/AD8VQXuzKladrZKkzkONCPWsXDspUitjM8HkQdOf0PsYn1CMUC1xKYbCxkg5TkEosIwGv6CoEArUrdu/4+10LVslq494mAvEItywzrluCLCnwELfW+h/m8UHoVhZ" + 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDmTillFzNTrrGgwaCKaSj+QCz81E6jBc/s9av0+3b1Hwfxgkqjl4nAK/OD2NjgyrONDTDfR8cRN4eAAy6nY8GLkOyYBDyuc5nTMqs5z3yVuTwf3koGm/YQQCmo91psZ2BgDFTor8SVEE5Mm1D1k3JDMhDFxzzrOtRYFPci9lskTJaBjpqWZ4E9rDTD2q/QZntCqbC3wE9uSemRQB5f8kik7vD/AD8VQXuzKladrZKkzkONCPWsXDspUitjM8HkQdOf0PsYn1CMUC1xKYbCxkg5TkEosIwGv6CoEArUrdu/4+10LVslq494mAvEItywzrluCLCnwELfW+h/m8UHoVhZ' end factory :another_deploy_key, class: 'DeployKey' do diff --git a/spec/factories/todos.rb b/spec/factories/todos.rb index 275561502cd..b4e4cd97780 100644 --- a/spec/factories/todos.rb +++ b/spec/factories/todos.rb @@ -14,6 +14,10 @@ FactoryGirl.define do action { Todo::MENTIONED } end + trait :directly_addressed do + action { Todo::DIRECTLY_ADDRESSED } + end + trait :on_commit do commit_id RepoHelpers.sample_commit.id target_type "Commit" diff --git a/spec/factories/wiki_directories.rb b/spec/factories/wiki_directories.rb new file mode 100644 index 00000000000..3f3c864ac2b --- /dev/null +++ b/spec/factories/wiki_directories.rb @@ -0,0 +1,6 @@ +FactoryGirl.define do + factory :wiki_directory do + slug '/path_up_to/dir' + initialize_with { new(slug) } + end +end diff --git a/spec/features/issuables/issuable_list_spec.rb b/spec/features/issuables/issuable_list_spec.rb new file mode 100644 index 00000000000..e31bc40adc3 --- /dev/null +++ b/spec/features/issuables/issuable_list_spec.rb @@ -0,0 +1,57 @@ +require 'rails_helper' + +describe 'issuable list', feature: true do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + + issuable_types = [:issue, :merge_request] + + before do + project.add_user(user, :developer) + login_as(user) + issuable_types.each { |type| create_issuables(type) } + end + + issuable_types.each do |issuable_type| + it "avoids N+1 database queries for #{issuable_type.to_s.humanize.pluralize}" do + control_count = ActiveRecord::QueryRecorder.new { visit_issuable_list(issuable_type) }.count + + create_issuables(issuable_type) + + expect { visit_issuable_list(issuable_type) }.not_to exceed_query_limit(control_count) + end + + it "counts upvotes, downvotes and notes count for each #{issuable_type.to_s.humanize}" do + visit_issuable_list(issuable_type) + + expect(first('.fa-thumbs-up').find(:xpath, '..')).to have_content(1) + expect(first('.fa-thumbs-down').find(:xpath, '..')).to have_content(1) + expect(first('.fa-comments').find(:xpath, '..')).to have_content(2) + end + end + + def visit_issuable_list(issuable_type) + if issuable_type == :issue + visit namespace_project_issues_path(project.namespace, project) + else + visit namespace_project_merge_requests_path(project.namespace, project) + end + end + + def create_issuables(issuable_type) + 3.times do + if issuable_type == :issue + issuable = create(:issue, project: project, author: user) + else + issuable = create(:merge_request, title: FFaker::Lorem.sentence, source_project: project, source_branch: FFaker::Name.name) + end + + 2.times do + create(:note_on_issue, noteable: issuable, project: project, note: 'Test note') + end + + create(:award_emoji, :downvote, awardable: issuable) + create(:award_emoji, :upvote, awardable: issuable) + end + end +end diff --git a/spec/features/issues/filtered_search/filter_issues_spec.rb b/spec/features/issues/filtered_search/filter_issues_spec.rb index 6f7046c8461..64f448a83b7 100644 --- a/spec/features/issues/filtered_search/filter_issues_spec.rb +++ b/spec/features/issues/filtered_search/filter_issues_spec.rb @@ -113,13 +113,11 @@ describe 'Filter issues', js: true, feature: true do end it 'filters issues by invalid author' do - pending('to be tested, issue #26546') - expect(true).to be(false) + skip('to be tested, issue #26546') end it 'filters issues by multiple authors' do - pending('to be tested, issue #26546') - expect(true).to be(false) + skip('to be tested, issue #26546') end end @@ -158,8 +156,7 @@ describe 'Filter issues', js: true, feature: true do end it 'sorting' do - pending('to be tested, issue #26546') - expect(true).to be(false) + skip('to be tested, issue #26546') end end @@ -182,13 +179,11 @@ describe 'Filter issues', js: true, feature: true do end it 'filters issues by invalid assignee' do - pending('to be tested, issue #26546') - expect(true).to be(false) + skip('to be tested, issue #26546') end it 'filters issues by multiple assignees' do - pending('to be tested, issue #26546') - expect(true).to be(false) + skip('to be tested, issue #26546') end end @@ -228,8 +223,7 @@ describe 'Filter issues', js: true, feature: true do context 'sorting' do it 'sorts' do - pending('to be tested, issue #26546') - expect(true).to be(false) + skip('to be tested, issue #26546') end end end @@ -253,8 +247,7 @@ describe 'Filter issues', js: true, feature: true do end it 'filters issues by invalid label' do - pending('to be tested, issue #26546') - expect(true).to be(false) + skip('to be tested, issue #26546') end it 'filters issues by multiple labels' do @@ -429,8 +422,7 @@ describe 'Filter issues', js: true, feature: true do context 'sorting' do it 'sorts' do - pending('to be tested, issue #26546') - expect(true).to be(false) + skip('to be tested, issue #26546') end end end @@ -456,13 +448,11 @@ describe 'Filter issues', js: true, feature: true do end it 'filters issues by invalid milestones' do - pending('to be tested, issue #26546') - expect(true).to be(false) + skip('to be tested, issue #26546') end it 'filters issues by multiple milestones' do - pending('to be tested, issue #26546') - expect(true).to be(false) + skip('to be tested, issue #26546') end it 'filters issues by milestone containing special characters' do @@ -523,8 +513,7 @@ describe 'Filter issues', js: true, feature: true do context 'sorting' do it 'sorts' do - pending('to be tested, issue #26546') - expect(true).to be(false) + skip('to be tested, issue #26546') end end end diff --git a/spec/features/merge_requests/closes_issues_spec.rb b/spec/features/merge_requests/closes_issues_spec.rb index c73065cdce1..eafcab6a0d7 100644 --- a/spec/features/merge_requests/closes_issues_spec.rb +++ b/spec/features/merge_requests/closes_issues_spec.rb @@ -10,10 +10,12 @@ feature 'Merge Request closing issues message', feature: true do :merge_request, :simple, source_project: project, - description: merge_request_description + description: merge_request_description, + title: merge_request_title ) end let(:merge_request_description) { 'Merge Request Description' } + let(:merge_request_title) { 'Merge Request Title' } before do project.team << [user, :master] @@ -45,8 +47,32 @@ feature 'Merge Request closing issues message', feature: true do end end - context 'closing some issues and mentioning, but not closing, others' do - let(:merge_request_description) { "Description\n\ncloses #{issue_1.to_reference}\n\n refers to #{issue_2.to_reference}" } + context 'closing some issues in title and mentioning, but not closing, others' do + let(:merge_request_title) { "closes #{issue_1.to_reference}\n\n refers to #{issue_2.to_reference}" } + + it 'does not display closing issue message' do + expect(page).to have_content("Accepting this merge request will close issue #{issue_1.to_reference}. Issue #{issue_2.to_reference} is mentioned but will not be closed.") + end + end + + context 'closing issues using title but not mentioning any other issue' do + let(:merge_request_title) { "closing #{issue_1.to_reference}, #{issue_2.to_reference}" } + + it 'does not display closing issue message' do + expect(page).to have_content("Accepting this merge request will close issues #{issue_1.to_reference} and #{issue_2.to_reference}") + end + end + + context 'mentioning issues using title but not closing them' do + let(:merge_request_title) { "Refers to #{issue_1.to_reference} and #{issue_2.to_reference}" } + + it 'does not display closing issue message' do + expect(page).to have_content("Issues #{issue_1.to_reference} and #{issue_2.to_reference} are mentioned but will not be closed.") + end + end + + context 'closing some issues using title and mentioning, but not closing, others' do + let(:merge_request_title) { "closes #{issue_1.to_reference}\n\n refers to #{issue_2.to_reference}" } it 'does not display closing issue message' do expect(page).to have_content("Accepting this merge request will close issue #{issue_1.to_reference}. Issue #{issue_2.to_reference} is mentioned but will not be closed.") diff --git a/spec/features/profiles/user_changes_notified_of_own_activity_spec.rb b/spec/features/profiles/user_changes_notified_of_own_activity_spec.rb new file mode 100644 index 00000000000..e05fbb3715c --- /dev/null +++ b/spec/features/profiles/user_changes_notified_of_own_activity_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +feature 'Profile > Notifications > User changes notified_of_own_activity setting', feature: true, js: true do + let(:user) { create(:user) } + + before do + login_as(user) + end + + scenario 'User opts into receiving notifications about their own activity' do + visit profile_notifications_path + + expect(page).not_to have_checked_field('user[notified_of_own_activity]') + + check 'user[notified_of_own_activity]' + + expect(page).to have_content('Notification settings saved') + expect(page).to have_checked_field('user[notified_of_own_activity]') + end + + scenario 'User opts out of receiving notifications about their own activity' do + user.update!(notified_of_own_activity: true) + visit profile_notifications_path + + expect(page).to have_checked_field('user[notified_of_own_activity]') + + uncheck 'user[notified_of_own_activity]' + + expect(page).to have_content('Notification settings saved') + expect(page).not_to have_checked_field('user[notified_of_own_activity]') + end +end diff --git a/spec/features/projects/builds_spec.rb b/spec/features/projects/builds_spec.rb index f7e0115643e..f1036b275f7 100644 --- a/spec/features/projects/builds_spec.rb +++ b/spec/features/projects/builds_spec.rb @@ -109,6 +109,10 @@ feature 'Builds', :feature do expect(page).to have_content pipeline.git_commit_message expect(page).to have_content pipeline.git_author_name end + + it 'shows active build' do + expect(page).to have_selector('.build-job.active') + end end context "Job from other project" do diff --git a/spec/helpers/wiki_helper_spec.rb b/spec/helpers/wiki_helper_spec.rb new file mode 100644 index 00000000000..92c6f27a867 --- /dev/null +++ b/spec/helpers/wiki_helper_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe WikiHelper do + describe '#breadcrumb' do + context 'when the page is at the root level' do + it 'returns the capitalized page name' do + slug = 'page-name' + + expect(helper.breadcrumb(slug)).to eq('Page name') + end + end + + context 'when the page is inside a directory' do + it 'returns the capitalized name of each directory and of the page itself' do + slug = 'dir_1/page-name' + + expect(helper.breadcrumb(slug)).to eq('Dir_1 / Page name') + end + end + end +end diff --git a/spec/javascripts/fixtures/branches.rb b/spec/javascripts/fixtures/branches.rb new file mode 100644 index 00000000000..0e7c2351b66 --- /dev/null +++ b/spec/javascripts/fixtures/branches.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Projects::BranchesController, '(JavaScript fixtures)', type: :controller do + include JavaScriptFixturesHelpers + + let(:admin) { create(:admin) } + let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} + let(:project) { create(:project, :repository, namespace: namespace, path: 'branches-project') } + + render_views + + before(:all) do + clean_frontend_fixtures('branches/') + end + + before(:each) do + sign_in(admin) + end + + it 'branches/new_branch.html.raw' do |example| + get :new, + namespace_id: project.namespace.to_param, + project_id: project.to_param + + expect(response).to be_success + store_frontend_fixture(response, example.description) + end +end diff --git a/spec/javascripts/fixtures/header.html.haml b/spec/javascripts/fixtures/header.html.haml deleted file mode 100644 index f397f69e753..00000000000 --- a/spec/javascripts/fixtures/header.html.haml +++ /dev/null @@ -1,35 +0,0 @@ -%header.navbar.navbar-gitlab.nav_header_class - .container-fluid - .header-content - %button.side-nav-toggle - %span.sr-only - Toggle navigation - %i.fa.fa-bars - %button.navbar-toggle - %span.sr-only - Toggle navigation - %i.fa.fa-ellipsis-v - .navbar-collapse.collapse - %ui.nav.navbar-nav - %li.hidden-sm.hidden-xs - %li.visible-sm.visible-xs - %li - %a - %i.fa.fa-bell.fa-fw - %span.badge.todos-pending-count - %li - %a - %i.fa.fa-plus.fa-fw - %li.header-user.dropdown - %a - %img - %span.caret - .dropdown-menu-nav - .dropdown-menu-align-right - %ul - %li - %a.profile-link - %li - %a - %li.divider - %li.sign-out-link diff --git a/spec/javascripts/fixtures/merge_request_tabs.html.haml b/spec/javascripts/fixtures/merge_request_tabs.html.haml deleted file mode 100644 index 68678c3d7e3..00000000000 --- a/spec/javascripts/fixtures/merge_request_tabs.html.haml +++ /dev/null @@ -1,22 +0,0 @@ -%ul.nav.nav-tabs.merge-request-tabs - %li.notes-tab - %a{href: '/foo/bar/merge_requests/1', data: {target: 'div#notes', action: 'notes', toggle: 'tab'}} - Discussion - %li.commits-tab - %a{href: '/foo/bar/merge_requests/1/commits', data: {target: 'div#commits', action: 'commits', toggle: 'tab'}} - Commits - %li.diffs-tab - %a{href: '/foo/bar/merge_requests/1/diffs', data: {target: 'div#diffs', action: 'diffs', toggle: 'tab'}} - Diffs - -.tab-content - #notes.notes.tab-pane - Notes Content - #commits.commits.tab-pane - Commits Content - #diffs.diffs.tab-pane - Diffs Content - -.mr-loading-status - .loading - Loading Animation diff --git a/spec/javascripts/fixtures/merge_requests.rb b/spec/javascripts/fixtures/merge_requests.rb new file mode 100644 index 00000000000..62984097099 --- /dev/null +++ b/spec/javascripts/fixtures/merge_requests.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: :controller do + include JavaScriptFixturesHelpers + + let(:admin) { create(:admin) } + let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} + let(:project) { create(:project, namespace: namespace, path: 'merge-requests-project') } + + render_views + + before(:all) do + clean_frontend_fixtures('merge_requests/') + end + + before(:each) do + sign_in(admin) + end + + it 'merge_requests/merge_request_with_task_list.html.raw' do |example| + merge_request = create(:merge_request, :with_diffs, source_project: project, target_project: project, description: '- [ ] Task List Item') + render_merge_request(example.description, merge_request) + end + + private + + def render_merge_request(fixture_file_name, merge_request) + get :show, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: merge_request.to_param + + expect(response).to be_success + store_frontend_fixture(response, fixture_file_name) + end +end diff --git a/spec/javascripts/fixtures/new_branch.html.haml b/spec/javascripts/fixtures/new_branch.html.haml deleted file mode 100644 index f06629e5ecc..00000000000 --- a/spec/javascripts/fixtures/new_branch.html.haml +++ /dev/null @@ -1,4 +0,0 @@ -%form.js-create-branch-form - %input.js-branch-name - .js-branch-name-error - %input{id: "ref"} diff --git a/spec/javascripts/header_spec.js b/spec/javascripts/header_spec.js index cecebb0b038..2b263b71b7d 100644 --- a/spec/javascripts/header_spec.js +++ b/spec/javascripts/header_spec.js @@ -6,7 +6,7 @@ require('~/lib/utils/text_utility'); (function() { describe('Header', function() { var todosPendingCount = '.todos-pending-count'; - var fixtureTemplate = 'static/header.html.raw'; + var fixtureTemplate = 'issues/open-issue.html.raw'; function isTodosCountHidden() { return $(todosPendingCount).hasClass('hidden'); diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index 92a0f1c05f7..3810991f104 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -25,7 +25,7 @@ require('vendor/jquery.scrollTo'); }; $.extend(stubLocation, defaults, stubs || {}); }; - preloadFixtures('static/merge_request_tabs.html.raw'); + preloadFixtures('merge_requests/merge_request_with_task_list.html.raw'); beforeEach(function () { this.class = new gl.MergeRequestTabs({ stubLocation: stubLocation }); @@ -41,7 +41,7 @@ require('vendor/jquery.scrollTo'); describe('#activateTab', function () { beforeEach(function () { spyOn($, 'ajax').and.callFake(function () {}); - loadFixtures('static/merge_request_tabs.html.raw'); + loadFixtures('merge_requests/merge_request_with_task_list.html.raw'); this.subject = this.class.activateTab; }); it('shows the first tab when action is show', function () { diff --git a/spec/javascripts/new_branch_spec.js b/spec/javascripts/new_branch_spec.js index 9b657868523..1d014502c2a 100644 --- a/spec/javascripts/new_branch_spec.js +++ b/spec/javascripts/new_branch_spec.js @@ -8,7 +8,7 @@ require('~/new_branch_form'); describe('Branch', function() { return describe('create a new branch', function() { var expectToHaveError, fillNameWith; - preloadFixtures('static/new_branch.html.raw'); + preloadFixtures('branches/new_branch.html.raw'); fillNameWith = function(value) { return $('.js-branch-name').val(value).trigger('blur'); }; @@ -16,7 +16,7 @@ require('~/new_branch_form'); return expect($('.js-branch-name-error span').text()).toEqual(error); }; beforeEach(function() { - loadFixtures('static/new_branch.html.raw'); + loadFixtures('branches/new_branch.html.raw'); $('form').on('submit', function(e) { return e.preventDefault(); }); diff --git a/spec/lib/gitlab/cycle_analytics/base_event_fetcher_spec.rb b/spec/lib/gitlab/cycle_analytics/base_event_fetcher_spec.rb new file mode 100644 index 00000000000..c455cd9b942 --- /dev/null +++ b/spec/lib/gitlab/cycle_analytics/base_event_fetcher_spec.rb @@ -0,0 +1,45 @@ +require 'spec_helper' + +describe Gitlab::CycleAnalytics::BaseEventFetcher do + let(:max_events) { 2 } + let(:project) { create(:project) } + let(:user) { create(:user, :admin) } + let(:start_time_attrs) { Issue.arel_table[:created_at] } + let(:end_time_attrs) { [Issue::Metrics.arel_table[:first_associated_with_milestone_at]] } + let(:options) do + { start_time_attrs: start_time_attrs, + end_time_attrs: end_time_attrs, + from: 30.days.ago } + end + + subject do + described_class.new(project: project, + stage: :issue, + options: options).fetch + end + + before do + allow_any_instance_of(Gitlab::ReferenceExtractor).to receive(:issues).and_return(Issue.all) + allow_any_instance_of(Gitlab::CycleAnalytics::BaseEventFetcher).to receive(:serialize) do |event| + event + end + + stub_const('Gitlab::CycleAnalytics::BaseEventFetcher::MAX_EVENTS', max_events) + + setup_events(count: 3) + end + + it 'limits the rows to the max number' do + expect(subject.count).to eq(max_events) + end + + def setup_events(count:) + count.times do + issue = create(:issue, project: project, created_at: 2.days.ago) + milestone = create(:milestone, project: project) + + issue.update(milestone: milestone) + create_merge_request_closing_issue(issue) + end + end +end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 7fd25b9e5bf..e94ca4fcfd2 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -12,15 +12,14 @@ describe Gitlab::Database::MigrationHelpers, lib: true do describe '#add_concurrent_index' do context 'outside a transaction' do before do - expect(model).to receive(:transaction_open?).and_return(false) - - unless Gitlab::Database.postgresql? - allow_any_instance_of(Gitlab::Database::MigrationHelpers).to receive(:disable_statement_timeout) - end + allow(model).to receive(:transaction_open?).and_return(false) end context 'using PostgreSQL' do - before { expect(Gitlab::Database).to receive(:postgresql?).and_return(true) } + before do + allow(Gitlab::Database).to receive(:postgresql?).and_return(true) + allow(model).to receive(:disable_statement_timeout) + end it 'creates the index concurrently' do expect(model).to receive(:add_index). @@ -59,6 +58,71 @@ describe Gitlab::Database::MigrationHelpers, lib: true do end end + describe '#add_concurrent_foreign_key' do + context 'inside a transaction' do + it 'raises an error' do + expect(model).to receive(:transaction_open?).and_return(true) + + expect do + model.add_concurrent_foreign_key(:projects, :users, column: :user_id) + end.to raise_error(RuntimeError) + end + end + + context 'outside a transaction' do + before do + allow(model).to receive(:transaction_open?).and_return(false) + end + + context 'using MySQL' do + it 'creates a regular foreign key' do + allow(Gitlab::Database).to receive(:mysql?).and_return(true) + + expect(model).to receive(:add_foreign_key). + with(:projects, :users, column: :user_id, on_delete: :cascade) + + model.add_concurrent_foreign_key(:projects, :users, column: :user_id) + end + end + + context 'using PostgreSQL' do + before do + allow(Gitlab::Database).to receive(:mysql?).and_return(false) + end + + it 'creates a concurrent foreign key' do + expect(model).to receive(:disable_statement_timeout) + expect(model).to receive(:execute).ordered.with(/NOT VALID/) + expect(model).to receive(:execute).ordered.with(/VALIDATE CONSTRAINT/) + + model.add_concurrent_foreign_key(:projects, :users, column: :user_id) + end + end + end + end + + describe '#disable_statement_timeout' do + context 'using PostgreSQL' do + it 'disables statement timeouts' do + expect(Gitlab::Database).to receive(:postgresql?).and_return(true) + + expect(model).to receive(:execute).with('SET statement_timeout TO 0') + + model.disable_statement_timeout + end + end + + context 'using MySQL' do + it 'does nothing' do + expect(Gitlab::Database).to receive(:postgresql?).and_return(false) + + expect(model).not_to receive(:execute) + + model.disable_statement_timeout + end + end + end + describe '#update_column_in_batches' do before do create_list(:empty_project, 5) diff --git a/spec/lib/gitlab/diff/position_tracer_spec.rb b/spec/lib/gitlab/diff/position_tracer_spec.rb index 8e3e4034c8f..994995b57b8 100644 --- a/spec/lib/gitlab/diff/position_tracer_spec.rb +++ b/spec/lib/gitlab/diff/position_tracer_spec.rb @@ -122,7 +122,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do end def delete_file(branch_name, file_name) - Files::DeleteService.new( + Files::DestroyService.new( project, current_user, start_branch: branch_name, diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 6b689c41ef6..84cfd934fa0 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -42,14 +42,85 @@ describe Gitlab::ReferenceExtractor, lib: true do > @offteam }) + expect(subject.users).to match_array([]) end + describe 'directly addressed users' do + before do + @u_foo = create(:user, username: 'foo') + @u_foo2 = create(:user, username: 'foo2') + @u_foo3 = create(:user, username: 'foo3') + @u_foo4 = create(:user, username: 'foo4') + @u_foo5 = create(:user, username: 'foo5') + + @u_bar = create(:user, username: 'bar') + @u_bar2 = create(:user, username: 'bar2') + @u_bar3 = create(:user, username: 'bar3') + @u_bar4 = create(:user, username: 'bar4') + + @u_tom = create(:user, username: 'tom') + @u_tom2 = create(:user, username: 'tom2') + end + + context 'when a user is directly addressed' do + it 'accesses the user object which is mentioned in the beginning of the line' do + subject.analyze('@foo What do you think? cc: @bar, @tom') + + expect(subject.directly_addressed_users).to match_array([@u_foo]) + end + + it "doesn't access the user object if it's not mentioned in the beginning of the line" do + subject.analyze('What do you think? cc: @bar') + + expect(subject.directly_addressed_users).to be_empty + end + end + + context 'when multiple users are addressed' do + it 'accesses the user objects which are mentioned in the beginning of the line' do + subject.analyze('@foo @bar What do you think? cc: @tom') + + expect(subject.directly_addressed_users).to match_array([@u_foo, @u_bar]) + end + + it "doesn't access the user objects if they are not mentioned in the beginning of the line" do + subject.analyze('What do you think? cc: @foo @bar @tom') + + expect(subject.directly_addressed_users).to be_empty + end + end + + context 'when multiple users are addressed in different paragraphs' do + it 'accesses user objects which are mentioned in the beginning of each paragraph' do + subject.analyze <<-NOTE.strip_heredoc + @foo What do you think? cc: @tom + + - @bar can you please have a look? + + >>> + @foo2 what do you think? cc: @bar2 + >>> + + @foo3 @foo4 thank you! + + > @foo5 well done! + + 1. @bar3 Can you please check? cc: @tom2 + 2. @bar4 What do you this of this MR? + NOTE + + expect(subject.directly_addressed_users).to match_array([@u_foo, @u_foo3, @u_foo4]) + end + end + end + it 'accesses valid issue objects' do @i0 = create(:issue, project: project) @i1 = create(:issue, project: project) subject.analyze("#{@i0.to_reference}, #{@i1.to_reference}, and #{Issue.reference_prefix}999.") + expect(subject.issues).to match_array([@i0, @i1]) end @@ -58,6 +129,7 @@ describe Gitlab::ReferenceExtractor, lib: true do @m1 = create(:merge_request, source_project: project, target_project: project, source_branch: 'feature_conflict') subject.analyze("!999, !#{@m1.iid}, and !#{@m0.iid}.") + expect(subject.merge_requests).to match_array([@m1, @m0]) end @@ -67,6 +139,7 @@ describe Gitlab::ReferenceExtractor, lib: true do @l2 = create(:label) subject.analyze("~#{@l0.id}, ~999, ~#{@l2.id}, ~#{@l1.id}") + expect(subject.labels).to match_array([@l0, @l1]) end @@ -76,6 +149,7 @@ describe Gitlab::ReferenceExtractor, lib: true do @s2 = create(:project_snippet) subject.analyze("$#{@s0.id}, $999, $#{@s2.id}, $#{@s1.id}") + expect(subject.snippets).to match_array([@s0, @s1]) end @@ -127,6 +201,7 @@ describe Gitlab::ReferenceExtractor, lib: true do it 'handles project issue references' do subject.analyze("this refers issue #{issue.to_reference(project)}") + extracted = subject.issues expect(extracted.size).to eq(1) expect(extracted).to match_array([issue]) diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index c78cd30157e..1dbc2f6eb13 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -2,47 +2,52 @@ require 'spec_helper' describe Gitlab::Regex, lib: true do - describe 'project path regex' do - it { expect('gitlab-ce').to match(Gitlab::Regex.project_path_regex) } - it { expect('gitlab_git').to match(Gitlab::Regex.project_path_regex) } - it { expect('_underscore.js').to match(Gitlab::Regex.project_path_regex) } - it { expect('100px.com').to match(Gitlab::Regex.project_path_regex) } - it { expect('?gitlab').not_to match(Gitlab::Regex.project_path_regex) } - it { expect('git lab').not_to match(Gitlab::Regex.project_path_regex) } - it { expect('gitlab.git').not_to match(Gitlab::Regex.project_path_regex) } - end + describe '.project_path_regex' do + subject { described_class.project_path_regex } - describe 'project name regex' do - it { expect('gitlab-ce').to match(Gitlab::Regex.project_name_regex) } - it { expect('GitLab CE').to match(Gitlab::Regex.project_name_regex) } - it { expect('100 lines').to match(Gitlab::Regex.project_name_regex) } - it { expect('gitlab.git').to match(Gitlab::Regex.project_name_regex) } - it { expect('ÄŒeský název').to match(Gitlab::Regex.project_name_regex) } - it { expect('Dash – is this').to match(Gitlab::Regex.project_name_regex) } - it { expect('?gitlab').not_to match(Gitlab::Regex.project_name_regex) } + it { is_expected.to match('gitlab-ce') } + it { is_expected.to match('gitlab_git') } + it { is_expected.to match('_underscore.js') } + it { is_expected.to match('100px.com') } + it { is_expected.not_to match('?gitlab') } + it { is_expected.not_to match('git lab') } + it { is_expected.not_to match('gitlab.git') } end - describe 'file name regex' do - it { expect('foo@bar').to match(Gitlab::Regex.file_name_regex) } + describe '.project_name_regex' do + subject { described_class.project_name_regex } + + it { is_expected.to match('gitlab-ce') } + it { is_expected.to match('GitLab CE') } + it { is_expected.to match('100 lines') } + it { is_expected.to match('gitlab.git') } + it { is_expected.to match('ÄŒeský název') } + it { is_expected.to match('Dash – is this') } + it { is_expected.not_to match('?gitlab') } end - describe 'file path regex' do - it { expect('foo@/bar').to match(Gitlab::Regex.file_path_regex) } + describe '.file_name_regex' do + subject { described_class.file_name_regex } + + it { is_expected.to match('foo@bar') } end - describe 'environment slug regex' do - def be_matched - match(Gitlab::Regex.environment_slug_regex) - end + describe '.file_path_regex' do + subject { described_class.file_path_regex } + + it { is_expected.to match('foo@/bar') } + end - it { expect('foo').to be_matched } - it { expect('foo-1').to be_matched } + describe '.environment_slug_regex' do + subject { described_class.environment_slug_regex } - it { expect('FOO').not_to be_matched } - it { expect('foo/1').not_to be_matched } - it { expect('foo.1').not_to be_matched } - it { expect('foo*1').not_to be_matched } - it { expect('9foo').not_to be_matched } - it { expect('foo-').not_to be_matched } + it { is_expected.to match('foo') } + it { is_expected.to match('foo-1') } + it { is_expected.not_to match('FOO') } + it { is_expected.not_to match('foo/1') } + it { is_expected.not_to match('foo.1') } + it { is_expected.not_to match('foo*1') } + it { is_expected.not_to match('9foo') } + it { is_expected.not_to match('foo-') } end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 9bfa6409607..838fd3754b2 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -351,6 +351,17 @@ describe Repository, models: true do expect(blob.data).to eq('Changelog!') end + it 'respects the autocrlf setting' do + repository.commit_file(user, 'hello.txt', "Hello,\r\nWorld", + message: 'Add hello world', + branch_name: 'master', + update: true) + + blob = repository.blob_at('master', 'hello.txt') + + expect(blob.data).to eq("Hello,\nWorld") + end + context "when an author is specified" do it "uses the given email/name to set the commit's author" do expect do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7fd49c73b37..89cef7ab978 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -19,6 +19,7 @@ describe User, models: true do it { is_expected.to have_many(:project_members).dependent(:destroy) } it { is_expected.to have_many(:groups) } it { is_expected.to have_many(:keys).dependent(:destroy) } + it { is_expected.to have_many(:deploy_keys).dependent(:destroy) } it { is_expected.to have_many(:events).dependent(:destroy) } it { is_expected.to have_many(:recent_events).class_name('Event') } it { is_expected.to have_many(:issues).dependent(:destroy) } @@ -303,6 +304,34 @@ describe User, models: true do end end + shared_context 'user keys' do + let(:user) { create(:user) } + let!(:key) { create(:key, user: user) } + let!(:deploy_key) { create(:deploy_key, user: user) } + end + + describe '#keys' do + include_context 'user keys' + + context 'with key and deploy key stored' do + it 'returns stored key, but not deploy_key' do + expect(user.keys).to include key + expect(user.keys).not_to include deploy_key + end + end + end + + describe '#deploy_keys' do + include_context 'user keys' + + context 'with key and deploy key stored' do + it 'returns stored deploy key, but not normal key' do + expect(user.deploy_keys).to include deploy_key + expect(user.deploy_keys).not_to include key + end + end + end + describe '#confirm' do before do allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) diff --git a/spec/models/wiki_directory_spec.rb b/spec/models/wiki_directory_spec.rb new file mode 100644 index 00000000000..1caaa557085 --- /dev/null +++ b/spec/models/wiki_directory_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +RSpec.describe WikiDirectory, models: true do + describe 'validations' do + subject { build(:wiki_directory) } + + it { is_expected.to validate_presence_of(:slug) } + end + + describe '#initialize' do + context 'when there are pages' do + let(:pages) { [build(:wiki_page)] } + let(:directory) { WikiDirectory.new('/path_up_to/dir', pages) } + + it 'sets the slug attribute' do + expect(directory.slug).to eq('/path_up_to/dir') + end + + it 'sets the pages attribute' do + expect(directory.pages).to eq(pages) + end + end + + context 'when there are no pages' do + let(:directory) { WikiDirectory.new('/path_up_to/dir') } + + it 'sets the slug attribute' do + expect(directory.slug).to eq('/path_up_to/dir') + end + + it 'sets the pages attribute to an empty array' do + expect(directory.pages).to eq([]) + end + end + end + + describe '#to_partial_path' do + it 'returns the relative path to the partial to be used' do + directory = build(:wiki_directory) + + expect(directory.to_partial_path).to eq('projects/wikis/wiki_directory') + end + end +end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index 5c34b1b0a30..579ebac7afb 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -7,6 +7,75 @@ describe WikiPage, models: true do subject { WikiPage.new(wiki) } + describe '.group_by_directory' do + context 'when there are no pages' do + it 'returns an empty array' do + expect(WikiPage.group_by_directory(nil)).to eq([]) + expect(WikiPage.group_by_directory([])).to eq([]) + end + end + + context 'when there are pages' do + before do + create_page('dir_1/dir_1_1/page_3', 'content') + create_page('dir_1/page_2', 'content') + create_page('dir_2/page_5', 'content') + create_page('dir_2/page_4', 'content') + create_page('page_1', 'content') + end + let(:page_1) { wiki.find_page('page_1') } + let(:dir_1) do + WikiDirectory.new('dir_1', [wiki.find_page('dir_1/page_2')]) + end + let(:dir_1_1) do + WikiDirectory.new('dir_1/dir_1_1', [wiki.find_page('dir_1/dir_1_1/page_3')]) + end + let(:dir_2) do + pages = [wiki.find_page('dir_2/page_5'), + wiki.find_page('dir_2/page_4')] + WikiDirectory.new('dir_2', pages) + end + + it 'returns an array with pages and directories' do + expected_grouped_entries = [page_1, dir_1, dir_1_1, dir_2] + + grouped_entries = WikiPage.group_by_directory(wiki.pages) + + grouped_entries.each_with_index do |page_or_dir, i| + expected_page_or_dir = expected_grouped_entries[i] + expected_slugs = get_slugs(expected_page_or_dir) + slugs = get_slugs(page_or_dir) + + expect(slugs).to match_array(expected_slugs) + end + end + + it 'returns an array sorted by alphabetical position' do + # Directories and pages within directories are sorted alphabetically. + # Pages at root come before everything. + expected_order = ['page_1', 'dir_1/page_2', 'dir_1/dir_1_1/page_3', + 'dir_2/page_4', 'dir_2/page_5'] + + grouped_entries = WikiPage.group_by_directory(wiki.pages) + + actual_order = + grouped_entries.map do |page_or_dir| + get_slugs(page_or_dir) + end. + flatten + expect(actual_order).to eq(expected_order) + end + end + end + + describe '.unhyphenize' do + it 'removes hyphens from a name' do + name = 'a-name--with-hyphens' + + expect(WikiPage.unhyphenize(name)).to eq('a name with hyphens') + end + end + describe "#initialize" do context "when initialized with an existing gollum page" do before do @@ -189,6 +258,26 @@ describe WikiPage, models: true do end end + describe '#directory' do + context 'when the page is at the root directory' do + it 'returns an empty string' do + create_page('file', 'content') + page = wiki.find_page('file') + + expect(page.directory).to eq('') + end + end + + context 'when the page is inside an actual directory' do + it 'returns the full directory hierarchy' do + create_page('dir_1/dir_1_1/file', 'content') + page = wiki.find_page('dir_1/dir_1_1/file') + + expect(page.directory).to eq('dir_1/dir_1_1') + end + end + end + describe '#historical?' do before do create_page('Update', 'content') @@ -221,6 +310,14 @@ describe WikiPage, models: true do end end + describe '#to_partial_path' do + it 'returns the relative path to the partial to be used' do + page = build(:wiki_page) + + expect(page.to_partial_path).to eq('projects/wikis/wiki_page') + end + end + private def remove_temp_repo(path) @@ -239,4 +336,12 @@ describe WikiPage, models: true do page = wiki.wiki.paged(title) wiki.wiki.delete_page(page, commit_details) end + + def get_slugs(page_or_dir) + if page_or_dir.is_a? WikiPage + [page_or_dir.slug] + else + page_or_dir.pages.present? ? page_or_dir.pages.map(&:slug) : [] + end + end end diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 9892e014cb9..3e9bcfd1a60 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -145,11 +145,11 @@ describe API::Members, api: true do end end - it "returns #{source_type == 'project' ? 201 : 409} if member already exists" do + it "returns 409 if member already exists" do post api("/#{source_type.pluralize}/#{source.id}/members", master), user_id: master.id, access_level: Member::MASTER - expect(response).to have_http_status(source_type == 'project' ? 201 : 409) + expect(response).to have_http_status(409) end it 'returns 400 when user_id is not given' do diff --git a/spec/requests/api/templates_spec.rb b/spec/requests/api/templates_spec.rb index d32ba60fc4c..c0a8c0832bb 100644 --- a/spec/requests/api/templates_spec.rb +++ b/spec/requests/api/templates_spec.rb @@ -3,23 +3,23 @@ require 'spec_helper' describe API::Templates, api: true do include ApiHelpers - shared_examples_for 'the Template Entity' do |path| - before { get api(path) } + context 'the Template Entity' do + before { get api('/templates/gitignores/Ruby') } it { expect(json_response['name']).to eq('Ruby') } it { expect(json_response['content']).to include('*.gem') } end - - shared_examples_for 'the TemplateList Entity' do |path| - before { get api(path) } + + context 'the TemplateList Entity' do + before { get api('/templates/gitignores') } it { expect(json_response.first['name']).not_to be_nil } it { expect(json_response.first['content']).to be_nil } end - shared_examples_for 'requesting gitignores' do |path| + context 'requesting gitignores' do it 'returns a list of available gitignore templates' do - get api(path) + get api('/templates/gitignores') expect(response).to have_http_status(200) expect(json_response).to be_an Array @@ -27,9 +27,9 @@ describe API::Templates, api: true do end end - shared_examples_for 'requesting gitlab-ci-ymls' do |path| + context 'requesting gitlab-ci-ymls' do it 'returns a list of available gitlab_ci_ymls' do - get api(path) + get api('/templates/gitlab_ci_ymls') expect(response).to have_http_status(200) expect(json_response).to be_an Array @@ -37,17 +37,17 @@ describe API::Templates, api: true do end end - shared_examples_for 'requesting gitlab-ci-yml for Ruby' do |path| + context 'requesting gitlab-ci-yml for Ruby' do it 'adds a disclaimer on the top' do - get api(path) + get api('/templates/gitlab_ci_ymls/Ruby') expect(response).to have_http_status(200) expect(json_response['content']).to start_with("# This file is a template,") end end - shared_examples_for 'the License Template Entity' do |path| - before { get api(path) } + context 'the License Template Entity' do + before { get api('/templates/licenses/mit') } it 'returns a license template' do expect(json_response['key']).to eq('mit') @@ -64,9 +64,9 @@ describe API::Templates, api: true do end end - shared_examples_for 'GET licenses' do |path| + context 'GET templates/licenses' do it 'returns a list of available license templates' do - get api(path) + get api('/templates/licenses') expect(response).to have_http_status(200) expect(json_response).to be_an Array @@ -77,7 +77,7 @@ describe API::Templates, api: true do describe 'the popular parameter' do context 'with popular=1' do it 'returns a list of available popular license templates' do - get api("#{path}?popular=1") + get api('/templates/licenses?popular=1') expect(response).to have_http_status(200) expect(json_response).to be_an Array @@ -88,10 +88,10 @@ describe API::Templates, api: true do end end - shared_examples_for 'GET licenses/:name' do |path| + context 'GET templates/licenses/:name' do context 'with :project and :fullname given' do before do - get api("#{path}/#{license_type}?project=My+Awesome+Project&fullname=Anton+#{license_type.upcase}") + get api("/templates/licenses/#{license_type}?project=My+Awesome+Project&fullname=Anton+#{license_type.upcase}") end context 'for the mit license' do @@ -178,26 +178,4 @@ describe API::Templates, api: true do end end end - - describe 'with /templates namespace' do - it_behaves_like 'the Template Entity', '/templates/gitignores/Ruby' - it_behaves_like 'the TemplateList Entity', '/templates/gitignores' - it_behaves_like 'requesting gitignores', '/templates/gitignores' - it_behaves_like 'requesting gitlab-ci-ymls', '/templates/gitlab_ci_ymls' - it_behaves_like 'requesting gitlab-ci-yml for Ruby', '/templates/gitlab_ci_ymls/Ruby' - it_behaves_like 'the License Template Entity', '/templates/licenses/mit' - it_behaves_like 'GET licenses', '/templates/licenses' - it_behaves_like 'GET licenses/:name', '/templates/licenses' - end - - describe 'without /templates namespace' do - it_behaves_like 'the Template Entity', '/gitignores/Ruby' - it_behaves_like 'the TemplateList Entity', '/gitignores' - it_behaves_like 'requesting gitignores', '/gitignores' - it_behaves_like 'requesting gitlab-ci-ymls', '/gitlab_ci_ymls' - it_behaves_like 'requesting gitlab-ci-yml for Ruby', '/gitlab_ci_ymls/Ruby' - it_behaves_like 'the License Template Entity', '/licenses/mit' - it_behaves_like 'GET licenses', '/licenses' - it_behaves_like 'GET licenses/:name', '/licenses' - end end diff --git a/spec/requests/api/v3/members_spec.rb b/spec/requests/api/v3/members_spec.rb new file mode 100644 index 00000000000..28c3ca03960 --- /dev/null +++ b/spec/requests/api/v3/members_spec.rb @@ -0,0 +1,342 @@ +require 'spec_helper' + +describe API::Members, api: true do + include ApiHelpers + + let(:master) { create(:user) } + let(:developer) { create(:user) } + let(:access_requester) { create(:user) } + let(:stranger) { create(:user) } + + let(:project) do + create(:empty_project, :public, :access_requestable, creator_id: master.id, namespace: master.namespace) do |project| + project.team << [developer, :developer] + project.team << [master, :master] + project.request_access(access_requester) + end + end + + let!(:group) do + create(:group, :public, :access_requestable) do |group| + group.add_developer(developer) + group.add_owner(master) + group.request_access(access_requester) + end + end + + shared_examples 'GET /:sources/:id/members' do |source_type| + context "with :sources == #{source_type.pluralize}" do + it_behaves_like 'a 404 response when source is private' do + let(:route) { get v3_api("/#{source_type.pluralize}/#{source.id}/members", stranger) } + end + + %i[master developer access_requester stranger].each do |type| + context "when authenticated as a #{type}" do + it 'returns 200' do + user = public_send(type) + get v3_api("/#{source_type.pluralize}/#{source.id}/members", user) + + expect(response).to have_http_status(200) + expect(json_response.size).to eq(2) + expect(json_response.map { |u| u['id'] }).to match_array [master.id, developer.id] + end + end + end + + it 'does not return invitees' do + create(:"#{source_type}_member", invite_token: '123', invite_email: 'test@abc.com', source: source, user: nil) + + get v3_api("/#{source_type.pluralize}/#{source.id}/members", developer) + + expect(response).to have_http_status(200) + expect(json_response.size).to eq(2) + expect(json_response.map { |u| u['id'] }).to match_array [master.id, developer.id] + end + + it 'finds members with query string' do + get v3_api("/#{source_type.pluralize}/#{source.id}/members", developer), query: master.username + + expect(response).to have_http_status(200) + expect(json_response.count).to eq(1) + expect(json_response.first['username']).to eq(master.username) + end + end + end + + shared_examples 'GET /:sources/:id/members/:user_id' do |source_type| + context "with :sources == #{source_type.pluralize}" do + it_behaves_like 'a 404 response when source is private' do + let(:route) { get v3_api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger) } + end + + context 'when authenticated as a non-member' do + %i[access_requester stranger].each do |type| + context "as a #{type}" do + it 'returns 200' do + user = public_send(type) + get v3_api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user) + + expect(response).to have_http_status(200) + # User attributes + expect(json_response['id']).to eq(developer.id) + expect(json_response['name']).to eq(developer.name) + expect(json_response['username']).to eq(developer.username) + expect(json_response['state']).to eq(developer.state) + expect(json_response['avatar_url']).to eq(developer.avatar_url) + expect(json_response['web_url']).to eq(Gitlab::Routing.url_helpers.user_url(developer)) + + # Member attributes + expect(json_response['access_level']).to eq(Member::DEVELOPER) + end + end + end + end + end + end + + shared_examples 'POST /:sources/:id/members' do |source_type| + context "with :sources == #{source_type.pluralize}" do + it_behaves_like 'a 404 response when source is private' do + let(:route) do + post v3_api("/#{source_type.pluralize}/#{source.id}/members", stranger), + user_id: access_requester.id, access_level: Member::MASTER + end + end + + context 'when authenticated as a non-member or member with insufficient rights' do + %i[access_requester stranger developer].each do |type| + context "as a #{type}" do + it 'returns 403' do + user = public_send(type) + post v3_api("/#{source_type.pluralize}/#{source.id}/members", user), + user_id: access_requester.id, access_level: Member::MASTER + + expect(response).to have_http_status(403) + end + end + end + end + + context 'when authenticated as a master/owner' do + context 'and new member is already a requester' do + it 'transforms the requester into a proper member' do + expect do + post v3_api("/#{source_type.pluralize}/#{source.id}/members", master), + user_id: access_requester.id, access_level: Member::MASTER + + expect(response).to have_http_status(201) + end.to change { source.members.count }.by(1) + expect(source.requesters.count).to eq(0) + expect(json_response['id']).to eq(access_requester.id) + expect(json_response['access_level']).to eq(Member::MASTER) + end + end + + it 'creates a new member' do + expect do + post v3_api("/#{source_type.pluralize}/#{source.id}/members", master), + user_id: stranger.id, access_level: Member::DEVELOPER, expires_at: '2016-08-05' + + expect(response).to have_http_status(201) + end.to change { source.members.count }.by(1) + expect(json_response['id']).to eq(stranger.id) + expect(json_response['access_level']).to eq(Member::DEVELOPER) + expect(json_response['expires_at']).to eq('2016-08-05') + end + end + + it "returns #{source_type == 'project' ? 201 : 409} if member already exists" do + post v3_api("/#{source_type.pluralize}/#{source.id}/members", master), + user_id: master.id, access_level: Member::MASTER + + expect(response).to have_http_status(source_type == 'project' ? 201 : 409) + end + + it 'returns 400 when user_id is not given' do + post v3_api("/#{source_type.pluralize}/#{source.id}/members", master), + access_level: Member::MASTER + + expect(response).to have_http_status(400) + end + + it 'returns 400 when access_level is not given' do + post v3_api("/#{source_type.pluralize}/#{source.id}/members", master), + user_id: stranger.id + + expect(response).to have_http_status(400) + end + + it 'returns 422 when access_level is not valid' do + post v3_api("/#{source_type.pluralize}/#{source.id}/members", master), + user_id: stranger.id, access_level: 1234 + + expect(response).to have_http_status(422) + end + end + end + + shared_examples 'PUT /:sources/:id/members/:user_id' do |source_type| + context "with :sources == #{source_type.pluralize}" do + it_behaves_like 'a 404 response when source is private' do + let(:route) do + put v3_api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger), + access_level: Member::MASTER + end + end + + context 'when authenticated as a non-member or member with insufficient rights' do + %i[access_requester stranger developer].each do |type| + context "as a #{type}" do + it 'returns 403' do + user = public_send(type) + put v3_api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user), + access_level: Member::MASTER + + expect(response).to have_http_status(403) + end + end + end + end + + context 'when authenticated as a master/owner' do + it 'updates the member' do + put v3_api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master), + access_level: Member::MASTER, expires_at: '2016-08-05' + + expect(response).to have_http_status(200) + expect(json_response['id']).to eq(developer.id) + expect(json_response['access_level']).to eq(Member::MASTER) + expect(json_response['expires_at']).to eq('2016-08-05') + end + end + + it 'returns 409 if member does not exist' do + put v3_api("/#{source_type.pluralize}/#{source.id}/members/123", master), + access_level: Member::MASTER + + expect(response).to have_http_status(404) + end + + it 'returns 400 when access_level is not given' do + put v3_api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master) + + expect(response).to have_http_status(400) + end + + it 'returns 422 when access level is not valid' do + put v3_api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master), + access_level: 1234 + + expect(response).to have_http_status(422) + end + end + end + + shared_examples 'DELETE /:sources/:id/members/:user_id' do |source_type| + context "with :sources == #{source_type.pluralize}" do + it_behaves_like 'a 404 response when source is private' do + let(:route) { delete v3_api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger) } + end + + context 'when authenticated as a non-member or member with insufficient rights' do + %i[access_requester stranger].each do |type| + context "as a #{type}" do + it 'returns 403' do + user = public_send(type) + delete v3_api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user) + + expect(response).to have_http_status(403) + end + end + end + end + + context 'when authenticated as a member and deleting themself' do + it 'deletes the member' do + expect do + delete v3_api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", developer) + + expect(response).to have_http_status(200) + end.to change { source.members.count }.by(-1) + end + end + + context 'when authenticated as a master/owner' do + context 'and member is a requester' do + it "returns #{source_type == 'project' ? 200 : 404}" do + expect do + delete v3_api("/#{source_type.pluralize}/#{source.id}/members/#{access_requester.id}", master) + + expect(response).to have_http_status(source_type == 'project' ? 200 : 404) + end.not_to change { source.requesters.count } + end + end + + it 'deletes the member' do + expect do + delete v3_api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master) + + expect(response).to have_http_status(200) + end.to change { source.members.count }.by(-1) + end + end + + it "returns #{source_type == 'project' ? 200 : 404} if member does not exist" do + delete v3_api("/#{source_type.pluralize}/#{source.id}/members/123", master) + + expect(response).to have_http_status(source_type == 'project' ? 200 : 404) + end + end + end + + it_behaves_like 'GET /:sources/:id/members', 'project' do + let(:source) { project } + end + + it_behaves_like 'GET /:sources/:id/members', 'group' do + let(:source) { group } + end + + it_behaves_like 'GET /:sources/:id/members/:user_id', 'project' do + let(:source) { project } + end + + it_behaves_like 'GET /:sources/:id/members/:user_id', 'group' do + let(:source) { group } + end + + it_behaves_like 'POST /:sources/:id/members', 'project' do + let(:source) { project } + end + + it_behaves_like 'POST /:sources/:id/members', 'group' do + let(:source) { group } + end + + it_behaves_like 'PUT /:sources/:id/members/:user_id', 'project' do + let(:source) { project } + end + + it_behaves_like 'PUT /:sources/:id/members/:user_id', 'group' do + let(:source) { group } + end + + it_behaves_like 'DELETE /:sources/:id/members/:user_id', 'project' do + let(:source) { project } + end + + it_behaves_like 'DELETE /:sources/:id/members/:user_id', 'group' do + let(:source) { group } + end + + context 'Adding owner to project' do + it 'returns 403' do + expect do + post v3_api("/projects/#{project.id}/members", master), + user_id: stranger.id, access_level: Member::OWNER + + expect(response).to have_http_status(422) + end.to change { project.members.count }.by(0) + end + end +end diff --git a/spec/requests/api/v3/templates_spec.rb b/spec/requests/api/v3/templates_spec.rb new file mode 100644 index 00000000000..4fd4e70bedd --- /dev/null +++ b/spec/requests/api/v3/templates_spec.rb @@ -0,0 +1,203 @@ +require 'spec_helper' + +describe API::V3::Templates, api: true do + include ApiHelpers + + shared_examples_for 'the Template Entity' do |path| + before { get v3_api(path) } + + it { expect(json_response['name']).to eq('Ruby') } + it { expect(json_response['content']).to include('*.gem') } + end + + shared_examples_for 'the TemplateList Entity' do |path| + before { get v3_api(path) } + + it { expect(json_response.first['name']).not_to be_nil } + it { expect(json_response.first['content']).to be_nil } + end + + shared_examples_for 'requesting gitignores' do |path| + it 'returns a list of available gitignore templates' do + get v3_api(path) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.size).to be > 15 + end + end + + shared_examples_for 'requesting gitlab-ci-ymls' do |path| + it 'returns a list of available gitlab_ci_ymls' do + get v3_api(path) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first['name']).not_to be_nil + end + end + + shared_examples_for 'requesting gitlab-ci-yml for Ruby' do |path| + it 'adds a disclaimer on the top' do + get v3_api(path) + + expect(response).to have_http_status(200) + expect(json_response['content']).to start_with("# This file is a template,") + end + end + + shared_examples_for 'the License Template Entity' do |path| + before { get v3_api(path) } + + it 'returns a license template' do + expect(json_response['key']).to eq('mit') + expect(json_response['name']).to eq('MIT License') + expect(json_response['nickname']).to be_nil + expect(json_response['popular']).to be true + expect(json_response['html_url']).to eq('http://choosealicense.com/licenses/mit/') + expect(json_response['source_url']).to eq('https://opensource.org/licenses/MIT') + expect(json_response['description']).to include('A permissive license that is short and to the point.') + expect(json_response['conditions']).to eq(%w[include-copyright]) + expect(json_response['permissions']).to eq(%w[commercial-use modifications distribution private-use]) + expect(json_response['limitations']).to eq(%w[no-liability]) + expect(json_response['content']).to include('The MIT License (MIT)') + end + end + + shared_examples_for 'GET licenses' do |path| + it 'returns a list of available license templates' do + get v3_api(path) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.size).to eq(15) + expect(json_response.map { |l| l['key'] }).to include('agpl-3.0') + end + + describe 'the popular parameter' do + context 'with popular=1' do + it 'returns a list of available popular license templates' do + get v3_api("#{path}?popular=1") + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.size).to eq(3) + expect(json_response.map { |l| l['key'] }).to include('apache-2.0') + end + end + end + end + + shared_examples_for 'GET licenses/:name' do |path| + context 'with :project and :fullname given' do + before do + get v3_api("#{path}/#{license_type}?project=My+Awesome+Project&fullname=Anton+#{license_type.upcase}") + end + + context 'for the mit license' do + let(:license_type) { 'mit' } + + it 'returns the license text' do + expect(json_response['content']).to include('The MIT License (MIT)') + end + + it 'replaces placeholder values' do + expect(json_response['content']).to include("Copyright (c) #{Time.now.year} Anton") + end + end + + context 'for the agpl-3.0 license' do + let(:license_type) { 'agpl-3.0' } + + it 'returns the license text' do + expect(json_response['content']).to include('GNU AFFERO GENERAL PUBLIC LICENSE') + end + + it 'replaces placeholder values' do + expect(json_response['content']).to include('My Awesome Project') + expect(json_response['content']).to include("Copyright (C) #{Time.now.year} Anton") + end + end + + context 'for the gpl-3.0 license' do + let(:license_type) { 'gpl-3.0' } + + it 'returns the license text' do + expect(json_response['content']).to include('GNU GENERAL PUBLIC LICENSE') + end + + it 'replaces placeholder values' do + expect(json_response['content']).to include('My Awesome Project') + expect(json_response['content']).to include("Copyright (C) #{Time.now.year} Anton") + end + end + + context 'for the gpl-2.0 license' do + let(:license_type) { 'gpl-2.0' } + + it 'returns the license text' do + expect(json_response['content']).to include('GNU GENERAL PUBLIC LICENSE') + end + + it 'replaces placeholder values' do + expect(json_response['content']).to include('My Awesome Project') + expect(json_response['content']).to include("Copyright (C) #{Time.now.year} Anton") + end + end + + context 'for the apache-2.0 license' do + let(:license_type) { 'apache-2.0' } + + it 'returns the license text' do + expect(json_response['content']).to include('Apache License') + end + + it 'replaces placeholder values' do + expect(json_response['content']).to include("Copyright #{Time.now.year} Anton") + end + end + + context 'for an uknown license' do + let(:license_type) { 'muth-over9000' } + + it 'returns a 404' do + expect(response).to have_http_status(404) + end + end + end + + context 'with no :fullname given' do + context 'with an authenticated user' do + let(:user) { create(:user) } + + it 'replaces the copyright owner placeholder with the name of the current user' do + get v3_api('/templates/licenses/mit', user) + + expect(json_response['content']).to include("Copyright (c) #{Time.now.year} #{user.name}") + end + end + end + end + + describe 'with /templates namespace' do + it_behaves_like 'the Template Entity', '/templates/gitignores/Ruby' + it_behaves_like 'the TemplateList Entity', '/templates/gitignores' + it_behaves_like 'requesting gitignores', '/templates/gitignores' + it_behaves_like 'requesting gitlab-ci-ymls', '/templates/gitlab_ci_ymls' + it_behaves_like 'requesting gitlab-ci-yml for Ruby', '/templates/gitlab_ci_ymls/Ruby' + it_behaves_like 'the License Template Entity', '/templates/licenses/mit' + it_behaves_like 'GET licenses', '/templates/licenses' + it_behaves_like 'GET licenses/:name', '/templates/licenses' + end + + describe 'without /templates namespace' do + it_behaves_like 'the Template Entity', '/gitignores/Ruby' + it_behaves_like 'the TemplateList Entity', '/gitignores' + it_behaves_like 'requesting gitignores', '/gitignores' + it_behaves_like 'requesting gitlab-ci-ymls', '/gitlab_ci_ymls' + it_behaves_like 'requesting gitlab-ci-yml for Ruby', '/gitlab_ci_ymls/Ruby' + it_behaves_like 'the License Template Entity', '/licenses/mit' + it_behaves_like 'GET licenses', '/licenses' + it_behaves_like 'GET licenses/:name', '/licenses' + end +end diff --git a/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb b/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb new file mode 100644 index 00000000000..7cb24dc5646 --- /dev/null +++ b/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/migration/add_concurrent_foreign_key' + +describe RuboCop::Cop::Migration::AddConcurrentForeignKey do + include CopHelper + + let(:cop) { described_class.new } + + context 'outside of a migration' do + it 'does not register any offenses' do + inspect_source(cop, 'def up; add_foreign_key(:projects, :users, column: :user_id); end') + + expect(cop.offenses).to be_empty + end + end + + context 'in a migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + it 'registers an offense when using add_foreign_key' do + inspect_source(cop, 'def up; add_foreign_key(:projects, :users, column: :user_id); end') + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end + end +end diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb index 4cfba35c830..09807e5d35b 100644 --- a/spec/services/issues/build_service_spec.rb +++ b/spec/services/issues/build_service_spec.rb @@ -120,11 +120,20 @@ describe Issues::BuildService, services: true do end describe '#execute' do + let(:milestone) { create(:milestone, project: project) } + it 'builds a new issues with given params' do - issue = described_class.new(project, user, title: 'Issue #1', description: 'Issue description').execute + issue = described_class.new( + project, + user, + title: 'Issue #1', + description: 'Issue description', + milestone_id: milestone.id, + ).execute expect(issue.title).to eq('Issue #1') expect(issue.description).to eq('Issue description') + expect(issue.milestone).to eq(milestone) end end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 30578ee4c7d..e1feeed8a67 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -46,6 +46,7 @@ describe Issues::CreateService, services: true do expect(issue).to be_persisted expect(issue.title).to eq('Awesome issue') + expect(issue.description).to eq('please fix') expect(issue.assignee).to be_nil expect(issue.labels).to be_empty expect(issue.milestone).to be_nil diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 2cc21acab7b..983dac6efdb 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -287,41 +287,64 @@ describe MergeRequests::RefreshService, services: true do it 'references the commit that caused the Work in Progress status' do refresh_service.execute(@oldrev, @newrev, 'refs/heads/master') - allow(refresh_service).to receive(:find_new_commits) refresh_service.instance_variable_set("@commits", [ - instance_double( - Commit, + double( id: 'aaaaaaa', + sha: '38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e', short_id: 'aaaaaaa', title: 'Fix issue', work_in_progress?: false ), - instance_double( - Commit, + double( id: 'bbbbbbb', + sha: '498214de67004b1da3d820901307bed2a68a8ef6', short_id: 'bbbbbbb', title: 'fixup! Fix issue', work_in_progress?: true, to_reference: 'bbbbbbb' ), - instance_double( - Commit, + double( id: 'ccccccc', + sha: '1b12f15a11fc6e62177bef08f47bc7b5ce50b141', short_id: 'ccccccc', title: 'fixup! Fix issue', work_in_progress?: true, to_reference: 'ccccccc' ), ]) - refresh_service.execute(@oldrev, @newrev, 'refs/heads/wip') reload_mrs - expect(@merge_request.notes.last.note).to eq( "marked as a **Work In Progress** from bbbbbbb" ) end + + it 'does not mark as WIP based on commits that do not belong to an MR' do + allow(refresh_service).to receive(:find_new_commits) + refresh_service.instance_variable_set("@commits", [ + double( + id: 'aaaaaaa', + sha: 'aaaaaaa', + short_id: 'aaaaaaa', + title: 'Fix issue', + work_in_progress?: false + ), + double( + id: 'bbbbbbb', + sha: 'bbbbbbbb', + short_id: 'bbbbbbb', + title: 'fixup! Fix issue', + work_in_progress?: true, + to_reference: 'bbbbbbb' + ) + ]) + + refresh_service.execute(@oldrev, @newrev, 'refs/heads/master') + reload_mrs + + expect(@merge_request.work_in_progress?).to be_falsey + end end def reload_mrs diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 7cf2cd9968f..839250b7d84 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -146,6 +146,16 @@ describe NotificationService, services: true do should_not_email(@u_lazy_participant) end + it "emails the note author if they've opted into notifications about their activity" do + add_users_with_subscription(note.project, issue) + note.author.notified_of_own_activity = true + reset_delivered_emails! + + notification.new_note(note) + + should_email(note.author) + end + it 'filters out "mentioned in" notes' do mentioned_note = SystemNoteService.cross_reference(mentioned_issue, issue, issue.author) @@ -476,6 +486,20 @@ describe NotificationService, services: true do should_not_email(issue.assignee) end + it "emails the author if they've opted into notifications about their activity" do + issue.author.notified_of_own_activity = true + + notification.new_issue(issue, issue.author) + + should_email(issue.author) + end + + it "doesn't email the author if they haven't opted into notifications about their activity" do + notification.new_issue(issue, issue.author) + + should_not_email(issue.author) + end + it "emails subscribers of the issue's labels" do user_1 = create(:user) user_2 = create(:user) @@ -665,6 +689,19 @@ describe NotificationService, services: true do should_email(subscriber_to_label_2) end + it "emails the current user if they've opted into notifications about their activity" do + subscriber_to_label_2.notified_of_own_activity = true + notification.relabeled_issue(issue, [group_label_2, label_2], subscriber_to_label_2) + + should_email(subscriber_to_label_2) + end + + it "doesn't email the current user if they haven't opted into notifications about their activity" do + notification.relabeled_issue(issue, [group_label_2, label_2], subscriber_to_label_2) + + should_not_email(subscriber_to_label_2) + end + it "doesn't send email to anyone but subscribers of the given labels" do notification.relabeled_issue(issue, [group_label_2, label_2], @u_disabled) @@ -818,6 +855,20 @@ describe NotificationService, services: true do should_not_email(@u_lazy_participant) end + it "emails the author if they've opted into notifications about their activity" do + merge_request.author.notified_of_own_activity = true + + notification.new_merge_request(merge_request, merge_request.author) + + should_email(merge_request.author) + end + + it "doesn't email the author if they haven't opted into notifications about their activity" do + notification.new_merge_request(merge_request, merge_request.author) + + should_not_email(merge_request.author) + end + it "emails subscribers of the merge request's labels" do user_1 = create(:user) user_2 = create(:user) @@ -1013,6 +1064,14 @@ describe NotificationService, services: true do should_not_email(@u_watcher) end + it "notifies the merger when merge_when_build_succeeds is false but they've opted into notifications about their activity" do + merge_request.merge_when_build_succeeds = false + @u_watcher.notified_of_own_activity = true + notification.merge_mr(merge_request, @u_watcher) + + should_email(@u_watcher) + end + it_behaves_like 'participating notifications' do let(:participant) { create(:user, username: 'user-participant') } let(:issuable) { merge_request } diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 13d584a8975..4320365ab57 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -9,7 +9,9 @@ describe TodoService, services: true do let(:admin) { create(:admin) } let(:john_doe) { create(:user) } let(:project) { create(:project) } - let(:mentions) { [author, assignee, john_doe, member, guest, non_member, admin].map(&:to_reference).join(' ') } + let(:mentions) { 'FYI: ' + [author, assignee, john_doe, member, guest, non_member, admin].map(&:to_reference).join(' ') } + let(:directly_addressed) { [author, assignee, john_doe, member, guest, non_member, admin].map(&:to_reference).join(' ') } + let(:directly_addressed_and_mentioned) { member.to_reference + ", what do you think? cc: " + [guest, admin].map(&:to_reference).join(' ') } let(:service) { described_class.new } before do @@ -21,8 +23,10 @@ describe TodoService, services: true do describe 'Issues' do let(:issue) { create(:issue, project: project, assignee: john_doe, author: author, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") } + let(:addressed_issue) { create(:issue, project: project, assignee: john_doe, author: author, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") } let(:unassigned_issue) { create(:issue, project: project, assignee: nil) } let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee, description: mentions) } + let(:addressed_confident_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee, description: directly_addressed) } describe '#new_issue' do it 'creates a todo if assigned' do @@ -52,6 +56,26 @@ describe TodoService, services: true do should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED) end + it 'creates a directly addressed todo for each valid addressed user' do + service.new_issue(addressed_issue, author) + + should_create_todo(user: member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: guest, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: author, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: john_doe, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: non_member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + end + + it 'creates correct todos for each valid user based on the type of mention' do + issue.update(description: directly_addressed_and_mentioned) + + service.new_issue(issue, author) + + should_create_todo(user: member, target: issue, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: admin, target: issue, action: Todo::MENTIONED) + should_create_todo(user: guest, target: issue, action: Todo::MENTIONED) + end + it 'does not create todo if user can not see the issue when issue is confidential' do service.new_issue(confidential_issue, john_doe) @@ -63,6 +87,17 @@ describe TodoService, services: true do should_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED) end + it 'does not create directly addressed todo if user cannot see the issue when issue is confidential' do + service.new_issue(addressed_confident_issue, john_doe) + + should_create_todo(user: assignee, target: addressed_confident_issue, author: john_doe, action: Todo::ASSIGNED) + should_create_todo(user: author, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: member, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: admin, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: guest, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: john_doe, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED) + end + context 'when a private group is mentioned' do let(:group) { create :group, :private } let(:project) { create :project, :private, group: group } @@ -94,12 +129,38 @@ describe TodoService, services: true do should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED) end + it 'creates a todo for each valid user based on the type of mention' do + issue.update(description: directly_addressed_and_mentioned) + + service.update_issue(issue, author) + + should_create_todo(user: member, target: issue, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: guest, target: issue, action: Todo::MENTIONED) + should_create_todo(user: admin, target: issue, action: Todo::MENTIONED) + end + + it 'creates a directly addressed todo for each valid addressed user' do + service.update_issue(addressed_issue, author) + + should_create_todo(user: member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: guest, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: john_doe, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: author, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: non_member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + end + it 'does not create a todo if user was already mentioned' do create(:todo, :mentioned, user: member, project: project, target: issue, author: author) expect { service.update_issue(issue, author) }.not_to change(member.todos, :count) end + it 'does not create a directly addressed todo if user was already mentioned or addressed' do + create(:todo, :directly_addressed, user: member, project: project, target: addressed_issue, author: author) + + expect { service.update_issue(addressed_issue, author) }.not_to change(member.todos, :count) + end + it 'does not create todo if user can not see the issue when issue is confidential' do service.update_issue(confidential_issue, john_doe) @@ -111,6 +172,17 @@ describe TodoService, services: true do should_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED) end + it 'does not create a directly addressed todo if user can not see the issue when issue is confidential' do + service.update_issue(addressed_confident_issue, john_doe) + + should_create_todo(user: author, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: assignee, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: member, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: admin, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: guest, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: john_doe, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED) + end + context 'issues with a task list' do it 'does not create todo when tasks are marked as completed' do issue.update(description: "- [x] Task 1\n- [X] Task 2 #{mentions}") @@ -125,6 +197,19 @@ describe TodoService, services: true do should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED) end + it 'does not create directly addressed todo when tasks are marked as completed' do + addressed_issue.update(description: "#{directly_addressed}\n- [x] Task 1\n- [x] Task 2\n") + + service.update_issue(addressed_issue, author) + + should_not_create_todo(user: admin, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: assignee, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: author, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: john_doe, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: non_member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + end + it 'does not raise an error when description not change' do issue.update(title: 'Sample') @@ -244,8 +329,11 @@ describe TodoService, services: true do let!(:second_todo) { create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) } let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) } let(:note) { create(:note, project: project, noteable: issue, author: john_doe, note: mentions) } + let(:addressed_note) { create(:note, project: project, noteable: issue, author: john_doe, note: directly_addressed) } let(:note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: mentions) } + let(:addressed_note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: directly_addressed) } let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project, note: mentions) } + let(:addressed_note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project, note: directly_addressed) } let(:note_on_project_snippet) { create(:note_on_project_snippet, project: project, author: john_doe, note: mentions) } let(:system_note) { create(:system_note, project: project, noteable: issue) } @@ -276,6 +364,26 @@ describe TodoService, services: true do should_not_create_todo(user: non_member, target: issue, author: john_doe, action: Todo::MENTIONED, note: note) end + it 'creates a todo for each valid user based on the type of mention' do + note.update(note: directly_addressed_and_mentioned) + + service.new_note(note, john_doe) + + should_create_todo(user: member, target: issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: note) + should_create_todo(user: admin, target: issue, author: john_doe, action: Todo::MENTIONED, note: note) + should_create_todo(user: guest, target: issue, author: john_doe, action: Todo::MENTIONED, note: note) + end + + it 'creates a directly addressed todo for each valid addressed user' do + service.new_note(addressed_note, john_doe) + + should_create_todo(user: member, target: issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note) + should_create_todo(user: guest, target: issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note) + should_create_todo(user: author, target: issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note) + should_create_todo(user: john_doe, target: issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note) + should_not_create_todo(user: non_member, target: issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note) + end + it 'does not create todo if user can not see the issue when leaving a note on a confidential issue' do service.new_note(note_on_confidential_issue, john_doe) @@ -287,6 +395,17 @@ describe TodoService, services: true do should_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue) end + it 'does not create a directly addressed todo if user can not see the issue when leaving a note on a confidential issue' do + service.new_note(addressed_note_on_confidential_issue, john_doe) + + should_create_todo(user: author, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue) + should_create_todo(user: assignee, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue) + should_create_todo(user: member, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue) + should_create_todo(user: admin, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue) + should_not_create_todo(user: guest, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue) + should_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue) + end + it 'creates a todo for each valid mentioned user when leaving a note on commit' do service.new_note(note_on_commit, john_doe) @@ -296,6 +415,15 @@ describe TodoService, services: true do should_not_create_todo(user: non_member, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit) end + it 'creates a directly addressed todo for each valid mentioned user when leaving a note on commit' do + service.new_note(addressed_note_on_commit, john_doe) + + should_create_todo(user: member, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit) + should_create_todo(user: author, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit) + should_create_todo(user: john_doe, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit) + should_not_create_todo(user: non_member, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit) + end + it 'does not create todo when leaving a note on snippet' do should_not_create_any_todo { service.new_note(note_on_project_snippet, john_doe) } end @@ -324,6 +452,7 @@ describe TodoService, services: true do describe 'Merge Requests' do let(:mr_assigned) { create(:merge_request, source_project: project, author: author, assignee: john_doe, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") } + let(:addressed_mr_assigned) { create(:merge_request, source_project: project, author: author, assignee: john_doe, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") } let(:mr_unassigned) { create(:merge_request, source_project: project, author: author, assignee: nil) } describe '#new_merge_request' do @@ -350,6 +479,25 @@ describe TodoService, services: true do should_not_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED) end + + it 'creates a todo for each valid user based on the type of mention' do + mr_assigned.update(description: directly_addressed_and_mentioned) + + service.new_merge_request(mr_assigned, author) + + should_create_todo(user: member, target: mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: admin, target: mr_assigned, action: Todo::MENTIONED) + end + + it 'creates a directly addressed todo for each valid addressed user' do + service.new_merge_request(addressed_mr_assigned, author) + + should_create_todo(user: member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: author, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: john_doe, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: non_member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + end end describe '#update_merge_request' do @@ -363,12 +511,37 @@ describe TodoService, services: true do should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED) end + it 'creates a todo for each valid user based on the type of mention' do + mr_assigned.update(description: directly_addressed_and_mentioned) + + service.update_merge_request(mr_assigned, author) + + should_create_todo(user: member, target: mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: admin, target: mr_assigned, action: Todo::MENTIONED) + end + + it 'creates a directly addressed todo for each valid addressed user' do + service.update_merge_request(addressed_mr_assigned, author) + + should_create_todo(user: member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: john_doe, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: author, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: non_member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + end + it 'does not create a todo if user was already mentioned' do create(:todo, :mentioned, user: member, project: project, target: mr_assigned, author: author) expect { service.update_merge_request(mr_assigned, author) }.not_to change(member.todos, :count) end + it 'does not create a directly addressed todo if user was already mentioned or addressed' do + create(:todo, :directly_addressed, user: member, project: project, target: addressed_mr_assigned, author: author) + + expect{ service.update_merge_request(addressed_mr_assigned, author) }.not_to change(member.todos, :count) + end + context 'with a task list' do it 'does not create todo when tasks are marked as completed' do mr_assigned.update(description: "- [x] Task 1\n- [X] Task 2 #{mentions}") @@ -384,6 +557,20 @@ describe TodoService, services: true do should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) end + it 'does not create directly addressed todo when tasks are marked as completed' do + addressed_mr_assigned.update(description: "#{directly_addressed}\n- [x] Task 1\n- [X] Task 2") + + service.update_merge_request(addressed_mr_assigned, author) + + should_not_create_todo(user: admin, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: assignee, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: author, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: john_doe, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: non_member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + end + it 'does not raise an error when description not change' do mr_assigned.update(title: 'Sample') @@ -436,6 +623,11 @@ describe TodoService, services: true do service.reassigned_merge_request(mr_assigned, author) should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) end + + it 'does not create a directly addressed todo for guests' do + service.reassigned_merge_request(addressed_mr_assigned, author) + should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + end end describe '#merge_merge_request' do @@ -452,6 +644,11 @@ describe TodoService, services: true do service.merge_merge_request(mr_assigned, john_doe) should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) end + + it 'does not create directly addressed todo for guests' do + service.merge_merge_request(addressed_mr_assigned, john_doe) + should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + end end describe '#new_award_emoji' do @@ -509,6 +706,7 @@ describe TodoService, services: true do describe '#new_note' do let(:mention) { john_doe.to_reference } let(:diff_note_on_merge_request) { create(:diff_note_on_merge_request, project: project, noteable: mr_unassigned, author: author, note: "Hey #{mention}") } + let(:addressed_diff_note_on_merge_request) { create(:diff_note_on_merge_request, project: project, noteable: mr_unassigned, author: author, note: "#{mention}, hey!") } let(:legacy_diff_note_on_merge_request) { create(:legacy_diff_note_on_merge_request, project: project, noteable: mr_unassigned, author: author, note: "Hey #{mention}") } it 'creates a todo for mentioned user on new diff note' do @@ -517,6 +715,12 @@ describe TodoService, services: true do should_create_todo(user: john_doe, target: mr_unassigned, author: author, action: Todo::MENTIONED, note: diff_note_on_merge_request) end + it 'creates a directly addressed todo for addressed user on new diff note' do + service.new_note(addressed_diff_note_on_merge_request, author) + + should_create_todo(user: john_doe, target: mr_unassigned, author: author, action: Todo::DIRECTLY_ADDRESSED, note: addressed_diff_note_on_merge_request) + end + it 'creates a todo for mentioned user on legacy diff note' do service.new_note(legacy_diff_note_on_merge_request, author) diff --git a/spec/support/issuables_list_metadata_shared_examples.rb b/spec/support/issuables_list_metadata_shared_examples.rb new file mode 100644 index 00000000000..dac94dfc31e --- /dev/null +++ b/spec/support/issuables_list_metadata_shared_examples.rb @@ -0,0 +1,35 @@ +shared_examples 'issuables list meta-data' do |issuable_type, action = nil| + before do + @issuable_ids = [] + + 2.times do + if issuable_type == :issue + issuable = create(issuable_type, project: project) + else + issuable = create(issuable_type, title: FFaker::Lorem.sentence, source_project: project, source_branch: FFaker::Name.name) + end + + @issuable_ids << issuable.id + + issuable.id.times { create(:note, noteable: issuable, project: issuable.project) } + (issuable.id + 1).times { create(:award_emoji, :downvote, awardable: issuable) } + (issuable.id + 2).times { create(:award_emoji, :upvote, awardable: issuable) } + end + end + + it "creates indexed meta-data object for issuable notes and votes count" do + if action + get action + else + get :index, namespace_id: project.namespace.path, project_id: project.path + end + + meta_data = assigns(:issuable_meta_data) + + @issuable_ids.each do |id| + expect(meta_data[id].notes_count).to eq(id) + expect(meta_data[id].downvotes).to eq(id + 1) + expect(meta_data[id].upvotes).to eq(id + 2) + end + end +end diff --git a/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb b/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb index dd54b0addda..c64574679b6 100644 --- a/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb +++ b/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb @@ -58,7 +58,7 @@ shared_examples 'new issuable record that supports slash commands' do let(:example_params) do { assignee: create(:user), - milestone_id: double(:milestone), + milestone_id: 1, description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}") } end |
