diff options
136 files changed, 2521 insertions, 718 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index 784697aaaa8..84a8015b410 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -247,7 +247,7 @@ Style/FlipFlop: # Checks use of for or each in multiline loops. Style/For: - Enabled: false + Enabled: true # Enforce the use of Kernel#sprintf, Kernel#format or String#%. Style/FormatString: @@ -318,7 +318,7 @@ Style/LambdaCall: # Comments should start with a space. Style/LeadingCommentSpace: - Enabled: false + Enabled: true # Use \ instead of + or << to concatenate two string literals at line end. Style/LineEndConcatenation: diff --git a/CHANGELOG b/CHANGELOG index fcaf022b416..848aaa8506e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,10 +2,13 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.9.0 (unreleased) - Allow enabling wiki page events from Webhook management UI + - Make EmailsOnPushWorker use Sidekiq mailers queue - Fix wiki page events' webhook to point to the wiki repository + - Fix issue todo not remove when leave project !4150 (Long Nguyen) - Allow forking projects with restricted visibility level - Improve note validation to prevent errors when creating invalid note via API - Remove project notification settings associated with deleted projects + - Fix 404 page when viewing TODOs that contain milestones or labels in different projects - Redesign navigation for project pages - Fix groups API to list only user's accessible projects - Redesign account and email confirmation emails @@ -15,15 +18,20 @@ v 8.9.0 (unreleased) - Changed the Slack build message to use the singular duration if necessary (Aran Koning) - Fix issues filter when ordering by milestone - Todos will display target state if issuable target is 'Closed' or 'Merged' + - Fix bug when sorting issues by milestone due date and filtering by two or more labels - Remove 'main language' feature + - Pipelines can be canceled only when there are running builds - Projects pending deletion will render a 404 page - Measure queue duration between gitlab-workhorse and Rails - Make authentication service for Container Registry to be compatible with < Docker 1.11 - Add Application Setting to configure Container Registry token expire delay (default 5min) v 8.8.3 + - Fix incorrect links on pipeline page when merge request created from fork - Fix gitlab importer failing to import new projects due to missing credentials - Fix import URL migration not rescuing with the correct Error + - In search results, only show notes on confidential issues that the user has access to + - Fix health check access token changing due to old application settings being used v 8.8.2 - Added remove due date button. !4209 diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1429ee40bb7..9b2a9d298b3 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -263,7 +263,7 @@ class ApplicationController < ActionController::Base # internal repos where you are not a member. Enable this filter # or improve current implementation to filter only issues you # created or assigned or mentioned - #@filter_params[:authorized_only] = true + # @filter_params[:authorized_only] = true end @filter_params diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 17ce8e2ad20..d54284d7b20 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -205,7 +205,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def branch_from - #This is always source + # This is always source @source_project = @merge_request.nil? ? @project : @merge_request.source_project @commit = @repository.commit(params[:ref]) if params[:ref].present? render layout: false diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index 0d6c32fabd2..4b404eb03fa 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -91,8 +91,8 @@ class Projects::WikisController < Projects::ApplicationController def markdown_preview text = params[:text] - ext = Gitlab::ReferenceExtractor.new(@project, current_user, current_user) - ext.analyze(text) + ext = Gitlab::ReferenceExtractor.new(@project, current_user) + ext.analyze(text, author: current_user) render json: { body: view_context.markdown(text, pipeline: :wiki, project_wiki: @project_wiki), diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 9697b88c032..f94e2a84fa2 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -197,8 +197,8 @@ class ProjectsController < Projects::ApplicationController def markdown_preview text = params[:text] - ext = Gitlab::ReferenceExtractor.new(@project, current_user, current_user) - ext.analyze(text) + ext = Gitlab::ReferenceExtractor.new(@project, current_user) + ext.analyze(text, author: current_user) render json: { body: view_context.markdown(text), diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index c29f4609e93..d68c2a708e3 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,5 +1,6 @@ class SessionsController < Devise::SessionsController include AuthenticatesWithTwoFactor + include Devise::Controllers::Rememberable include Recaptcha::ClientHelper skip_before_action :check_2fa_requirement, only: [:destroy] @@ -96,6 +97,7 @@ class SessionsController < Devise::SessionsController # Remove any lingering user data from login session.delete(:otp_user_id) + remember_me(user) if user_params[:remember_me] == '1' sign_in(user) and return else flash.now[:alert] = 'Invalid two-factor code.' diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 8ed3ccf1c02..7d8c56f4c22 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -271,7 +271,7 @@ class IssuableFinder if filter_by_no_label? items = items.without_label else - items = items.with_label(label_names) + items = items.with_label(label_names, params[:sort]) if projects items = items.where(labels: { project_id: projects }) end diff --git a/app/models/ability.rb b/app/models/ability.rb index 8c5b255223d..44515550d9e 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -23,6 +23,28 @@ class Ability end.concat(global_abilities(user)) end + # Given a list of users and a project this method returns the users that can + # read the given project. + def users_that_can_read_project(users, project) + if project.public? + users + else + users.select do |user| + if user.admin? + true + elsif project.internal? && !user.external? + true + elsif project.owner == user + true + elsif project.team.members.include?(user) + true + else + false + end + end + end + end + # List of possible abilities for anonymous user def anonymous_abilities(user, subject) if subject.is_a?(PersonalSnippet) diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index fc48307b75d..42f908aa344 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -102,6 +102,10 @@ class ApplicationSetting < ActiveRecord::Base Rails.cache.delete(CACHE_KEY) end + def self.cached + Rails.cache.fetch(CACHE_KEY) + end + def self.create_from_defaults create( default_projects_limit: Settings.gitlab['default_projects_limit'], diff --git a/app/models/ci/commit.rb b/app/models/ci/commit.rb index 6675a3f5d53..f22b573a94c 100644 --- a/app/models/ci/commit.rb +++ b/app/models/ci/commit.rb @@ -66,6 +66,10 @@ module Ci end end + def cancelable? + builds.running_or_pending.any? + end + def cancel_running builds.running_or_pending.each(&:cancel) end diff --git a/app/models/commit.rb b/app/models/commit.rb index 562c3ed15b2..f96c7cb34d0 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -8,7 +8,10 @@ class Commit include StaticModel attr_mentionable :safe_message, pipeline: :single_line - participant :author, :committer, :notes + + participant :author + participant :committer + participant :notes_with_associations attr_accessor :project @@ -194,6 +197,10 @@ class Commit project.notes.for_commit_id(self.id) end + def notes_with_associations + notes.includes(:author, :project) + end + def method_missing(m, *args, &block) @raw.send(m, *args, &block) end @@ -219,7 +226,7 @@ class Commit def revert_branch_name "revert-#{short_id}" end - + def cherry_pick_branch_name project.repository.next_branch("cherry-pick-#{short_id}", mild: true) end @@ -251,11 +258,13 @@ class Commit end def has_been_reverted?(current_user = nil, noteable = self) - Gitlab::ReferenceExtractor.lazily do - noteable.notes.system.flat_map do |note| - note.all_references(current_user).commits - end - end.any? { |commit_ref| commit_ref.reverts_commit?(self) } + ext = all_references(current_user) + + noteable.notes_with_associations.system.each do |note| + note.all_references(current_user, extractor: ext) + end + + ext.commits.any? { |commit_ref| commit_ref.reverts_commit?(self) } end def change_type_title diff --git a/app/models/commit_range.rb b/app/models/commit_range.rb index 51673897d98..4066958f67c 100644 --- a/app/models/commit_range.rb +++ b/app/models/commit_range.rb @@ -62,7 +62,7 @@ class CommitRange def initialize(range_string, project) @project = project - range_string.strip! + range_string = range_string.strip unless range_string =~ /\A#{PATTERN}\z/ raise ArgumentError, "invalid CommitRange string format: #{range_string}" diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 91315b3459f..2326a395cb8 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -59,8 +59,12 @@ module Issuable prefix: true attr_mentionable :title, pipeline: :single_line - attr_mentionable :description, cache: true - participant :author, :assignee, :notes_with_associations + attr_mentionable :description + + participant :author + participant :assignee + participant :notes_with_associations + strip_attributes :title acts_as_paranoid @@ -126,13 +130,29 @@ module Issuable joins(join_clause).group(issuable_table[:id]).reorder("COUNT(notes.id) DESC") end - def with_label(title) + def with_label(title, sort = nil) if title.is_a?(Array) && title.size > 1 - joins(:labels).where(labels: { title: title }).group(arel_table[:id]).having("COUNT(DISTINCT labels.title) = #{title.size}") + joins(:labels).where(labels: { title: title }).group(*grouping_columns(sort)).having("COUNT(DISTINCT labels.title) = #{title.size}") else joins(:labels).where(labels: { title: title }) end end + + # Includes table keys in group by clause when sorting + # preventing errors in postgres + # + # Returns an array of arel columns + def grouping_columns(sort) + grouping_columns = [arel_table[:id]] + + if ["milestone_due_desc", "milestone_due_asc"].include?(sort) + milestone_table = Milestone.arel_table + grouping_columns << milestone_table[:id] + grouping_columns << milestone_table[:due_date] + end + + grouping_columns + end end def today? diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index b381d225485..f00b5b8497c 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -23,7 +23,7 @@ module Mentionable included do if self < Participable - participant ->(current_user) { mentioned_users(current_user) } + participant -> (user, ext) { all_references(user, extractor: ext) } end end @@ -43,23 +43,22 @@ module Mentionable self end - def all_references(current_user = nil, text = nil) - ext = Gitlab::ReferenceExtractor.new(self.project, current_user || self.author, self.author) + def all_references(current_user = nil, text = nil, extractor: nil) + extractor ||= Gitlab::ReferenceExtractor. + new(project, current_user || author) if text - ext.analyze(text) + extractor.analyze(text, author: author) else self.class.mentionable_attrs.each do |attr, options| - text = send(attr) + text = __send__(attr) + options = options.merge(cache_key: [self, attr], author: author) - context = options.dup - context[:cache_key] = [self, attr] if context.delete(:cache) && self.persisted? - - ext.analyze(text, context) + extractor.analyze(text, options) end end - ext + extractor end def mentioned_users(current_user = nil) diff --git a/app/models/concerns/participable.rb b/app/models/concerns/participable.rb index fc6f83b918b..9056722f45e 100644 --- a/app/models/concerns/participable.rb +++ b/app/models/concerns/participable.rb @@ -3,8 +3,6 @@ # Contains functionality related to objects that can have participants, such as # an author, an assignee and people mentioned in its description or comments. # -# Used by Issue, Note, MergeRequest, Snippet and Commit. -# # Usage: # # class Issue < ActiveRecord::Base @@ -12,22 +10,36 @@ # # # ... # -# participant :author, :assignee, :notes, ->(current_user) { mentioned_users(current_user) } +# participant :author +# participant :assignee +# participant :notes +# +# participant -> (current_user, ext) do +# ext.analyze('...') +# end # end # # issue = Issue.last # users = issue.participants -# # `users` will contain the issue's author, its assignee, -# # all users returned by its #mentioned_users method, -# # as well as all participants to all of the issue's notes, -# # since Note implements Participable as well. -# module Participable extend ActiveSupport::Concern module ClassMethods - def participant(*attrs) - participant_attrs.concat(attrs) + # Adds a list of participant attributes. Attributes can either be symbols or + # Procs. + # + # When using a Proc instead of a Symbol the Proc will be given two + # arguments: + # + # 1. The current user (as an instance of User) + # 2. An instance of `Gitlab::ReferenceExtractor` + # + # It is expected that a Proc populates the given reference extractor + # instance with data. The return value of the Proc is ignored. + # + # attr - The name of the attribute or a Proc + def participant(attr) + participant_attrs << attr end def participant_attrs @@ -35,42 +47,42 @@ module Participable end end - # Be aware that this method makes a lot of sql queries. - # Save result into variable if you are going to reuse it inside same request - def participants(current_user = self.author) - participants = - Gitlab::ReferenceExtractor.lazily do - self.class.participant_attrs.flat_map do |attr| - value = - if attr.respond_to?(:call) - instance_exec(current_user, &attr) - else - send(attr) - end + # Returns the users participating in a discussion. + # + # This method processes attributes of objects in breadth-first order. + # + # Returns an Array of User instances. + def participants(current_user = nil) + current_user ||= author + ext = Gitlab::ReferenceExtractor.new(project, current_user) + participants = Set.new + process = [self] - participants_for(value, current_user) - end.compact.uniq - end + until process.empty? + source = process.pop - unless Gitlab::ReferenceExtractor.lazy? - participants.select! do |user| - user.can?(:read_project, project) + case source + when User + participants << source + when Participable + source.class.participant_attrs.each do |attr| + if attr.respond_to?(:call) + source.instance_exec(current_user, ext, &attr) + else + process << source.__send__(attr) + end + end + when Enumerable, ActiveRecord::Relation + # This uses reverse_each so we can use "pop" to get the next value to + # process (in order). Using unshift instead of pop would require + # moving all Array values one index to the left (which can be + # expensive). + source.reverse_each { |obj| process << obj } end end - participants - end - - private + participants.merge(ext.users) - def participants_for(value, current_user = nil) - case value - when User, Banzai::LazyReference - [value] - when Enumerable, ActiveRecord::Relation - value.flat_map { |v| participants_for(v, current_user) } - when Participable - value.participants(current_user) - end + Ability.users_that_can_read_project(participants.to_a, project) end end diff --git a/app/models/issue.rb b/app/models/issue.rb index 2d4a9b9f19a..bd0fbc96d18 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -95,14 +95,13 @@ class Issue < ActiveRecord::Base end def referenced_merge_requests(current_user = nil) - @referenced_merge_requests ||= {} - @referenced_merge_requests[current_user] ||= begin - Gitlab::ReferenceExtractor.lazily do - [self, *notes].flat_map do |note| - note.all_references(current_user).merge_requests - end - end.sort_by(&:iid).uniq + ext = all_references(current_user) + + notes_with_associations.each do |object| + object.all_references(current_user, extractor: ext) end + + ext.merge_requests.sort_by(&:iid) end # All branches containing the current issue's ID, except for @@ -139,9 +138,13 @@ class Issue < ActiveRecord::Base def closed_by_merge_requests(current_user = nil) return [] unless open? - notes.system.flat_map do |note| - note.all_references(current_user).merge_requests - end.uniq.select { |mr| mr.open? && mr.closes_issue?(self) } + ext = all_references(current_user) + + notes.system.each do |note| + note.all_references(current_user, extractor: ext) + end + + ext.merge_requests.select { |mr| mr.open? && mr.closes_issue?(self) } end def moved? diff --git a/app/models/key.rb b/app/models/key.rb index d52afda67d1..0532e84f47d 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -26,7 +26,7 @@ class Key < ActiveRecord::Base end def publishable_key - #Removes anything beyond the keytype and key itself + # Removes anything beyond the keytype and key itself self.key.split[0..1].join(' ') end diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index 8dae3bb2ef2..46955b430f3 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -5,7 +5,6 @@ class ProjectMember < Member belongs_to :project, class_name: 'Project', foreign_key: 'source_id' - # Make sure project member points only to project as it source default_value_for :source_type, SOURCE_TYPE validates_format_of :source_type, with: /\AProject\z/ @@ -15,6 +14,8 @@ class ProjectMember < Member scope :in_projects, ->(projects) { where(source_id: projects.pluck(:id)) } scope :with_user, ->(user) { where(user_id: user.id) } + before_destroy :delete_member_todos + class << self # Add users to project teams with passed access option @@ -102,6 +103,10 @@ class ProjectMember < Member private + def delete_member_todos + user.todos.where(project_id: source_id).destroy_all if user + end + def send_invite notification_service.invite_project_member(self, @raw_invite_token) diff --git a/app/models/network/graph.rb b/app/models/network/graph.rb index 39ffb2d8f8b..a2aee2f925b 100644 --- a/app/models/network/graph.rb +++ b/app/models/network/graph.rb @@ -205,7 +205,7 @@ module Network # Visit branching chains leaves.each do |l| parents = l.parents(@map).select{|p| p.space.zero?} - for p in parents + parents.each do |p| place_chain(p, l.time) end end @@ -223,7 +223,7 @@ module Network end def mark_reserved(time_range, space) - for day in time_range + time_range.each do |day| @reserved[day].push(space) end end @@ -232,7 +232,7 @@ module Network space_default ||= space_base reserved = [] - for day in time_range + time_range.each do |day| reserved.push(*@reserved[day]) end reserved.uniq! diff --git a/app/models/note.rb b/app/models/note.rb index 5f669c02e8b..c21981ead84 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -6,7 +6,7 @@ class Note < ActiveRecord::Base default_value_for :system, false - attr_mentionable :note, cache: true, pipeline: :note + attr_mentionable :note, pipeline: :note participant :author belongs_to :project @@ -84,14 +84,30 @@ class Note < ActiveRecord::Base # # This method uses ILIKE on PostgreSQL and LIKE on MySQL. # - # query - The search query as a String. + # query - The search query as a String. + # as_user - Limit results to those viewable by a specific user # # Returns an ActiveRecord::Relation. - def search(query) + def search(query, as_user: nil) table = arel_table pattern = "%#{query}%" - where(table[:note].matches(pattern)) + found_notes = joins('LEFT JOIN issues ON issues.id = noteable_id'). + where(table[:note].matches(pattern)) + + if as_user + found_notes.where(' + issues.confidential IS NULL + OR issues.confidential IS FALSE + OR (issues.confidential IS TRUE + AND (issues.author_id = :user_id + OR issues.assignee_id = :user_id + OR issues.project_id IN(:project_ids)))', + user_id: as_user.id, + project_ids: as_user.authorized_projects.select(:id)) + else + found_notes.where('issues.confidential IS NULL OR issues.confidential IS FALSE') + end end def grouped_awards diff --git a/app/models/project_snippet.rb b/app/models/project_snippet.rb index 5fba6baa204..25b5d777641 100644 --- a/app/models/project_snippet.rb +++ b/app/models/project_snippet.rb @@ -7,5 +7,6 @@ class ProjectSnippet < Snippet # Scopes scope :fresh, -> { order("created_at DESC") } - participant :author, :notes + participant :author + participant :notes_with_associations end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 0a3c3b57669..407697b745c 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -30,7 +30,8 @@ class Snippet < ActiveRecord::Base scope :public_and_internal, -> { where(visibility_level: [Snippet::PUBLIC, Snippet::INTERNAL]) } scope :fresh, -> { order("created_at DESC") } - participant :author, :notes + participant :author + participant :notes_with_associations def self.reference_prefix '$' @@ -100,6 +101,10 @@ class Snippet < ActiveRecord::Base content.lines.count > 1000 end + def notes_with_associations + notes.includes(:author, :project) + end + class << self # Searches for snippets with a matching title or file name. # diff --git a/app/views/admin/abuse_reports/_abuse_report.html.haml b/app/views/admin/abuse_reports/_abuse_report.html.haml index 2ab01704b77..862b86d9d4a 100644 --- a/app/views/admin/abuse_reports/_abuse_report.html.haml +++ b/app/views/admin/abuse_reports/_abuse_report.html.haml @@ -16,7 +16,7 @@ .light.small = time_ago_with_tooltip(abuse_report.created_at) %td - = markdown(abuse_report.message.squish!, pipeline: :single_line) + = markdown(abuse_report.message.squish!, pipeline: :single_line, author: reporter) %td - if user = link_to 'Remove user & report', admin_abuse_report_path(abuse_report, remove_user: true), diff --git a/app/views/devise/sessions/two_factor.html.haml b/app/views/devise/sessions/two_factor.html.haml index c9d1e454a5e..8c6a1552a53 100644 --- a/app/views/devise/sessions/two_factor.html.haml +++ b/app/views/devise/sessions/two_factor.html.haml @@ -4,6 +4,7 @@ %h3 Two-factor Authentication .login-body = form_for(resource, as: resource_name, url: session_path(resource_name), method: :post) do |f| + = f.hidden_field :remember_me, value: params[resource_name][:remember_me] = f.text_field :otp_attempt, class: 'form-control', placeholder: 'Two-factor Authentication code', required: true, autofocus: true %p.help-block.hint Enter the code from the two-factor app on your mobile device. If you've lost your device, you may enter one of your recovery codes. .prepend-top-20 diff --git a/app/views/events/_commit.html.haml b/app/views/events/_commit.html.haml index dce4081288c..1bc9f604438 100644 --- a/app/views/events/_commit.html.haml +++ b/app/views/events/_commit.html.haml @@ -2,4 +2,4 @@ .commit-row-title = link_to truncate_sha(commit[:id]), namespace_project_commit_path(project.namespace, project, commit[:id]), class: "commit_short_id", alt: '', title: truncate_sha(commit[:id]) · - = markdown event_commit_title(commit[:message]), project: project, pipeline: :single_line + = markdown event_commit_title(commit[:message]), project: project, pipeline: :single_line, author: event.author diff --git a/app/views/events/_event_issue.atom.haml b/app/views/events/_event_issue.atom.haml index fad65310021..083c3936212 100644 --- a/app/views/events/_event_issue.atom.haml +++ b/app/views/events/_event_issue.atom.haml @@ -1,2 +1,2 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - = markdown(issue.description, pipeline: :atom, project: issue.project) + = markdown(issue.description, pipeline: :atom, project: issue.project, author: issue.author) diff --git a/app/views/events/_event_merge_request.atom.haml b/app/views/events/_event_merge_request.atom.haml index 19bdc7b9ca5..d7e05600627 100644 --- a/app/views/events/_event_merge_request.atom.haml +++ b/app/views/events/_event_merge_request.atom.haml @@ -1,2 +1,2 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - = markdown(merge_request.description, pipeline: :atom, project: merge_request.project) + = markdown(merge_request.description, pipeline: :atom, project: merge_request.project, author: merge_request.author) diff --git a/app/views/events/_event_note.atom.haml b/app/views/events/_event_note.atom.haml index b730ebbd5f9..1154f982821 100644 --- a/app/views/events/_event_note.atom.haml +++ b/app/views/events/_event_note.atom.haml @@ -1,2 +1,2 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - = markdown(note.note, pipeline: :atom, project: note.project) + = markdown(note.note, pipeline: :atom, project: note.project, author: note.author) diff --git a/app/views/events/_event_push.atom.haml b/app/views/events/_event_push.atom.haml index b271b9daff1..28bee1d0a33 100644 --- a/app/views/events/_event_push.atom.haml +++ b/app/views/events/_event_push.atom.haml @@ -6,7 +6,7 @@ %i at = commit[:timestamp].to_time.to_s(:short) - %blockquote= markdown(escape_once(commit[:message]), pipeline: :atom, project: event.project) + %blockquote= markdown(escape_once(commit[:message]), pipeline: :atom, project: event.project, author: event.author) - if event.commits_count > 15 %p %i diff --git a/app/views/events/event/_common.html.haml b/app/views/events/event/_common.html.haml index f9f623cc031..c7f29f2fc0e 100644 --- a/app/views/events/event/_common.html.haml +++ b/app/views/events/event/_common.html.haml @@ -4,7 +4,7 @@ = event_action_name(event) - if event.target - %strong= link_to event.target.reference_link_text, [event.project.namespace.becomes(Namespace), event.project, event.target], title: event.target_title + %strong= link_to event.target.reference_link_text, [event.project.namespace.becomes(Namespace), event.project, event.target], class: 'has-tooltip', title: event.target_title = event_preposition(event) diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml index 235bd46107e..dc4ff17e31a 100644 --- a/app/views/events/event/_push.html.haml +++ b/app/views/events/event/_push.html.haml @@ -15,7 +15,7 @@ %ul.well-list.event_commits - few_commits = event.commits[0...2] - few_commits.each do |commit| - = render "events/commit", commit: commit, project: project + = render "events/commit", commit: commit, project: project, event: event - create_mr = event.new_ref? && create_mr_button?(event.project.default_branch, event.ref_name, event.project) - if event.commits_count > 1 diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index 087b7472701..2c9b9006668 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -52,15 +52,6 @@ = icon('ship fw') %span Pipelines - %span.badge.count.ci_counter= number_with_delimiter(@project.ci_commits.running_or_pending.count) - - - if project_nav_tab? :builds - = nav_link(controller: %w(builds)) do - = link_to project_builds_path(@project), title: 'Builds', class: 'shortcuts-builds' do - = icon('cubes fw') - %span - Builds - %span.badge.count.builds_counter= number_with_delimiter(@project.builds.running_or_pending.count(:all)) - if project_nav_tab? :container_registry = nav_link(controller: %w(container_registry)) do @@ -132,4 +123,10 @@ = link_to new_namespace_project_issue_path(@project.namespace, @project), class: 'shortcuts-new-issue' do Create a new issue + -# Shortcut to builds page + - if project_nav_tab? :builds + %li.hidden + = link_to project_builds_path(@project), title: 'Builds', class: 'shortcuts-builds' do + Builds + .fade-right diff --git a/app/views/notify/_note_message.html.haml b/app/views/notify/_note_message.html.haml index 12ded41fbf2..e9c66170877 100644 --- a/app/views/notify/_note_message.html.haml +++ b/app/views/notify/_note_message.html.haml @@ -2,4 +2,4 @@ %div #{link_to @note.author_name, user_url(@note.author)} wrote: %div - = markdown(@note.note, pipeline: :email) + = markdown(@note.note, pipeline: :email, author: @note.author) diff --git a/app/views/notify/new_issue_email.html.haml b/app/views/notify/new_issue_email.html.haml index ad3ab2525bb..f42b150c0d6 100644 --- a/app/views/notify/new_issue_email.html.haml +++ b/app/views/notify/new_issue_email.html.haml @@ -2,7 +2,7 @@ %div #{link_to @issue.author_name, user_url(@issue.author)} wrote: -if @issue.description - = markdown(@issue.description, pipeline: :email) + = markdown(@issue.description, pipeline: :email, author: @issue.author) - if @issue.assignee_id.present? %p diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index 23423e7d981..158404de396 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -9,4 +9,4 @@ Assignee: #{@merge_request.author_name} → #{@merge_request.assignee_name} -if @merge_request.description - = markdown(@merge_request.description, pipeline: :email) + = markdown(@merge_request.description, pipeline: :email, author: @merge_request.author) diff --git a/app/views/projects/builds/index.html.haml b/app/views/projects/builds/index.html.haml index 8fb9ebc1b8b..818d5d28f04 100644 --- a/app/views/projects/builds/index.html.haml +++ b/app/views/projects/builds/index.html.haml @@ -1,4 +1,5 @@ - page_title "Builds" += render "projects/pipelines/head" .top-area %ul.nav-links diff --git a/app/views/projects/ci/commits/_commit.html.haml b/app/views/projects/ci/commits/_commit.html.haml index 5b6b940a0c4..5e3a4123a8e 100644 --- a/app/views/projects/ci/commits/_commit.html.haml +++ b/app/views/projects/ci/commits/_commit.html.haml @@ -63,9 +63,9 @@ %span #{build.name} - if can?(current_user, :update_pipeline, @project) - - if commit.retryable? && commit.builds.failed.any? + - if commit.retryable? = link_to retry_namespace_project_pipeline_path(@project.namespace, @project, commit.id), class: 'btn has-tooltip', title: "Retry", method: :post do = icon("repeat") - - if commit.active? + - if commit.cancelable? = link_to cancel_namespace_project_pipeline_path(@project.namespace, @project, commit.id), class: 'btn btn-remove has-tooltip', title: "Cancel", method: :post do = icon("remove") diff --git a/app/views/projects/commit/_ci_commit.html.haml b/app/views/projects/commit/_ci_commit.html.haml index ce5c550b441..32ff4d30977 100644 --- a/app/views/projects/commit/_ci_commit.html.haml +++ b/app/views/projects/commit/_ci_commit.html.haml @@ -1,24 +1,24 @@ .row-content-block.build-content.middle-block .pull-right - - if can?(current_user, :update_pipeline, @project) + - if can?(current_user, :update_pipeline, ci_commit.project) - if ci_commit.builds.latest.failed.any?(&:retryable?) - = link_to "Retry failed", retry_namespace_project_pipeline_path(@project.namespace, @project, ci_commit.id), class: 'btn btn-grouped btn-primary', method: :post + = link_to "Retry failed", retry_namespace_project_pipeline_path(ci_commit.project.namespace, ci_commit.project, ci_commit.id), class: 'btn btn-grouped btn-primary', method: :post - if ci_commit.builds.running_or_pending.any? - = link_to "Cancel running", cancel_namespace_project_pipeline_path(@project.namespace, @project, ci_commit.id), data: { confirm: 'Are you sure?' }, class: 'btn btn-grouped btn-danger', method: :post + = link_to "Cancel running", cancel_namespace_project_pipeline_path(ci_commit.project.namespace, ci_commit.project, ci_commit.id), data: { confirm: 'Are you sure?' }, class: 'btn btn-grouped btn-danger', method: :post .oneline.clearfix - if defined?(pipeline_details) && pipeline_details Pipeline - = link_to "##{ci_commit.id}", namespace_project_pipeline_path(@project.namespace, @project, ci_commit.id), class: "monospace" + = link_to "##{ci_commit.id}", namespace_project_pipeline_path(ci_commit.project.namespace, ci_commit.project, ci_commit.id), class: "monospace" with = pluralize ci_commit.statuses.count(:id), "build" - if ci_commit.ref for - = link_to ci_commit.ref, namespace_project_commits_path(@project.namespace, @project, ci_commit.ref), class: "monospace" + = link_to ci_commit.ref, namespace_project_commits_path(ci_commit.project.namespace, ci_commit.project, ci_commit.ref), class: "monospace" - if defined?(link_to_commit) && link_to_commit for commit - = link_to ci_commit.short_sha, namespace_project_commit_path(@project.namespace, @project, ci_commit.sha), class: "monospace" + = link_to ci_commit.short_sha, namespace_project_commit_path(ci_commit.project.namespace, ci_commit.project, ci_commit.sha), class: "monospace" - if ci_commit.duration in = time_interval_in_words ci_commit.duration @@ -31,7 +31,7 @@ %li= error You can also test your .gitlab-ci.yml in the #{link_to "Lint", ci_lint_path} -- if @project.builds_enabled? && !ci_commit.ci_yaml_file +- if ci_commit.project.builds_enabled? && !ci_commit.ci_yaml_file .bs-callout.bs-callout-warning \.gitlab-ci.yml not found in this commit @@ -45,7 +45,7 @@ %th Tags %th Duration %th Finished at - - if @project.build_coverage_enabled? + - if ci_commit.project.build_coverage_enabled? %th Coverage %th - ci_commit.statuses.stages.each do |stage| diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 429bf041248..6674d58417b 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -63,10 +63,10 @@ .commit-box.content-block %h3.commit-title - = markdown escape_once(@commit.title), pipeline: :single_line + = markdown escape_once(@commit.title), pipeline: :single_line, author: @commit.author - if @commit.description.present? %pre.commit-description - = preserve(markdown(escape_once(@commit.description), pipeline: :single_line)) + = preserve(markdown(escape_once(@commit.description), pipeline: :single_line, author: @commit.author)) :javascript $(".commit-info.branches").load("#{branches_namespace_project_commit_path(@project.namespace, @project, @commit.id)}"); diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 655cb0ac3cb..367027182b6 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -24,7 +24,7 @@ - if commit.description? .commit-row-description.js-toggle-content %pre - = preserve(markdown(escape_once(commit.description), pipeline: :single_line)) + = preserve(markdown(escape_once(commit.description), pipeline: :single_line, author: commit.author)) .commit-row-info by diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index e95f42064ac..f3b0469b7d4 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -52,12 +52,12 @@ .issue-details.issuable-details .detail-page-description.content-block %h2.title - = markdown escape_once(@issue.title), pipeline: :single_line + = markdown escape_once(@issue.title), pipeline: :single_line, author: @issue.author - if @issue.description.present? .description{ class: can?(current_user, :update_issue, @issue) ? 'js-task-list-container' : '' } .wiki = preserve do - = markdown(@issue.description, cache_key: [@issue, "description"]) + = markdown(@issue.description, cache_key: [@issue, "description"], author: @issue.author) %textarea.hidden.js-task-list-field = @issue.description = edited_time_ago_with_tooltip(@issue, placement: 'bottom', html_class: 'issue_edited_ago') diff --git a/app/views/projects/merge_requests/show/_mr_box.html.haml b/app/views/projects/merge_requests/show/_mr_box.html.haml index a23bd8d18d0..ebf18f6ac85 100644 --- a/app/views/projects/merge_requests/show/_mr_box.html.haml +++ b/app/views/projects/merge_requests/show/_mr_box.html.haml @@ -1,13 +1,13 @@ .detail-page-description.content-block %h2.title - = markdown escape_once(@merge_request.title), pipeline: :single_line + = markdown escape_once(@merge_request.title), pipeline: :single_line, author: @merge_request.author %div - if @merge_request.description.present? .description{class: can?(current_user, :update_merge_request, @merge_request) ? 'js-task-list-container' : ''} .wiki = preserve do - = markdown(@merge_request.description, cache_key: [@merge_request, "description"]) + = markdown(@merge_request.description, cache_key: [@merge_request, "description"], author: @merge_request.author) %textarea.hidden.js-task-list-field = @merge_request.description diff --git a/app/views/projects/merge_requests/widget/_open.html.haml b/app/views/projects/merge_requests/widget/_open.html.haml index 55dbae598d3..13359abede7 100644 --- a/app/views/projects/merge_requests/widget/_open.html.haml +++ b/app/views/projects/merge_requests/widget/_open.html.haml @@ -26,4 +26,4 @@ %i.fa.fa-check Accepting this merge request will close #{"issue".pluralize(@closes_issues.size)} = succeed '.' do - != markdown issues_sentence(@closes_issues), pipeline: :gfm + != markdown issues_sentence(@closes_issues), pipeline: :gfm, author: @merge_request.author diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 9fbc9a45549..f1045bbd8c3 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -29,7 +29,7 @@ .note-body{class: note_editable ? 'js-task-list-container' : ''} .note-text = preserve do - = markdown(note.note, pipeline: :note, cache_key: [note, "note"]) + = markdown(note.note, pipeline: :note, cache_key: [note, "note"], author: note.author) - if note_editable = render 'projects/notes/edit_form', note: note = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true) diff --git a/app/views/projects/pipelines/_head.html.haml b/app/views/projects/pipelines/_head.html.haml new file mode 100644 index 00000000000..2c8ae625e67 --- /dev/null +++ b/app/views/projects/pipelines/_head.html.haml @@ -0,0 +1,14 @@ +%ul.nav-links + - if project_nav_tab? :pipelines + = nav_link(controller: :pipelines) do + = link_to project_pipelines_path(@project), title: 'Pipelines', class: 'shortcuts-pipelines' do + %span + Pipelines + %span.badge.count.ci_counter= number_with_delimiter(@project.ci_commits.running_or_pending.count) + + - if project_nav_tab? :builds + = nav_link(controller: %w(builds)) do + = link_to project_builds_path(@project), title: 'Builds', class: 'shortcuts-builds' do + %span + Builds + %span.badge.count.builds_counter= number_with_delimiter(@project.builds.running_or_pending.count(:all)) diff --git a/app/views/projects/pipelines/index.html.haml b/app/views/projects/pipelines/index.html.haml index 8788db09dbe..453767920b5 100644 --- a/app/views/projects/pipelines/index.html.haml +++ b/app/views/projects/pipelines/index.html.haml @@ -1,4 +1,5 @@ - page_title "Pipelines" += render "projects/pipelines/head" .top-area %ul.nav-links diff --git a/app/views/projects/repositories/_feed.html.haml b/app/views/projects/repositories/_feed.html.haml index 6ca919f7f80..43a6fdfd103 100644 --- a/app/views/projects/repositories/_feed.html.haml +++ b/app/views/projects/repositories/_feed.html.haml @@ -12,7 +12,7 @@ = link_to namespace_project_commits_path(@project.namespace, @project, commit.id) do %code= commit.short_id = image_tag avatar_icon(commit.author_email), class: "", width: 16, alt: '' - = markdown escape_once(truncate(commit.title, length: 40)), pipeline: :single_line + = markdown escape_once(truncate(commit.title, length: 40)), pipeline: :single_line, author: commit.author %td %span.pull-right.cgray = time_ago_with_tooltip(commit.committed_date) diff --git a/app/views/search/results/_issue.html.haml b/app/views/search/results/_issue.html.haml index 640890fbe92..8f68d6d1b87 100644 --- a/app/views/search/results/_issue.html.haml +++ b/app/views/search/results/_issue.html.haml @@ -7,7 +7,7 @@ - if issue.description.present? .description.term = preserve do - = search_md_sanitize(markdown(truncate(issue.description, length: 200, separator: " "), { project: issue.project })) + = search_md_sanitize(markdown(truncate(issue.description, length: 200, separator: " "), { project: issue.project, author: issue.author })) %span.light #{issue.project.name_with_namespace} - if issue.closed? diff --git a/app/views/search/results/_merge_request.html.haml b/app/views/search/results/_merge_request.html.haml index 333f6533213..6331c2bd6b0 100644 --- a/app/views/search/results/_merge_request.html.haml +++ b/app/views/search/results/_merge_request.html.haml @@ -6,7 +6,7 @@ - if merge_request.description.present? .description.term = preserve do - = search_md_sanitize(markdown(merge_request.description, { project: merge_request.project })) + = search_md_sanitize(markdown(merge_request.description, { project: merge_request.project, author: merge_request.author })) %span.light #{merge_request.project.name_with_namespace} .pull-right diff --git a/app/views/search/results/_note.html.haml b/app/views/search/results/_note.html.haml index d9400b1d9fa..8163aff43b6 100644 --- a/app/views/search/results/_note.html.haml +++ b/app/views/search/results/_note.html.haml @@ -19,4 +19,4 @@ .note-search-result .term = preserve do - = search_md_sanitize(markdown(note.note, {no_header_anchors: true})) + = search_md_sanitize(markdown(note.note, {no_header_anchors: true, author: note.author})) diff --git a/app/views/shared/snippets/_header.html.haml b/app/views/shared/snippets/_header.html.haml index e0d0d967da6..af753496260 100644 --- a/app/views/shared/snippets/_header.html.haml +++ b/app/views/shared/snippets/_header.html.haml @@ -4,7 +4,7 @@ = visibility_level_label(@snippet.visibility_level) = visibility_level_icon(@snippet.visibility_level, fw: false) %strong.item-title - Snippet $#{@snippet.id} + Snippet #{@snippet.to_reference} %span.creator created by #{link_to_member(@project, @snippet.author, size: 24, author_class: "author item-title")} = time_ago_with_tooltip(@snippet.created_at, placement: 'bottom', html_class: 'snippet_updated_ago') @@ -21,4 +21,4 @@ .content-block.second-block %h2.snippet-title.prepend-top-0.append-bottom-0 - = markdown escape_once(@snippet.title), pipeline: :single_line + = markdown escape_once(@snippet.title), pipeline: :single_line, author: @snippet.author diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb index fa959fc56e3..971f969e25e 100644 --- a/app/workers/emails_on_push_worker.rb +++ b/app/workers/emails_on_push_worker.rb @@ -1,6 +1,7 @@ class EmailsOnPushWorker include Sidekiq::Worker + sidekiq_options queue: :mailers attr_reader :email, :skip_premailer def perform(project_id, recipients, push_data, options = {}) diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 393ad909b45..7bd13105045 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -52,7 +52,7 @@ Doorkeeper.configure do # For more information go to # https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes default_scopes :api - #optional_scopes :write, :update + # optional_scopes :write, :update # Change the way client credentials are retrieved from the request object. # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 4c164119fff..26c30e523a7 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -13,7 +13,7 @@ end OmniAuth.config.full_host = Settings.gitlab['base_url'] OmniAuth.config.allowed_request_methods = [:post] -#In case of auto sign-in, the GET method is used (users don't get to click on a button) +# In case of auto sign-in, the GET method is used (users don't get to click on a button) OmniAuth.config.allowed_request_methods << :get if Gitlab.config.omniauth.auto_sign_in_with_provider.present? OmniAuth.config.before_request_phase do |env| OmniAuth::RequestForgeryProtection.call(env) diff --git a/doc/api/services.md b/doc/api/services.md index b5000eff653..ccfc0fccb7f 100644 --- a/doc/api/services.md +++ b/doc/api/services.md @@ -503,6 +503,8 @@ Parameters: - `project_url` (**required**) - Project url - `issues_url` (**required**) - Issue url - `description` (optional) - Jira issue tracker +- `username` (optional) - Jira username +- `password` (optional) - Jira password ### Delete JIRA service diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index b8962379cb5..db95d7c908b 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -18,10 +18,6 @@ module Banzai @object_sym ||= object_name.to_sym end - def self.data_reference - @data_reference ||= "data-#{object_name.dasherize}" - end - def self.object_class_title @object_title ||= object_class.name.titleize end @@ -45,10 +41,6 @@ module Banzai end end - def self.referenced_by(node) - { object_sym => LazyReference.new(object_class, node.attr(data_reference)) } - end - def object_class self.class.object_class end @@ -236,7 +228,9 @@ module Banzai if cache.key?(key) cache[key] else - cache[key] = yield + value = yield + cache[key] = value if key.present? + value end end end diff --git a/lib/banzai/filter/commit_range_reference_filter.rb b/lib/banzai/filter/commit_range_reference_filter.rb index b469ea0f626..bbb88c979cc 100644 --- a/lib/banzai/filter/commit_range_reference_filter.rb +++ b/lib/banzai/filter/commit_range_reference_filter.rb @@ -4,6 +4,8 @@ module Banzai # # This filter supports cross-project references. class CommitRangeReferenceFilter < AbstractReferenceFilter + self.reference_type = :commit_range + def self.object_class CommitRange end @@ -14,34 +16,18 @@ module Banzai end end - def self.referenced_by(node) - project = Project.find(node.attr("data-project")) rescue nil - return unless project - - id = node.attr("data-commit-range") - range = find_object(project, id) - - return unless range - - { commit_range: range } - end - def initialize(*args) super @commit_map = {} end - def self.find_object(project, id) + def find_object(project, id) range = CommitRange.new(id, project) range.valid_commits? ? range : nil end - def find_object(*args) - self.class.find_object(*args) - end - def url_for_object(range, project) h = Gitlab::Routing.url_helpers h.namespace_project_compare_url(project.namespace, project, diff --git a/lib/banzai/filter/commit_reference_filter.rb b/lib/banzai/filter/commit_reference_filter.rb index bd88207326c..2ce1816672b 100644 --- a/lib/banzai/filter/commit_reference_filter.rb +++ b/lib/banzai/filter/commit_reference_filter.rb @@ -4,6 +4,8 @@ module Banzai # # This filter supports cross-project references. class CommitReferenceFilter < AbstractReferenceFilter + self.reference_type = :commit + def self.object_class Commit end @@ -14,28 +16,12 @@ module Banzai end end - def self.referenced_by(node) - project = Project.find(node.attr("data-project")) rescue nil - return unless project - - id = node.attr("data-commit") - commit = find_object(project, id) - - return unless commit - - { commit: commit } - end - - def self.find_object(project, id) + def find_object(project, id) if project && project.valid_repo? project.commit(id) end end - def find_object(*args) - self.class.find_object(*args) - end - def url_for_object(commit, project) h = Gitlab::Routing.url_helpers h.namespace_project_commit_url(project.namespace, project, commit, diff --git a/lib/banzai/filter/external_issue_reference_filter.rb b/lib/banzai/filter/external_issue_reference_filter.rb index 37344b90576..eaa702952cc 100644 --- a/lib/banzai/filter/external_issue_reference_filter.rb +++ b/lib/banzai/filter/external_issue_reference_filter.rb @@ -4,6 +4,8 @@ module Banzai # References are ignored if the project doesn't use an external issue # tracker. class ExternalIssueReferenceFilter < ReferenceFilter + self.reference_type = :external_issue + # Public: Find `JIRA-123` issue references in text # # ExternalIssueReferenceFilter.references_in(text) do |match, issue| @@ -21,18 +23,6 @@ module Banzai end end - def self.referenced_by(node) - project = Project.find(node.attr("data-project")) rescue nil - return unless project - - id = node.attr("data-external-issue") - external_issue = ExternalIssue.new(id, project) - - return unless external_issue - - { external_issue: external_issue } - end - def call # Early return if the project isn't using an external tracker return doc if project.nil? || default_issues_tracker? diff --git a/lib/banzai/filter/issue_reference_filter.rb b/lib/banzai/filter/issue_reference_filter.rb index 59c5e89c546..2496e704002 100644 --- a/lib/banzai/filter/issue_reference_filter.rb +++ b/lib/banzai/filter/issue_reference_filter.rb @@ -5,18 +5,12 @@ module Banzai # # This filter supports cross-project references. class IssueReferenceFilter < AbstractReferenceFilter + self.reference_type = :issue + def self.object_class Issue end - def self.user_can_see_reference?(user, node, context) - # It is not possible to check access rights for external issue trackers - return true if context[:project].try(:external_issue_tracker) - - issue = Issue.find(node.attr('data-issue')) rescue nil - Ability.abilities.allowed?(user, :read_issue, issue) - end - def find_object(project, id) project.get_issue(id) end diff --git a/lib/banzai/filter/label_reference_filter.rb b/lib/banzai/filter/label_reference_filter.rb index 8488a493b55..e4d3f87d0aa 100644 --- a/lib/banzai/filter/label_reference_filter.rb +++ b/lib/banzai/filter/label_reference_filter.rb @@ -2,6 +2,8 @@ module Banzai module Filter # HTML filter that replaces label references with links. class LabelReferenceFilter < AbstractReferenceFilter + self.reference_type = :label + def self.object_class Label end diff --git a/lib/banzai/filter/merge_request_reference_filter.rb b/lib/banzai/filter/merge_request_reference_filter.rb index cad38a51851..ac5216d9cfb 100644 --- a/lib/banzai/filter/merge_request_reference_filter.rb +++ b/lib/banzai/filter/merge_request_reference_filter.rb @@ -5,6 +5,8 @@ module Banzai # # This filter supports cross-project references. class MergeRequestReferenceFilter < AbstractReferenceFilter + self.reference_type = :merge_request + def self.object_class MergeRequest end diff --git a/lib/banzai/filter/milestone_reference_filter.rb b/lib/banzai/filter/milestone_reference_filter.rb index dad0768f51b..ca686c87d97 100644 --- a/lib/banzai/filter/milestone_reference_filter.rb +++ b/lib/banzai/filter/milestone_reference_filter.rb @@ -2,6 +2,8 @@ module Banzai module Filter # HTML filter that replaces milestone references with links. class MilestoneReferenceFilter < AbstractReferenceFilter + self.reference_type = :milestone + def self.object_class Milestone end diff --git a/lib/banzai/filter/redactor_filter.rb b/lib/banzai/filter/redactor_filter.rb index e589b5df6ec..c753a84a20d 100644 --- a/lib/banzai/filter/redactor_filter.rb +++ b/lib/banzai/filter/redactor_filter.rb @@ -7,8 +7,11 @@ module Banzai # class RedactorFilter < HTML::Pipeline::Filter def call - Querying.css(doc, 'a.gfm').each do |node| - unless user_can_see_reference?(node) + nodes = Querying.css(doc, 'a.gfm[data-reference-type]') + visible = nodes_visible_to_user(nodes) + + nodes.each do |node| + unless visible.include?(node) # The reference should be replaced by the original text, # which is not always the same as the rendered text. text = node.attr('data-original') || node.text @@ -21,20 +24,30 @@ module Banzai private - def user_can_see_reference?(node) - if node.has_attribute?('data-reference-filter') - reference_type = node.attr('data-reference-filter') - reference_filter = Banzai::Filter.const_get(reference_type) + def nodes_visible_to_user(nodes) + per_type = Hash.new { |h, k| h[k] = [] } + visible = Set.new + + nodes.each do |node| + per_type[node.attr('data-reference-type')] << node + end + + per_type.each do |type, nodes| + parser = Banzai::ReferenceParser[type].new(project, current_user) - reference_filter.user_can_see_reference?(current_user, node, context) - else - true + visible.merge(parser.nodes_visible_to_user(current_user, nodes)) end + + visible end def current_user context[:current_user] end + + def project + context[:project] + end end end end diff --git a/lib/banzai/filter/reference_filter.rb b/lib/banzai/filter/reference_filter.rb index 31386cf851c..41ae0e1f9cc 100644 --- a/lib/banzai/filter/reference_filter.rb +++ b/lib/banzai/filter/reference_filter.rb @@ -8,24 +8,8 @@ module Banzai # :project (required) - Current project, ignored if reference is cross-project. # :only_path - Generate path-only links. class ReferenceFilter < HTML::Pipeline::Filter - def self.user_can_see_reference?(user, node, context) - if node.has_attribute?('data-project') - project_id = node.attr('data-project').to_i - return true if project_id == context[:project].try(:id) - - project = Project.find(project_id) rescue nil - Ability.abilities.allowed?(user, :read_project, project) - else - true - end - end - - def self.user_can_reference?(user, node, context) - true - end - - def self.referenced_by(node) - raise NotImplementedError, "#{self} does not implement #{__method__}" + class << self + attr_accessor :reference_type end # Returns a data attribute String to attach to a reference link @@ -43,7 +27,9 @@ module Banzai # # Returns a String def data_attribute(attributes = {}) - attributes[:reference_filter] = self.class.name.demodulize + attributes = attributes.reject { |_, v| v.nil? } + + attributes[:reference_type] = self.class.reference_type attributes.delete(:original) if context[:no_original_data] attributes.map { |key, value| %Q(data-#{key.to_s.dasherize}="#{escape_once(value)}") }.join(" ") end diff --git a/lib/banzai/filter/reference_gatherer_filter.rb b/lib/banzai/filter/reference_gatherer_filter.rb deleted file mode 100644 index 96fdb06304e..00000000000 --- a/lib/banzai/filter/reference_gatherer_filter.rb +++ /dev/null @@ -1,65 +0,0 @@ -module Banzai - module Filter - # HTML filter that gathers all referenced records that the current user has - # permission to view. - # - # Expected to be run in its own post-processing pipeline. - # - class ReferenceGathererFilter < HTML::Pipeline::Filter - def initialize(*) - super - - result[:references] ||= Hash.new { |hash, type| hash[type] = [] } - end - - def call - Querying.css(doc, 'a.gfm').each do |node| - gather_references(node) - end - - load_lazy_references unless ReferenceExtractor.lazy? - - doc - end - - private - - def gather_references(node) - return unless node.has_attribute?('data-reference-filter') - - reference_type = node.attr('data-reference-filter') - reference_filter = Banzai::Filter.const_get(reference_type) - - return if context[:reference_filter] && reference_filter != context[:reference_filter] - - return if author && !reference_filter.user_can_reference?(author, node, context) - - return unless reference_filter.user_can_see_reference?(current_user, node, context) - - references = reference_filter.referenced_by(node) - return unless references - - references.each do |type, values| - Array.wrap(values).each do |value| - result[:references][type] << value - end - end - end - - def load_lazy_references - refs = result[:references] - refs.each do |type, values| - refs[type] = ReferenceExtractor.lazily(values) - end - end - - def current_user - context[:current_user] - end - - def author - context[:author] - end - end - end -end diff --git a/lib/banzai/filter/snippet_reference_filter.rb b/lib/banzai/filter/snippet_reference_filter.rb index d507eb5ebe1..212a0bbf2a0 100644 --- a/lib/banzai/filter/snippet_reference_filter.rb +++ b/lib/banzai/filter/snippet_reference_filter.rb @@ -5,6 +5,8 @@ module Banzai # # This filter supports cross-project references. class SnippetReferenceFilter < AbstractReferenceFilter + self.reference_type = :snippet + def self.object_class Snippet end diff --git a/lib/banzai/filter/user_reference_filter.rb b/lib/banzai/filter/user_reference_filter.rb index eea3af842b6..331d8007257 100644 --- a/lib/banzai/filter/user_reference_filter.rb +++ b/lib/banzai/filter/user_reference_filter.rb @@ -4,6 +4,8 @@ module Banzai # # A special `@all` reference is also supported. class UserReferenceFilter < ReferenceFilter + self.reference_type = :user + # Public: Find `@user` user references in text # # UserReferenceFilter.references_in(text) do |match, username| @@ -21,43 +23,6 @@ module Banzai end end - def self.referenced_by(node) - if node.has_attribute?('data-group') - group = Group.find(node.attr('data-group')) rescue nil - return unless group - - { user: group.users } - elsif node.has_attribute?('data-user') - { user: LazyReference.new(User, node.attr('data-user')) } - elsif node.has_attribute?('data-project') - project = Project.find(node.attr('data-project')) rescue nil - return unless project - - { user: project.team.members.flatten } - end - end - - def self.user_can_see_reference?(user, node, context) - if node.has_attribute?('data-group') - group = Group.find(node.attr('data-group')) rescue nil - Ability.abilities.allowed?(user, :read_group, group) - else - super - end - end - - def self.user_can_reference?(user, node, context) - # Only team members can reference `@all` - if node.has_attribute?('data-project') - project = Project.find(node.attr('data-project')) rescue nil - return false unless project - - user && project.team.member?(user) - else - super - end - end - def call return doc if project.nil? @@ -114,9 +79,12 @@ module Banzai def link_to_all(link_text: nil) project = context[:project] + author = context[:author] + url = urls.namespace_project_url(project.namespace, project, only_path: context[:only_path]) - data = data_attribute(project: project.id) + + data = data_attribute(project: project.id, author: author.try(:id)) text = link_text || User.reference_prefix + 'all' link_tag(url, data, text) diff --git a/lib/banzai/lazy_reference.rb b/lib/banzai/lazy_reference.rb deleted file mode 100644 index 1095b4debc7..00000000000 --- a/lib/banzai/lazy_reference.rb +++ /dev/null @@ -1,25 +0,0 @@ -module Banzai - class LazyReference - def self.load(refs) - lazy_references, values = refs.partition { |ref| ref.is_a?(self) } - - lazy_values = lazy_references.group_by(&:klass).flat_map do |klass, refs| - ids = refs.flat_map(&:ids) - klass.where(id: ids) - end - - values + lazy_values - end - - attr_reader :klass, :ids - - def initialize(klass, ids) - @klass = klass - @ids = Array.wrap(ids).map(&:to_i) - end - - def load - self.klass.where(id: self.ids) - end - end -end diff --git a/lib/banzai/pipeline/reference_extraction_pipeline.rb b/lib/banzai/pipeline/reference_extraction_pipeline.rb deleted file mode 100644 index 919998380e4..00000000000 --- a/lib/banzai/pipeline/reference_extraction_pipeline.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Banzai - module Pipeline - class ReferenceExtractionPipeline < BasePipeline - def self.filters - FilterArray[ - Filter::ReferenceGathererFilter - ] - end - end - end -end diff --git a/lib/banzai/reference_extractor.rb b/lib/banzai/reference_extractor.rb index f4079538ec5..bf366962aef 100644 --- a/lib/banzai/reference_extractor.rb +++ b/lib/banzai/reference_extractor.rb @@ -1,28 +1,6 @@ module Banzai # Extract possible GFM references from an arbitrary String for further processing. class ReferenceExtractor - class << self - LAZY_KEY = :banzai_reference_extractor_lazy - - def lazy? - Thread.current[LAZY_KEY] - end - - def lazily(values = nil, &block) - return (values || block.call).uniq if lazy? - - begin - Thread.current[LAZY_KEY] = true - - values ||= block.call - - Banzai::LazyReference.load(values.uniq).uniq - ensure - Thread.current[LAZY_KEY] = false - end - end - end - def initialize @texts = [] end @@ -31,23 +9,21 @@ module Banzai @texts << Renderer.render(text, context) end - def references(type, context = {}) - filter = Banzai::Filter["#{type}_reference"] + def references(type, project, current_user = nil) + processor = Banzai::ReferenceParser[type]. + new(project, current_user) + + processor.process(html_documents) + end - context.merge!( - pipeline: :reference_extraction, + private - # ReferenceGathererFilter - reference_filter: filter - ) + def html_documents + # This ensures that we don't memoize anything until we have a number of + # text blobs to parse. + return [] if @texts.empty? - self.class.lazily do - @texts.flat_map do |html| - text_context = context.dup - result = Renderer.render_result(html, text_context) - result[:references][type] - end.uniq - end + @html_documents ||= @texts.map { |html| Nokogiri::HTML.fragment(html) } end end end diff --git a/lib/banzai/reference_parser.rb b/lib/banzai/reference_parser.rb new file mode 100644 index 00000000000..557bec4316e --- /dev/null +++ b/lib/banzai/reference_parser.rb @@ -0,0 +1,14 @@ +module Banzai + module ReferenceParser + # Returns the reference parser class for the given type + # + # Example: + # + # Banzai::ReferenceParser['issue'] + # + # This would return the `Banzai::ReferenceParser::IssueParser` class. + def self.[](name) + const_get("#{name.to_s.camelize}Parser") + end + end +end diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb new file mode 100644 index 00000000000..3d7b9c4a024 --- /dev/null +++ b/lib/banzai/reference_parser/base_parser.rb @@ -0,0 +1,204 @@ +module Banzai + module ReferenceParser + # Base class for reference parsing classes. + # + # Each parser should also specify its reference type by calling + # `self.reference_type = ...` in the body of the class. The value of this + # method should be a symbol such as `:issue` or `:merge_request`. For + # example: + # + # class IssueParser < BaseParser + # self.reference_type = :issue + # end + # + # The reference type is used to determine what nodes to pass to the + # `referenced_by` method. + # + # Parser classes should either implement the instance method + # `references_relation` or overwrite `referenced_by`. The + # `references_relation` method is supposed to return an + # ActiveRecord::Relation used as a base relation for retrieving the objects + # referenced in a set of HTML nodes. + # + # Each class can implement two additional methods: + # + # * `nodes_user_can_reference`: returns an Array of nodes the given user can + # refer to. + # * `nodes_visible_to_user`: returns an Array of nodes that are visible to + # the given user. + # + # You only need to overwrite these methods if you want to tweak who can see + # which references. For example, the IssueParser class defines its own + # `nodes_visible_to_user` method so it can ensure users can only see issues + # they have access to. + class BaseParser + class << self + attr_accessor :reference_type + end + + # Returns the attribute name containing the value for every object to be + # parsed by the current parser. + # + # For example, for a parser class that returns "Animal" objects this + # attribute would be "data-animal". + def self.data_attribute + @data_attribute ||= "data-#{reference_type.to_s.dasherize}" + end + + def initialize(project = nil, current_user = nil) + @project = project + @current_user = current_user + end + + # Returns all the nodes containing references that the user can refer to. + def nodes_user_can_reference(user, nodes) + nodes + end + + # Returns all the nodes that are visible to the given user. + def nodes_visible_to_user(user, nodes) + projects = lazy { projects_for_nodes(nodes) } + project_attr = 'data-project' + + nodes.select do |node| + if node.has_attribute?(project_attr) + node_id = node.attr(project_attr).to_i + + if project && project.id == node_id + true + else + can?(user, :read_project, projects[node_id]) + end + else + true + end + end + end + + # Returns an Array of objects referenced by any of the given HTML nodes. + def referenced_by(nodes) + ids = unique_attribute_values(nodes, self.class.data_attribute) + + references_relation.where(id: ids) + end + + # Returns the ActiveRecord::Relation to use for querying references in the + # DB. + def references_relation + raise NotImplementedError, + "#{self.class} does not implement #{__method__}" + end + + # Returns a Hash containing attribute values per project ID. + # + # The returned Hash uses the following format: + # + # { project id => [value1, value2, ...] } + # + # nodes - An Array of HTML nodes to process. + # attribute - The name of the attribute (as a String) for which to gather + # values. + # + # Returns a Hash. + def gather_attributes_per_project(nodes, attribute) + per_project = Hash.new { |hash, key| hash[key] = Set.new } + + nodes.each do |node| + project_id = node.attr('data-project').to_i + id = node.attr(attribute) + + per_project[project_id] << id if id + end + + per_project + end + + # Returns a Hash containing objects for an attribute grouped per their + # IDs. + # + # The returned Hash uses the following format: + # + # { id value => row } + # + # nodes - An Array of HTML nodes to process. + # + # collection - The model or ActiveRecord relation to use for retrieving + # rows from the database. + # + # attribute - The name of the attribute containing the primary key values + # for every row. + # + # Returns a Hash. + def grouped_objects_for_nodes(nodes, collection, attribute) + return {} if nodes.empty? + + ids = unique_attribute_values(nodes, attribute) + + collection.where(id: ids).each_with_object({}) do |row, hash| + hash[row.id] = row + end + end + + # Returns an Array containing all unique values of an attribute of the + # given nodes. + def unique_attribute_values(nodes, attribute) + values = Set.new + + nodes.each do |node| + if node.has_attribute?(attribute) + values << node.attr(attribute) + end + end + + values.to_a + end + + # Processes the list of HTML documents and returns an Array containing all + # the references. + def process(documents) + type = self.class.reference_type + + nodes = documents.flat_map do |document| + Querying.css(document, "a[data-reference-type='#{type}'].gfm").to_a + end + + gather_references(nodes) + end + + # Gathers the references for the given HTML nodes. + def gather_references(nodes) + nodes = nodes_user_can_reference(current_user, nodes) + nodes = nodes_visible_to_user(current_user, nodes) + + referenced_by(nodes) + end + + # Returns a Hash containing the projects for a given list of HTML nodes. + # + # The returned Hash uses the following format: + # + # { project ID => project } + # + def projects_for_nodes(nodes) + @projects_for_nodes ||= + grouped_objects_for_nodes(nodes, Project, 'data-project') + end + + def can?(user, permission, subject) + Ability.abilities.allowed?(user, permission, subject) + end + + def find_projects_for_hash_keys(hash) + Project.where(id: hash.keys) + end + + private + + attr_reader :current_user, :project + + def lazy(&block) + Gitlab::Lazy.new(&block) + end + end + end +end diff --git a/lib/banzai/reference_parser/commit_parser.rb b/lib/banzai/reference_parser/commit_parser.rb new file mode 100644 index 00000000000..0fee9d267de --- /dev/null +++ b/lib/banzai/reference_parser/commit_parser.rb @@ -0,0 +1,34 @@ +module Banzai + module ReferenceParser + class CommitParser < BaseParser + self.reference_type = :commit + + def referenced_by(nodes) + commit_ids = commit_ids_per_project(nodes) + projects = find_projects_for_hash_keys(commit_ids) + + projects.flat_map do |project| + find_commits(project, commit_ids[project.id]) + end + end + + def commit_ids_per_project(nodes) + gather_attributes_per_project(nodes, self.class.data_attribute) + end + + def find_commits(project, ids) + commits = [] + + return commits unless project.valid_repo? + + ids.each do |id| + commit = project.commit(id) + + commits << commit if commit + end + + commits + end + end + end +end diff --git a/lib/banzai/reference_parser/commit_range_parser.rb b/lib/banzai/reference_parser/commit_range_parser.rb new file mode 100644 index 00000000000..69d01f8db15 --- /dev/null +++ b/lib/banzai/reference_parser/commit_range_parser.rb @@ -0,0 +1,38 @@ +module Banzai + module ReferenceParser + class CommitRangeParser < BaseParser + self.reference_type = :commit_range + + def referenced_by(nodes) + range_ids = commit_range_ids_per_project(nodes) + projects = find_projects_for_hash_keys(range_ids) + + projects.flat_map do |project| + find_ranges(project, range_ids[project.id]) + end + end + + def commit_range_ids_per_project(nodes) + gather_attributes_per_project(nodes, self.class.data_attribute) + end + + def find_ranges(project, range_ids) + ranges = [] + + range_ids.each do |id| + range = find_object(project, id) + + ranges << range if range + end + + ranges + end + + def find_object(project, id) + range = CommitRange.new(id, project) + + range.valid_commits? ? range : nil + end + end + end +end diff --git a/lib/banzai/reference_parser/external_issue_parser.rb b/lib/banzai/reference_parser/external_issue_parser.rb new file mode 100644 index 00000000000..a1264db2111 --- /dev/null +++ b/lib/banzai/reference_parser/external_issue_parser.rb @@ -0,0 +1,25 @@ +module Banzai + module ReferenceParser + class ExternalIssueParser < BaseParser + self.reference_type = :external_issue + + def referenced_by(nodes) + issue_ids = issue_ids_per_project(nodes) + projects = find_projects_for_hash_keys(issue_ids) + issues = [] + + projects.each do |project| + issue_ids[project.id].each do |id| + issues << ExternalIssue.new(id, project) + end + end + + issues + end + + def issue_ids_per_project(nodes) + gather_attributes_per_project(nodes, self.class.data_attribute) + end + end + end +end diff --git a/lib/banzai/reference_parser/issue_parser.rb b/lib/banzai/reference_parser/issue_parser.rb new file mode 100644 index 00000000000..24076e3d9ec --- /dev/null +++ b/lib/banzai/reference_parser/issue_parser.rb @@ -0,0 +1,40 @@ +module Banzai + module ReferenceParser + class IssueParser < BaseParser + self.reference_type = :issue + + def nodes_visible_to_user(user, nodes) + # It is not possible to check access rights for external issue trackers + return nodes if project && project.external_issue_tracker + + issues = issues_for_nodes(nodes) + + nodes.select do |node| + issue = issue_for_node(issues, node) + + issue ? can?(user, :read_issue, issue) : false + end + end + + def referenced_by(nodes) + issues = issues_for_nodes(nodes) + + nodes.map { |node| issue_for_node(issues, node) }.uniq + end + + def issues_for_nodes(nodes) + @issues_for_nodes ||= grouped_objects_for_nodes( + nodes, + Issue.all.includes(:author, :assignee, :project), + self.class.data_attribute + ) + end + + private + + def issue_for_node(issues, node) + issues[node.attr(self.class.data_attribute).to_i] + end + end + end +end diff --git a/lib/banzai/reference_parser/label_parser.rb b/lib/banzai/reference_parser/label_parser.rb new file mode 100644 index 00000000000..e5d1eb11d7f --- /dev/null +++ b/lib/banzai/reference_parser/label_parser.rb @@ -0,0 +1,11 @@ +module Banzai + module ReferenceParser + class LabelParser < BaseParser + self.reference_type = :label + + def references_relation + Label + end + end + end +end diff --git a/lib/banzai/reference_parser/merge_request_parser.rb b/lib/banzai/reference_parser/merge_request_parser.rb new file mode 100644 index 00000000000..c9a9ca79c09 --- /dev/null +++ b/lib/banzai/reference_parser/merge_request_parser.rb @@ -0,0 +1,11 @@ +module Banzai + module ReferenceParser + class MergeRequestParser < BaseParser + self.reference_type = :merge_request + + def references_relation + MergeRequest.includes(:author, :assignee, :target_project) + end + end + end +end diff --git a/lib/banzai/reference_parser/milestone_parser.rb b/lib/banzai/reference_parser/milestone_parser.rb new file mode 100644 index 00000000000..a000ac61e5c --- /dev/null +++ b/lib/banzai/reference_parser/milestone_parser.rb @@ -0,0 +1,11 @@ +module Banzai + module ReferenceParser + class MilestoneParser < BaseParser + self.reference_type = :milestone + + def references_relation + Milestone + end + end + end +end diff --git a/lib/banzai/reference_parser/snippet_parser.rb b/lib/banzai/reference_parser/snippet_parser.rb new file mode 100644 index 00000000000..fa71b3c952a --- /dev/null +++ b/lib/banzai/reference_parser/snippet_parser.rb @@ -0,0 +1,11 @@ +module Banzai + module ReferenceParser + class SnippetParser < BaseParser + self.reference_type = :snippet + + def references_relation + Snippet + end + end + end +end diff --git a/lib/banzai/reference_parser/user_parser.rb b/lib/banzai/reference_parser/user_parser.rb new file mode 100644 index 00000000000..a12b0d19560 --- /dev/null +++ b/lib/banzai/reference_parser/user_parser.rb @@ -0,0 +1,92 @@ +module Banzai + module ReferenceParser + class UserParser < BaseParser + self.reference_type = :user + + def referenced_by(nodes) + group_ids = [] + user_ids = [] + project_ids = [] + + nodes.each do |node| + if node.has_attribute?('data-group') + group_ids << node.attr('data-group').to_i + elsif node.has_attribute?(self.class.data_attribute) + user_ids << node.attr(self.class.data_attribute).to_i + elsif node.has_attribute?('data-project') + project_ids << node.attr('data-project').to_i + end + end + + find_users_for_groups(group_ids) | find_users(user_ids) | + find_users_for_projects(project_ids) + end + + def nodes_visible_to_user(user, nodes) + group_attr = 'data-group' + groups = lazy { grouped_objects_for_nodes(nodes, Group, group_attr) } + visible = [] + remaining = [] + + nodes.each do |node| + if node.has_attribute?(group_attr) + node_group = groups[node.attr(group_attr).to_i] + + if node_group && + can?(user, :read_group, node_group) + visible << node + end + # Remaining nodes will be processed by the parent class' + # implementation of this method. + else + remaining << node + end + end + + visible + super(current_user, remaining) + end + + def nodes_user_can_reference(current_user, nodes) + project_attr = 'data-project' + author_attr = 'data-author' + + projects = lazy { projects_for_nodes(nodes) } + users = lazy { grouped_objects_for_nodes(nodes, User, author_attr) } + + nodes.select do |node| + project_id = node.attr(project_attr) + user_id = node.attr(author_attr) + + if project && project_id && project.id == project_id.to_i + true + elsif project_id && user_id + project = projects[project_id.to_i] + user = users[user_id.to_i] + + project && user ? project.team.member?(user) : false + else + true + end + end + end + + def find_users(ids) + return [] if ids.empty? + + User.where(id: ids).to_a + end + + def find_users_for_groups(ids) + return [] if ids.empty? + + User.joins(:group_members).where(members: { source_id: ids }).to_a + end + + def find_users_for_projects(ids) + return [] if ids.empty? + + Project.where(id: ids).flat_map { |p| p.team.members.to_a } + end + end + end +end diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 29c4e221dd4..92c7e8b9d88 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -1,18 +1,22 @@ module Gitlab module CurrentSettings def current_application_settings - key = :current_application_settings - - RequestStore.store[key] ||= begin - settings = nil + if RequestStore.active? + RequestStore.fetch(:current_application_settings) { ensure_application_settings! } + else + ensure_application_settings! + end + end - if connect_to_db? - settings = ::ApplicationSetting.current - settings ||= ::ApplicationSetting.create_from_defaults unless ActiveRecord::Migrator.needs_migration? - end + def ensure_application_settings! + settings = ::ApplicationSetting.cached - settings || fake_application_settings + if !settings && connect_to_db? + settings = ::ApplicationSetting.current + settings ||= ::ApplicationSetting.create_from_defaults unless ActiveRecord::Migrator.needs_migration? end + + settings || fake_application_settings end def fake_application_settings diff --git a/lib/gitlab/diff/parser.rb b/lib/gitlab/diff/parser.rb index 6fe7faa547a..522dd2b9428 100644 --- a/lib/gitlab/diff/parser.rb +++ b/lib/gitlab/diff/parser.rb @@ -17,16 +17,16 @@ module Gitlab Enumerator.new do |yielder| @lines.each do |line| next if filename?(line) - + full_line = line.delete("\n") - + if line.match(/^@@ -/) type = "match" - + line_old = line.match(/\-[0-9]*/)[0].to_i.abs rescue 0 line_new = line.match(/\+[0-9]*/)[0].to_i.abs rescue 0 - - next if line_old <= 1 && line_new <= 1 #top of file + + next if line_old <= 1 && line_new <= 1 # top of file yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new) line_obj_index += 1 next @@ -39,8 +39,8 @@ module Gitlab yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new) line_obj_index += 1 end - - + + case line[0] when "+" line_new += 1 diff --git a/lib/gitlab/gitlab_import/importer.rb b/lib/gitlab/gitlab_import/importer.rb index e32ef8a44c2..3f76ec97977 100644 --- a/lib/gitlab/gitlab_import/importer.rb +++ b/lib/gitlab/gitlab_import/importer.rb @@ -17,7 +17,7 @@ module Gitlab def execute project_identifier = CGI.escape(project.import_source) - #Issues && Comments + # Issues && Comments issues = client.issues(project_identifier) issues.each do |issue| diff --git a/lib/gitlab/lazy.rb b/lib/gitlab/lazy.rb new file mode 100644 index 00000000000..2a659ae4c74 --- /dev/null +++ b/lib/gitlab/lazy.rb @@ -0,0 +1,34 @@ +module Gitlab + # A class that can be wrapped around an expensive method call so it's only + # executed when actually needed. + # + # Usage: + # + # object = Gitlab::Lazy.new { some_expensive_work_here } + # + # object['foo'] + # object.bar + class Lazy < BasicObject + def initialize(&block) + @block = block + end + + def method_missing(name, *args, &block) + __evaluate__ + + @result.__send__(name, *args, &block) + end + + def respond_to_missing?(name, include_private = false) + __evaluate__ + + @result.respond_to?(name, include_private) || super + end + + private + + def __evaluate__ + @result = @block.call unless defined?(@result) + end + end +end diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index 71c5b6801fb..183bd10d6a3 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -74,7 +74,7 @@ module Gitlab end def notes - project.notes.user.search(query).order('updated_at DESC') + project.notes.user.search(query, as_user: @current_user).order('updated_at DESC') end def commits diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index 13c4d64c99b..11c0b01f0dc 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -4,10 +4,9 @@ module Gitlab REFERABLES = %i(user issue label milestone merge_request snippet commit commit_range) attr_accessor :project, :current_user, :author - def initialize(project, current_user = nil, author = nil) + def initialize(project, current_user = nil) @project = project @current_user = current_user - @author = author @references = {} @@ -18,17 +17,21 @@ module Gitlab super(text, context.merge(project: project)) end + def references(type) + super(type, project, current_user) + end + REFERABLES.each do |type| define_method("#{type}s") do - @references[type] ||= references(type, reference_context) + @references[type] ||= references(type) end end def issues if project && project.jira_tracker? - @references[:external_issue] ||= references(:external_issue, reference_context) + @references[:external_issue] ||= references(:external_issue) else - @references[:issue] ||= references(:issue, reference_context) + @references[:issue] ||= references(:issue) end end @@ -46,11 +49,5 @@ module Gitlab @pattern = Regexp.union(patterns.compact) end - - private - - def reference_context - { project: project, current_user: current_user, author: author } - end end end diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index fbe50d10ec5..209fa37d97d 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -11,7 +11,7 @@ describe RegistrationsController do let(:user_params) { { user: { name: "new_user", username: "new_username", email: "new@user.com", password: "Any_password" } } } context 'when sending email confirmation' do - before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(false) } + before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(false) } it 'logs user in directly' do post(:create, user_params) @@ -21,7 +21,7 @@ describe RegistrationsController do end context 'when not sending email confirmation' do - before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(true) } + before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) } it 'does not authenticate user and sends confirmation email' do post(:create, user_params) diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index ab57c52c7cd..b39d8c8cd5b 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -35,6 +35,27 @@ describe SessionsController do post(:create, { user: user_params }, { otp_user_id: user.id }) end + context 'remember_me field' do + it 'sets a remember_user_token cookie when enabled' do + allow(controller).to receive(:find_user).and_return(user) + expect(controller). + to receive(:remember_me).with(user).and_call_original + + authenticate_2fa(remember_me: '1', otp_attempt: user.current_otp) + + expect(response.cookies['remember_user_token']).to be_present + end + + it 'does nothing when disabled' do + allow(controller).to receive(:find_user).and_return(user) + expect(controller).not_to receive(:remember_me) + + authenticate_2fa(remember_me: '0', otp_attempt: user.current_otp) + + expect(response.cookies['remember_user_token']).to be_nil + end + end + ## # See #14900 issue # diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb index 8c38dd5b122..a7dc3b2701b 100644 --- a/spec/features/login_spec.rb +++ b/spec/features/login_spec.rb @@ -32,7 +32,7 @@ feature 'Login', feature: true do let(:user) { create(:user, :two_factor) } before do - login_with(user) + login_with(user, remember: true) expect(page).to have_content('Two-factor Authentication') end @@ -52,6 +52,12 @@ feature 'Login', feature: true do expect(current_path).to eq root_path end + it 'persists remember_me value via hidden field' do + field = first('input#user_remember_me', visible: false) + + expect(field.value).to eq '1' + end + it 'blocks login with invalid code' do enter_code('foo') expect(page).to have_content('Invalid two-factor code') diff --git a/spec/features/merge_requests/created_from_fork_spec.rb b/spec/features/merge_requests/created_from_fork_spec.rb new file mode 100644 index 00000000000..edc0bdec3db --- /dev/null +++ b/spec/features/merge_requests/created_from_fork_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +feature 'Merge request created from fork' do + given(:user) { create(:user) } + given(:project) { create(:project, :public) } + given(:fork_project) { create(:project, :public) } + + given!(:merge_request) do + create(:forked_project_link, forked_to_project: fork_project, + forked_from_project: project) + + create(:merge_request_with_diffs, source_project: fork_project, + target_project: project, + description: 'Test merge request') + end + + background do + fork_project.team << [user, :master] + login_as user + end + + scenario 'user can access merge request' do + visit_merge_request(merge_request) + + expect(page).to have_content 'Test merge request' + end + + context 'pipeline present in source project' do + include WaitForAjax + + given(:pipeline) do + create(:ci_commit_with_two_jobs, project: fork_project, + sha: merge_request.last_commit.id, + ref: merge_request.source_branch) + end + + background { pipeline.create_builds(user) } + + scenario 'user visits a pipelines page', js: true do + visit_merge_request(merge_request) + page.within('.merge-request-tabs') { click_link 'Builds' } + wait_for_ajax + + page.within('table.builds') do + expect(page).to have_content 'rspec' + expect(page).to have_content 'spinach' + end + + expect(find_link('Cancel running')[:href]) + .to include fork_project.path_with_namespace + end + end + + def visit_merge_request(mr) + visit namespace_project_merge_request_path(project.namespace, + project, mr) + end +end diff --git a/spec/features/merge_requests/user_lists_merge_requests_spec.rb b/spec/features/merge_requests/user_lists_merge_requests_spec.rb index 2c7e1c748ad..1c130057c56 100644 --- a/spec/features/merge_requests/user_lists_merge_requests_spec.rb +++ b/spec/features/merge_requests/user_lists_merge_requests_spec.rb @@ -131,6 +131,15 @@ describe 'Projects > Merge requests > User lists merge requests', feature: true expect(first_merge_request).to include('fix') expect(count_merge_requests).to eq(1) end + + it 'sorts by recently due milestone' do + visit namespace_project_merge_requests_path(project.namespace, project, + label_name: [label.name, label2.name], + assignee_id: user.id, + sort: sort_value_milestone_soon) + + expect(first_merge_request).to include('fix') + end end end diff --git a/spec/features/pipelines_spec.rb b/spec/features/pipelines_spec.rb index bef0578a9bb..acd6fb3538c 100644 --- a/spec/features/pipelines_spec.rb +++ b/spec/features/pipelines_spec.rb @@ -62,6 +62,36 @@ describe "Pipelines" do end end + context 'for generic statuses' do + context 'when running' do + let!(:running) { create(:generic_commit_status, status: 'running', commit: pipeline, stage: 'test') } + + before { visit namespace_project_pipelines_path(project.namespace, project) } + + it 'not be cancelable' do + expect(page).not_to have_link('Cancel') + end + + it 'pipeline is running' do + expect(page).to have_selector('.ci-running') + end + end + + context 'when failed' do + let!(:running) { create(:generic_commit_status, status: 'failed', commit: pipeline, stage: 'test') } + + before { visit namespace_project_pipelines_path(project.namespace, project) } + + it 'not be retryable' do + expect(page).not_to have_link('Retry') + end + + it 'pipeline is failed' do + expect(page).to have_selector('.ci-failed') + end + end + end + context 'downloadable pipelines' do context 'with artifacts' do let!(:with_artifacts) { create(:ci_build, :artifacts, :success, commit: pipeline, name: 'rspec tests', stage: 'test') } diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb index 3354f529295..4e627753cc7 100644 --- a/spec/features/todos/todos_spec.rb +++ b/spec/features/todos/todos_spec.rb @@ -43,6 +43,27 @@ describe 'Dashboard Todos', feature: true do end end + context 'User has Todos with labels spanning multiple projects' do + before do + label1 = create(:label, project: project) + note1 = create(:note_on_issue, note: "Hello #{label1.to_reference(format: :name)}", noteable_id: issue.id, noteable_type: 'Issue', project: issue.project) + create(:todo, :mentioned, project: project, target: issue, user: user, note_id: note1.id) + + project2 = create(:project) + label2 = create(:label, project: project2) + issue2 = create(:issue, project: project2) + note2 = create(:note_on_issue, note: "Test #{label2.to_reference(format: :name)}", noteable_id: issue2.id, noteable_type: 'Issue', project: project2) + create(:todo, :mentioned, project: project2, target: issue2, user: user, note_id: note2.id) + + login_as(user) + visit dashboard_todos_path + end + + it 'shows page with two Todos' do + expect(page).to have_selector('.todos-list .todo', count: 2) + end + end + context 'User has multiple pages of Todos' do before do allow(Todo).to receive(:default_per_page).and_return(1) diff --git a/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb b/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb index c2a8ad36c30..593bd6d5cac 100644 --- a/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb @@ -98,11 +98,6 @@ describe Banzai::Filter::CommitRangeReferenceFilter, lib: true do expect(link).not_to match %r(https?://) expect(link).to eq urls.namespace_project_compare_url(project.namespace, project, from: commit1.id, to: commit2.id, only_path: true) end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit_range]).not_to be_empty - end end context 'cross-project reference' do @@ -135,11 +130,6 @@ describe Banzai::Filter::CommitRangeReferenceFilter, lib: true do exp = act = "Fixed #{project2.to_reference}@#{commit1.id}...#{commit2.id.reverse}" expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit_range]).not_to be_empty - end end context 'cross-project URL reference' do @@ -173,10 +163,5 @@ describe Banzai::Filter::CommitRangeReferenceFilter, lib: true do exp = act = "Fixed #{project2.to_reference}@#{commit1.id}...#{commit2.id.reverse}" expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit_range]).not_to be_empty - end end end diff --git a/spec/lib/banzai/filter/commit_reference_filter_spec.rb b/spec/lib/banzai/filter/commit_reference_filter_spec.rb index 63a32d9d455..d46d3f1489e 100644 --- a/spec/lib/banzai/filter/commit_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_reference_filter_spec.rb @@ -93,11 +93,6 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do expect(link).not_to match %r(https?://) expect(link).to eq urls.namespace_project_commit_url(project.namespace, project, reference, only_path: true) end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit]).not_to be_empty - end end context 'cross-project reference' do @@ -124,11 +119,6 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do exp = act = "Committed #{invalidate_reference(reference)}" expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit]).not_to be_empty - end end context 'cross-project URL reference' do @@ -154,10 +144,5 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do act = "Committed #{invalidate_reference(reference)}" expect(reference_filter(act).to_html).to match(/<a.+>#{Regexp.escape(invalidate_reference(reference))}<\/a>/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit]).not_to be_empty - end end end diff --git a/spec/lib/banzai/filter/issue_reference_filter_spec.rb b/spec/lib/banzai/filter/issue_reference_filter_spec.rb index 266ebef33d6..8e6a264970d 100644 --- a/spec/lib/banzai/filter/issue_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/issue_reference_filter_spec.rb @@ -91,11 +91,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do expect(link).to eq helper.url_for_issue(issue.iid, project, only_path: true) end - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end - it 'does not process links containing issue numbers followed by text' do href = "#{reference}st" doc = reference_filter("<a href='#{href}'></a>") @@ -136,11 +131,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end end context 'cross-project URL reference' do @@ -160,11 +150,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do doc = reference_filter("Fixed (#{reference}.)") expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(issue.to_reference(project))} \(comment 123\)<\/a>\.\)/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end end context 'cross-project reference in link href' do @@ -184,11 +169,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do doc = reference_filter("Fixed (#{reference}.)") expect(doc.to_html).to match(/\(<a.+>Reference<\/a>\.\)/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end end context 'cross-project URL in link href' do @@ -208,10 +188,5 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do doc = reference_filter("Fixed (#{reference}.)") expect(doc.to_html).to match(/\(<a.+>Reference<\/a>\.\)/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end end end diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb index b0a38e7c251..f1064a701d8 100644 --- a/spec/lib/banzai/filter/label_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/label_reference_filter_spec.rb @@ -48,11 +48,6 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do expect(link).to eq urls.namespace_project_issues_path(project.namespace, project, label_name: label.name) end - it 'adds to the results hash' do - result = reference_pipeline_result("Label #{reference}") - expect(result[:references][:label]).to eq [label] - end - describe 'label span element' do it 'includes default classes' do doc = reference_filter("Label #{reference}") @@ -170,11 +165,6 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do expect(link).to have_attribute('data-label') expect(link.attr('data-label')).to eq label.id.to_s end - - it 'adds to the results hash' do - result = reference_pipeline_result("Label #{reference}") - expect(result[:references][:label]).to eq [label] - end end describe 'cross project label references' do diff --git a/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb b/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb index 352710df307..3185e41fe5c 100644 --- a/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb @@ -78,11 +78,6 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do expect(link).not_to match %r(https?://) expect(link).to eq urls.namespace_project_merge_request_url(project.namespace, project, merge, only_path: true) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Merge #{reference}") - expect(result[:references][:merge_request]).to eq [merge] - end end context 'cross-project reference' do @@ -109,11 +104,6 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("Merge #{reference}") - expect(result[:references][:merge_request]).to eq [merge] - end end context 'cross-project URL reference' do @@ -133,10 +123,5 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do doc = reference_filter("Merge (#{reference}.)") expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(merge.to_reference(project))} \(diffs, comment 123\)<\/a>\.\)/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Merge #{reference}") - expect(result[:references][:merge_request]).to eq [merge] - end end end diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb index bdf48eabb0e..9424f2363e1 100644 --- a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb @@ -48,11 +48,6 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do namespace_project_milestone_path(project.namespace, project, milestone) end - it 'adds to the results hash' do - result = reference_pipeline_result("Milestone #{reference}") - expect(result[:references][:milestone]).to eq [milestone] - end - context 'Integer-based references' do it 'links to a valid reference' do doc = reference_filter("See #{reference}") @@ -151,11 +146,6 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do expect(link).to have_attribute('data-milestone') expect(link.attr('data-milestone')).to eq milestone.id.to_s end - - it 'adds to the results hash' do - result = reference_pipeline_result("Milestone #{reference}") - expect(result[:references][:milestone]).to eq [milestone] - end end describe 'cross project milestone references' do diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb index c2c2fd0eb6a..697d10bbf70 100644 --- a/spec/lib/banzai/filter/redactor_filter_spec.rb +++ b/spec/lib/banzai/filter/redactor_filter_spec.rb @@ -16,11 +16,23 @@ describe Banzai::Filter::RedactorFilter, lib: true do end context 'with data-project' do + let(:parser_class) do + Class.new(Banzai::ReferenceParser::BaseParser) do + self.reference_type = :test + end + end + + before do + allow(Banzai::ReferenceParser).to receive(:[]). + with('test'). + and_return(parser_class) + end + it 'removes unpermitted Project references' do user = create(:user) project = create(:empty_project) - link = reference_link(project: project.id, reference_filter: 'ReferenceFilter') + link = reference_link(project: project.id, reference_type: 'test') doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 0 @@ -31,14 +43,14 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project) project.team << [user, :master] - link = reference_link(project: project.id, reference_filter: 'ReferenceFilter') + link = reference_link(project: project.id, reference_type: 'test') doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 end it 'handles invalid Project references' do - link = reference_link(project: 12345, reference_filter: 'ReferenceFilter') + link = reference_link(project: 12345, reference_type: 'test') expect { filter(link) }.not_to raise_error end @@ -51,7 +63,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: non_member) expect(doc.css('a').length).to eq 0 @@ -62,7 +74,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, :confidential, project: project, author: author) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: author) expect(doc.css('a').length).to eq 1 @@ -73,7 +85,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, :confidential, project: project, assignee: assignee) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: assignee) expect(doc.css('a').length).to eq 1 @@ -85,7 +97,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project.team << [member, :developer] issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: member) expect(doc.css('a').length).to eq 1 @@ -96,7 +108,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: admin) expect(doc.css('a').length).to eq 1 @@ -108,7 +120,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 @@ -121,7 +133,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do user = create(:user) group = create(:group, :private) - link = reference_link(group: group.id, reference_filter: 'UserReferenceFilter') + link = reference_link(group: group.id, reference_type: 'user') doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 0 @@ -132,14 +144,14 @@ describe Banzai::Filter::RedactorFilter, lib: true do group = create(:group, :private) group.add_developer(user) - link = reference_link(group: group.id, reference_filter: 'UserReferenceFilter') + link = reference_link(group: group.id, reference_type: 'user') doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 end it 'handles invalid Group references' do - link = reference_link(group: 12345, reference_filter: 'UserReferenceFilter') + link = reference_link(group: 12345, reference_type: 'user') expect { filter(link) }.not_to raise_error end @@ -149,7 +161,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do it 'allows any User reference' do user = create(:user) - link = reference_link(user: user.id, reference_filter: 'UserReferenceFilter') + link = reference_link(user: user.id, reference_type: 'user') doc = filter(link) expect(doc.css('a').length).to eq 1 diff --git a/spec/lib/banzai/filter/reference_gatherer_filter_spec.rb b/spec/lib/banzai/filter/reference_gatherer_filter_spec.rb deleted file mode 100644 index c8b1dfdf944..00000000000 --- a/spec/lib/banzai/filter/reference_gatherer_filter_spec.rb +++ /dev/null @@ -1,87 +0,0 @@ -require 'spec_helper' - -describe Banzai::Filter::ReferenceGathererFilter, lib: true do - include ActionView::Helpers::UrlHelper - include FilterSpecHelper - - def reference_link(data) - link_to('text', '', class: 'gfm', data: data) - end - - context "for issue references" do - - context 'with data-project' do - it 'removes unpermitted Project references' do - user = create(:user) - project = create(:empty_project) - issue = create(:issue, project: project) - - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') - result = pipeline_result(link, current_user: user) - - expect(result[:references][:issue]).to be_empty - end - - it 'allows permitted Project references' do - user = create(:user) - project = create(:empty_project) - issue = create(:issue, project: project) - project.team << [user, :master] - - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') - result = pipeline_result(link, current_user: user) - - expect(result[:references][:issue]).to eq([issue]) - end - - it 'handles invalid Project references' do - link = reference_link(project: 12345, issue: 12345, reference_filter: 'IssueReferenceFilter') - - expect { pipeline_result(link) }.not_to raise_error - end - end - end - - context "for user references" do - - context 'with data-group' do - it 'removes unpermitted Group references' do - user = create(:user) - group = create(:group) - - link = reference_link(group: group.id, reference_filter: 'UserReferenceFilter') - result = pipeline_result(link, current_user: user) - - expect(result[:references][:user]).to be_empty - end - - it 'allows permitted Group references' do - user = create(:user) - group = create(:group) - group.add_developer(user) - - link = reference_link(group: group.id, reference_filter: 'UserReferenceFilter') - result = pipeline_result(link, current_user: user) - - expect(result[:references][:user]).to eq([user]) - end - - it 'handles invalid Group references' do - link = reference_link(group: 12345, reference_filter: 'UserReferenceFilter') - - expect { pipeline_result(link) }.not_to raise_error - end - end - - context 'with data-user' do - it 'allows any User reference' do - user = create(:user) - - link = reference_link(user: user.id, reference_filter: 'UserReferenceFilter') - result = pipeline_result(link) - - expect(result[:references][:user]).to eq([user]) - end - end - end -end diff --git a/spec/lib/banzai/filter/snippet_reference_filter_spec.rb b/spec/lib/banzai/filter/snippet_reference_filter_spec.rb index 26466fbb180..5068ddd7faa 100644 --- a/spec/lib/banzai/filter/snippet_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/snippet_reference_filter_spec.rb @@ -77,11 +77,6 @@ describe Banzai::Filter::SnippetReferenceFilter, lib: true do expect(link).not_to match %r(https?://) expect(link).to eq urls.namespace_project_snippet_url(project.namespace, project, snippet, only_path: true) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Snippet #{reference}") - expect(result[:references][:snippet]).to eq [snippet] - end end context 'cross-project reference' do @@ -107,11 +102,6 @@ describe Banzai::Filter::SnippetReferenceFilter, lib: true do expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("Snippet #{reference}") - expect(result[:references][:snippet]).to eq [snippet] - end end context 'cross-project URL reference' do @@ -137,10 +127,5 @@ describe Banzai::Filter::SnippetReferenceFilter, lib: true do expect(reference_filter(act).to_html).to match(/<a.+>#{Regexp.escape(invalidate_reference(reference))}<\/a>/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Snippet #{reference}") - expect(result[:references][:snippet]).to eq [snippet] - end end end diff --git a/spec/lib/banzai/filter/user_reference_filter_spec.rb b/spec/lib/banzai/filter/user_reference_filter_spec.rb index 8bdebae1841..d7dfd6699ef 100644 --- a/spec/lib/banzai/filter/user_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/user_reference_filter_spec.rb @@ -31,28 +31,22 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do end it 'supports a special @all mention' do - doc = reference_filter("Hey #{reference}") + doc = reference_filter("Hey #{reference}", author: user) expect(doc.css('a').length).to eq 1 expect(doc.css('a').first.attr('href')) .to eq urls.namespace_project_url(project.namespace, project) end - context "when the author is a member of the project" do + it 'includes a data-author attribute when there is an author' do + doc = reference_filter(reference, author: user) - it 'adds to the results hash' do - result = reference_pipeline_result("Hey #{reference}", author: project.creator) - expect(result[:references][:user]).to eq [project.creator] - end + expect(doc.css('a').first.attr('data-author')).to eq(user.id.to_s) end - context "when the author is not a member of the project" do - - let(:other_user) { create(:user) } + it 'does not include a data-author attribute when there is no author' do + doc = reference_filter(reference) - it "doesn't add to the results hash" do - result = reference_pipeline_result("Hey #{reference}", author: other_user) - expect(result[:references][:user]).to eq [] - end + expect(doc.css('a').first.has_attribute?('data-author')).to eq(false) end end @@ -83,11 +77,6 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do expect(link).to have_attribute('data-user') expect(link.attr('data-user')).to eq user.namespace.owner_id.to_s end - - it 'adds to the results hash' do - result = reference_pipeline_result("Hey #{reference}") - expect(result[:references][:user]).to eq [user] - end end context 'mentioning a group' do @@ -106,11 +95,6 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do expect(link).to have_attribute('data-group') expect(link.attr('data-group')).to eq group.id.to_s end - - it 'adds to the results hash' do - result = reference_pipeline_result("Hey #{reference}") - expect(result[:references][:user]).to eq group.users - end end it 'links with adjacent text' do @@ -151,10 +135,5 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do expect(link).to have_attribute('data-user') expect(link.attr('data-user')).to eq user.namespace.owner_id.to_s end - - it 'adds to the results hash' do - result = reference_pipeline_result("Hey #{reference}") - expect(result[:references][:user]).to eq [user] - end end end diff --git a/spec/lib/banzai/reference_parser/base_parser_spec.rb b/spec/lib/banzai/reference_parser/base_parser_spec.rb new file mode 100644 index 00000000000..543b4786d84 --- /dev/null +++ b/spec/lib/banzai/reference_parser/base_parser_spec.rb @@ -0,0 +1,237 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::BaseParser, lib: true do + include ReferenceParserHelpers + + let(:user) { create(:user) } + let(:project) { create(:empty_project, :public) } + + subject do + klass = Class.new(described_class) do + self.reference_type = :foo + end + + klass.new(project, user) + end + + describe '.reference_type=' do + it 'sets the reference type' do + dummy = Class.new(described_class) + dummy.reference_type = :foo + + expect(dummy.reference_type).to eq(:foo) + end + end + + describe '#nodes_visible_to_user' do + let(:link) { empty_html_link } + + context 'when the link has a data-project attribute' do + it 'returns the nodes if the attribute value equals the current project ID' do + link['data-project'] = project.id.to_s + + expect(Ability.abilities).not_to receive(:allowed?) + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns the nodes if the user can read the project' do + other_project = create(:empty_project, :public) + + link['data-project'] = other_project.id.to_s + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, other_project). + and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns an empty Array when the attribute value is empty' do + link['data-project'] = '' + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + + it 'returns an empty Array when the user can not read the project' do + other_project = create(:empty_project, :public) + + link['data-project'] = other_project.id.to_s + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, other_project). + and_return(false) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'when the link does not have a data-project attribute' do + it 'returns the nodes' do + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + end + end + + describe '#nodes_user_can_reference' do + it 'returns the nodes' do + link = double(:link) + + expect(subject.nodes_user_can_reference(user, [link])).to eq([link]) + end + end + + describe '#referenced_by' do + context 'when references_relation is implemented' do + it 'returns a collection of objects' do + links = Nokogiri::HTML.fragment("<a data-foo='#{user.id}'></a>"). + children + + expect(subject).to receive(:references_relation).and_return(User) + expect(subject.referenced_by(links)).to eq([user]) + end + end + + context 'when references_relation is not implemented' do + it 'raises NotImplementedError' do + links = Nokogiri::HTML.fragment('<a data-foo="1"></a>').children + + expect { subject.referenced_by(links) }. + to raise_error(NotImplementedError) + end + end + end + + describe '#references_relation' do + it 'raises NotImplementedError' do + expect { subject.references_relation }.to raise_error(NotImplementedError) + end + end + + describe '#gather_attributes_per_project' do + it 'returns a Hash containing attribute values per project' do + link = Nokogiri::HTML.fragment('<a data-project="1" data-foo="2"></a>'). + children[0] + + hash = subject.gather_attributes_per_project([link], 'data-foo') + + expect(hash).to be_an_instance_of(Hash) + + expect(hash[1].to_a).to eq(['2']) + end + end + + describe '#grouped_objects_for_nodes' do + it 'returns a Hash grouping objects per ID' do + nodes = [double(:node)] + + expect(subject).to receive(:unique_attribute_values). + with(nodes, 'data-user'). + and_return([user.id]) + + hash = subject.grouped_objects_for_nodes(nodes, User, 'data-user') + + expect(hash).to eq({ user.id => user }) + end + + it 'returns an empty Hash when the list of nodes is empty' do + expect(subject.grouped_objects_for_nodes([], User, 'data-user')).to eq({}) + end + end + + describe '#unique_attribute_values' do + it 'returns an Array of unique values' do + link = double(:link) + + expect(link).to receive(:has_attribute?). + with('data-foo'). + twice. + and_return(true) + + expect(link).to receive(:attr). + with('data-foo'). + twice. + and_return('1') + + nodes = [link, link] + + expect(subject.unique_attribute_values(nodes, 'data-foo')).to eq(['1']) + end + end + + describe '#process' do + it 'gathers the references for every node matching the reference type' do + dummy = Class.new(described_class) do + self.reference_type = :test + end + + instance = dummy.new(project, user) + document = Nokogiri::HTML.fragment('<a class="gfm"></a><a class="gfm" data-reference-type="test"></a>') + + expect(instance).to receive(:gather_references). + with([document.children[1]]). + and_return([user]) + + expect(instance.process([document])).to eq([user]) + end + end + + describe '#gather_references' do + let(:link) { double(:link) } + + it 'does not process links a user can not reference' do + expect(subject).to receive(:nodes_user_can_reference). + with(user, [link]). + and_return([]) + + expect(subject).to receive(:referenced_by).with([]) + + subject.gather_references([link]) + end + + it 'does not process links a user can not see' do + expect(subject).to receive(:nodes_user_can_reference). + with(user, [link]). + and_return([link]) + + expect(subject).to receive(:nodes_visible_to_user). + with(user, [link]). + and_return([]) + + expect(subject).to receive(:referenced_by).with([]) + + subject.gather_references([link]) + end + + it 'returns the references if a user can reference and see a link' do + expect(subject).to receive(:nodes_user_can_reference). + with(user, [link]). + and_return([link]) + + expect(subject).to receive(:nodes_visible_to_user). + with(user, [link]). + and_return([link]) + + expect(subject).to receive(:referenced_by).with([link]) + + subject.gather_references([link]) + end + end + + describe '#can?' do + it 'delegates the permissions check to the Ability class' do + user = double(:user) + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, project) + + subject.can?(user, :read_project, project) + end + end + + describe '#find_projects_for_hash_keys' do + it 'returns a list of Projects' do + expect(subject.find_projects_for_hash_keys(project.id => project)). + to eq([project]) + end + end +end diff --git a/spec/lib/banzai/reference_parser/commit_parser_spec.rb b/spec/lib/banzai/reference_parser/commit_parser_spec.rb new file mode 100644 index 00000000000..0b76d29fce0 --- /dev/null +++ b/spec/lib/banzai/reference_parser/commit_parser_spec.rb @@ -0,0 +1,113 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::CommitParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + context 'when the link has a data-project attribute' do + before do + link['data-project'] = project.id.to_s + end + + context 'when the link has a data-commit attribute' do + before do + link['data-commit'] = '123' + end + + it 'returns an Array of commits' do + commit = double(:commit) + + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(true) + + expect(subject).to receive(:find_commits). + with(project, ['123']). + and_return([commit]) + + expect(subject.referenced_by([link])).to eq([commit]) + end + + it 'returns an empty Array when the commit could not be found' do + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(true) + + expect(subject).to receive(:find_commits). + with(project, ['123']). + and_return([]) + + expect(subject.referenced_by([link])).to eq([]) + end + + it 'skips projects without valid repositories' do + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(false) + + expect(subject.referenced_by([link])).to eq([]) + end + end + + context 'when the link does not have a data-commit attribute' do + it 'returns an empty Array' do + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(true) + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + context 'when the link does not have a data-project attribute' do + it 'returns an empty Array' do + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(true) + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + describe '#commit_ids_per_project' do + before do + link['data-project'] = project.id.to_s + end + + it 'returns a Hash containing commit IDs per project' do + link['data-commit'] = '123' + + hash = subject.commit_ids_per_project([link]) + + expect(hash).to be_an_instance_of(Hash) + + expect(hash[project.id].to_a).to eq(['123']) + end + + it 'does not add a project when the data-commit attribute is empty' do + hash = subject.commit_ids_per_project([link]) + + expect(hash).to be_empty + end + end + + describe '#find_commits' do + it 'returns an Array of commit objects' do + commit = double(:commit) + + expect(project).to receive(:commit).with('123').and_return(commit) + expect(project).to receive(:valid_repo?).and_return(true) + + expect(subject.find_commits(project, %w{123})).to eq([commit]) + end + + it 'skips commit IDs for which no commit could be found' do + expect(project).to receive(:commit).with('123').and_return(nil) + expect(project).to receive(:valid_repo?).and_return(true) + + expect(subject.find_commits(project, %w{123})).to eq([]) + end + end +end diff --git a/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb b/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb new file mode 100644 index 00000000000..ba982f38542 --- /dev/null +++ b/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb @@ -0,0 +1,120 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::CommitRangeParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + context 'when the link has a data-project attribute' do + before do + link['data-project'] = project.id.to_s + end + + context 'when the link as a data-commit-range attribute' do + before do + link['data-commit-range'] = '123..456' + end + + it 'returns an Array of commit ranges' do + range = double(:range) + + expect(subject).to receive(:find_object). + with(project, '123..456'). + and_return(range) + + expect(subject.referenced_by([link])).to eq([range]) + end + + it 'returns an empty Array when the commit range could not be found' do + expect(subject).to receive(:find_object). + with(project, '123..456'). + and_return(nil) + + expect(subject.referenced_by([link])).to eq([]) + end + end + + context 'when the link does not have a data-commit-range attribute' do + it 'returns an empty Array' do + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + context 'when the link does not have a data-project attribute' do + it 'returns an empty Array' do + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + describe '#commit_range_ids_per_project' do + before do + link['data-project'] = project.id.to_s + end + + it 'returns a Hash containing range IDs per project' do + link['data-commit-range'] = '123..456' + + hash = subject.commit_range_ids_per_project([link]) + + expect(hash).to be_an_instance_of(Hash) + + expect(hash[project.id].to_a).to eq(['123..456']) + end + + it 'does not add a project when the data-commit-range attribute is empty' do + hash = subject.commit_range_ids_per_project([link]) + + expect(hash).to be_empty + end + end + + describe '#find_ranges' do + it 'returns an Array of range objects' do + range = double(:commit) + + expect(subject).to receive(:find_object). + with(project, '123..456'). + and_return(range) + + expect(subject.find_ranges(project, ['123..456'])).to eq([range]) + end + + it 'skips ranges that could not be found' do + expect(subject).to receive(:find_object). + with(project, '123..456'). + and_return(nil) + + expect(subject.find_ranges(project, ['123..456'])).to eq([]) + end + end + + describe '#find_object' do + let(:range) { double(:range) } + + before do + expect(CommitRange).to receive(:new).and_return(range) + end + + context 'when the range has valid commits' do + it 'returns the commit range' do + expect(range).to receive(:valid_commits?).and_return(true) + + expect(subject.find_object(project, '123..456')).to eq(range) + end + end + + context 'when the range does not have any valid commits' do + it 'returns nil' do + expect(range).to receive(:valid_commits?).and_return(false) + + expect(subject.find_object(project, '123..456')).to be_nil + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb b/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb new file mode 100644 index 00000000000..a6ef8394fe7 --- /dev/null +++ b/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb @@ -0,0 +1,62 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::ExternalIssueParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + context 'when the link has a data-project attribute' do + before do + link['data-project'] = project.id.to_s + end + + context 'when the link has a data-external-issue attribute' do + it 'returns an Array of ExternalIssue instances' do + link['data-external-issue'] = '123' + + refs = subject.referenced_by([link]) + + expect(refs).to eq([ExternalIssue.new('123', project)]) + end + end + + context 'when the link does not have a data-external-issue attribute' do + it 'returns an empty Array' do + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + context 'when the link does not have a data-project attribute' do + it 'returns an empty Array' do + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + describe '#issue_ids_per_project' do + before do + link['data-project'] = project.id.to_s + end + + it 'returns a Hash containing range IDs per project' do + link['data-external-issue'] = '123' + + hash = subject.issue_ids_per_project([link]) + + expect(hash).to be_an_instance_of(Hash) + + expect(hash[project.id].to_a).to eq(['123']) + end + + it 'does not add a project when the data-external-issue attribute is empty' do + hash = subject.issue_ids_per_project([link]) + + expect(hash).to be_empty + end + end +end diff --git a/spec/lib/banzai/reference_parser/issue_parser_spec.rb b/spec/lib/banzai/reference_parser/issue_parser_spec.rb new file mode 100644 index 00000000000..514c752546d --- /dev/null +++ b/spec/lib/banzai/reference_parser/issue_parser_spec.rb @@ -0,0 +1,79 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::IssueParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + let(:issue) { create(:issue, project: project) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#nodes_visible_to_user' do + context 'when the link has a data-issue attribute' do + before do + link['data-issue'] = issue.id.to_s + end + + it 'returns the nodes when the user can read the issue' do + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_issue, issue). + and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns an empty Array when the user can not read the issue' do + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_issue, issue). + and_return(false) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'when the link does not have a data-issue attribute' do + it 'returns an empty Array' do + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'when the project uses an external issue tracker' do + it 'returns all nodes' do + link = double(:link) + + expect(project).to receive(:external_issue_tracker).and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + end + end + + describe '#referenced_by' do + context 'when the link has a data-issue attribute' do + context 'using an existing issue ID' do + before do + link['data-issue'] = issue.id.to_s + end + + it 'returns an Array of issues' do + expect(subject.referenced_by([link])).to eq([issue]) + end + + it 'returns an empty Array when the list of nodes is empty' do + expect(subject.referenced_by([link])).to eq([issue]) + expect(subject.referenced_by([])).to eq([]) + end + end + end + end + + describe '#issues_for_nodes' do + it 'returns a Hash containing the issues for a list of nodes' do + link['data-issue'] = issue.id.to_s + nodes = [link] + + expect(subject.issues_for_nodes(nodes)).to eq({ issue.id => issue }) + end + end +end diff --git a/spec/lib/banzai/reference_parser/label_parser_spec.rb b/spec/lib/banzai/reference_parser/label_parser_spec.rb new file mode 100644 index 00000000000..77fda47f0e7 --- /dev/null +++ b/spec/lib/banzai/reference_parser/label_parser_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::LabelParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + let(:label) { create(:label, project: project) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + describe 'when the link has a data-label attribute' do + context 'using an existing label ID' do + it 'returns an Array of labels' do + link['data-label'] = label.id.to_s + + expect(subject.referenced_by([link])).to eq([label]) + end + end + + context 'using a non-existing label ID' do + it 'returns an empty Array' do + link['data-label'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb b/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb new file mode 100644 index 00000000000..cf89ad598ea --- /dev/null +++ b/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::MergeRequestParser, lib: true do + include ReferenceParserHelpers + + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request) } + subject { described_class.new(merge_request.target_project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + describe 'when the link has a data-merge-request attribute' do + context 'using an existing merge request ID' do + it 'returns an Array of merge requests' do + link['data-merge-request'] = merge_request.id.to_s + + expect(subject.referenced_by([link])).to eq([merge_request]) + end + end + + context 'using a non-existing merge request ID' do + it 'returns an empty Array' do + link['data-merge-request'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/milestone_parser_spec.rb b/spec/lib/banzai/reference_parser/milestone_parser_spec.rb new file mode 100644 index 00000000000..6aa45a22cc4 --- /dev/null +++ b/spec/lib/banzai/reference_parser/milestone_parser_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::MilestoneParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + let(:milestone) { create(:milestone, project: project) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + describe 'when the link has a data-milestone attribute' do + context 'using an existing milestone ID' do + it 'returns an Array of milestones' do + link['data-milestone'] = milestone.id.to_s + + expect(subject.referenced_by([link])).to eq([milestone]) + end + end + + context 'using a non-existing milestone ID' do + it 'returns an empty Array' do + link['data-milestone'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/snippet_parser_spec.rb b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb new file mode 100644 index 00000000000..59127b7c5d1 --- /dev/null +++ b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::SnippetParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + let(:snippet) { create(:snippet, project: project) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + describe 'when the link has a data-snippet attribute' do + context 'using an existing snippet ID' do + it 'returns an Array of snippets' do + link['data-snippet'] = snippet.id.to_s + + expect(subject.referenced_by([link])).to eq([snippet]) + end + end + + context 'using a non-existing snippet ID' do + it 'returns an empty Array' do + link['data-snippet'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/user_parser_spec.rb b/spec/lib/banzai/reference_parser/user_parser_spec.rb new file mode 100644 index 00000000000..9a82891297d --- /dev/null +++ b/spec/lib/banzai/reference_parser/user_parser_spec.rb @@ -0,0 +1,189 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::UserParser, lib: true do + include ReferenceParserHelpers + + let(:group) { create(:group) } + let(:user) { create(:user) } + let(:project) { create(:empty_project, :public, group: group, creator: user) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + context 'when the link has a data-group attribute' do + context 'using an existing group ID' do + before do + link['data-group'] = project.group.id.to_s + end + + it 'returns the users of the group' do + create(:group_member, group: group, user: user) + + expect(subject.referenced_by([link])).to eq([user]) + end + + it 'returns an empty Array when the group has no users' do + expect(subject.referenced_by([link])).to eq([]) + end + end + + context 'using a non-existing group ID' do + it 'returns an empty Array' do + link['data-group'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + context 'when the link has a data-user attribute' do + it 'returns an Array of users' do + link['data-user'] = user.id.to_s + + expect(subject.referenced_by([link])).to eq([user]) + end + end + + context 'when the link has a data-project attribute' do + context 'using an existing project ID' do + let(:contributor) { create(:user) } + + before do + project.team << [user, :developer] + project.team << [contributor, :developer] + end + + it 'returns the members of a project' do + link['data-project'] = project.id.to_s + + # This uses an explicit sort to make sure this spec doesn't randomly + # fail when objects are returned in a different order. + refs = subject.referenced_by([link]).sort_by(&:id) + + expect(refs).to eq([user, contributor]) + end + end + + context 'using a non-existing project ID' do + it 'returns an empty Array' do + link['data-project'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end + + describe '#nodes_visible_to_use?' do + context 'when the link has a data-group attribute' do + context 'using an existing group ID' do + before do + link['data-group'] = group.id.to_s + end + + it 'returns the nodes if the user can read the group' do + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_group, group). + and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns an empty Array if the user can not read the group' do + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_group, group). + and_return(false) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'when the link does not have a data-group attribute' do + context 'with a data-project attribute' do + it 'returns the nodes if the attribute value equals the current project ID' do + link['data-project'] = project.id.to_s + + expect(Ability.abilities).not_to receive(:allowed?) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns the nodes if the user can read the project' do + other_project = create(:empty_project, :public) + + link['data-project'] = other_project.id.to_s + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, other_project). + and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns an empty Array if the user can not read the project' do + other_project = create(:empty_project, :public) + + link['data-project'] = other_project.id.to_s + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, other_project). + and_return(false) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'without a data-project attribute' do + it 'returns the nodes' do + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + end + end + end + end + + describe '#nodes_user_can_reference' do + context 'when the link has a data-author attribute' do + it 'returns the nodes when the user is a member of the project' do + other_project = create(:project) + other_project.team << [user, :developer] + + link['data-project'] = other_project.id.to_s + link['data-author'] = user.id.to_s + + expect(subject.nodes_user_can_reference(user, [link])).to eq([link]) + end + + it 'returns an empty Array when the project could not be found' do + link['data-project'] = '' + link['data-author'] = user.id.to_s + + expect(subject.nodes_user_can_reference(user, [link])).to eq([]) + end + + it 'returns an empty Array when the user could not be found' do + other_project = create(:project) + + link['data-project'] = other_project.id.to_s + link['data-author'] = '' + + expect(subject.nodes_user_can_reference(user, [link])).to eq([]) + end + + it 'returns an empty Array when the user is not a team member' do + other_project = create(:project) + + link['data-project'] = other_project.id.to_s + link['data-author'] = user.id.to_s + + expect(subject.nodes_user_can_reference(user, [link])).to eq([]) + end + end + + context 'when the link does not have a data-author attribute' do + it 'returns the nodes' do + expect(subject.nodes_user_can_reference(user, [link])).to eq([link]) + end + end + end +end diff --git a/spec/lib/gitlab/akismet_helper_spec.rb b/spec/lib/gitlab/akismet_helper_spec.rb index 53f5d6c5c80..88a71528867 100644 --- a/spec/lib/gitlab/akismet_helper_spec.rb +++ b/spec/lib/gitlab/akismet_helper_spec.rb @@ -6,8 +6,8 @@ describe Gitlab::AkismetHelper, type: :helper do before do allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url)) - current_application_settings.akismet_enabled = true - current_application_settings.akismet_api_key = '12345' + allow_any_instance_of(ApplicationSetting).to receive(:akismet_enabled).and_return(true) + allow_any_instance_of(ApplicationSetting).to receive(:akismet_api_key).and_return('12345') end describe '#check_for_spam?' do diff --git a/spec/lib/gitlab/lazy_spec.rb b/spec/lib/gitlab/lazy_spec.rb new file mode 100644 index 00000000000..b5ca89dd242 --- /dev/null +++ b/spec/lib/gitlab/lazy_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Gitlab::Lazy, lib: true do + let(:dummy) { double(:dummy) } + + context 'when not calling any methods' do + it 'does not call the supplied block' do + expect(dummy).not_to receive(:foo) + + described_class.new { dummy.foo } + end + end + + context 'when calling a method on the object' do + it 'lazy loads the value returned by the block' do + expect(dummy).to receive(:foo).and_return('foo') + + lazy = described_class.new { dummy.foo } + + expect(lazy.to_s).to eq('foo') + end + end + + describe '#respond_to?' do + it 'returns true for a method defined on the wrapped object' do + lazy = described_class.new { 'foo' } + + expect(lazy).to respond_to(:downcase) + end + + it 'returns false for a method not defined on the wrapped object' do + lazy = described_class.new { 'foo' } + + expect(lazy).not_to respond_to(:quack) + end + end +end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index b963a3e0324..818825b1477 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -51,7 +51,7 @@ describe Notify do context 'when enabled email_author_in_body' do before do - allow(current_application_settings).to receive(:email_author_in_body).and_return(true) + allow_any_instance_of(ApplicationSetting).to receive(:email_author_in_body).and_return(true) end it 'contains a link to note author' do @@ -230,7 +230,7 @@ describe Notify do context 'when enabled email_author_in_body' do before do - allow(current_application_settings).to receive(:email_author_in_body).and_return(true) + allow_any_instance_of(ApplicationSetting).to receive(:email_author_in_body).and_return(true) end it 'contains a link to note author' do @@ -454,7 +454,7 @@ describe Notify do context 'when enabled email_author_in_body' do before do - allow(current_application_settings).to receive(:email_author_in_body).and_return(true) + allow_any_instance_of(ApplicationSetting).to receive(:email_author_in_body).and_return(true) end it 'contains a link to note author' do diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb new file mode 100644 index 00000000000..1acb5846fcf --- /dev/null +++ b/spec/models/ability_spec.rb @@ -0,0 +1,117 @@ +require 'spec_helper' + +describe Ability, lib: true do + describe '.users_that_can_read_project' do + context 'using a public project' do + it 'returns all the users' do + project = create(:project, :public) + user = build(:user) + + expect(described_class.users_that_can_read_project([user], project)). + to eq([user]) + end + end + + context 'using an internal project' do + let(:project) { create(:project, :internal) } + + it 'returns users that are administrators' do + user = build(:user, admin: true) + + expect(described_class.users_that_can_read_project([user], project)). + to eq([user]) + end + + it 'returns internal users while skipping external users' do + user1 = build(:user) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns external users if they are the project owner' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(project).to receive(:owner).twice.and_return(user1) + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns external users if they are project members' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(project.team).to receive(:members).twice.and_return([user1]) + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns an empty Array if all users are external users without access' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(described_class.users_that_can_read_project(users, project)). + to eq([]) + end + end + + context 'using a private project' do + let(:project) { create(:project, :private) } + + it 'returns users that are administrators' do + user = build(:user, admin: true) + + expect(described_class.users_that_can_read_project([user], project)). + to eq([user]) + end + + it 'returns external users if they are the project owner' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(project).to receive(:owner).twice.and_return(user1) + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns external users if they are project members' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(project.team).to receive(:members).twice.and_return([user1]) + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns an empty Array if all users are internal users without access' do + user1 = build(:user) + user2 = build(:user) + users = [user1, user2] + + expect(described_class.users_that_can_read_project(users, project)). + to eq([]) + end + + it 'returns an empty Array if all users are external users without access' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(described_class.users_that_can_read_project(users, project)). + to eq([]) + end + end + end +end diff --git a/spec/models/commit_range_spec.rb b/spec/models/commit_range_spec.rb index 9307d97e214..6bc496414a3 100644 --- a/spec/models/commit_range_spec.rb +++ b/spec/models/commit_range_spec.rb @@ -24,6 +24,16 @@ describe CommitRange, models: true do expect { described_class.new("Foo", project) }.to raise_error(ArgumentError) end + describe '#initialize' do + it 'does not modify strings in-place' do + input = "#{sha_from}...#{sha_to} " + + described_class.new(input, project) + + expect(input).to eq("#{sha_from}...#{sha_to} ") + end + end + describe '#to_s' do it 'is correct for three-dot syntax' do expect(range.to_s).to eq "#{full_sha_from}...#{full_sha_to}" @@ -135,4 +145,27 @@ describe CommitRange, models: true do end end end + + describe '#has_been_reverted?' do + it 'returns true if the commit has been reverted' do + issue = create(:issue) + + create(:note_on_issue, + noteable_id: issue.id, + system: true, + note: commit1.revert_description) + + expect_any_instance_of(Commit).to receive(:reverts_commit?). + with(commit1). + and_return(true) + + expect(commit1.has_been_reverted?(nil, issue)).to eq(true) + end + + it 'returns false a commit has not been reverted' do + issue = create(:issue) + + expect(commit1.has_been_reverted?(nil, issue)).to eq(false) + end + end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index ccb100cd96f..beca8708c9d 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Commit, models: true do - let(:project) { create(:project) } + let(:project) { create(:project, :public) } let(:commit) { project.commit } describe 'modules' do @@ -171,4 +171,40 @@ eos describe '#status' do # TODO: kamil end + + describe '#participants' do + let(:user1) { build(:user) } + let(:user2) { build(:user) } + + let!(:note1) do + create(:note_on_commit, + commit_id: commit.id, + project: project, + note: 'foo') + end + + let!(:note2) do + create(:note_on_commit, + commit_id: commit.id, + project: project, + note: 'bar') + end + + before do + allow(commit).to receive(:author).and_return(user1) + allow(commit).to receive(:committer).and_return(user2) + end + + it 'includes the commit author' do + expect(commit.participants).to include(commit.author) + end + + it 'includes the committer' do + expect(commit.participants).to include(commit.committer) + end + + it 'includes the authors of the commit notes' do + expect(commit.participants).to include(note1.author, note2.author) + end + end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 70bbe633269..fb20578d8d3 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -118,10 +118,10 @@ describe Issue, "Issuable" do let(:project) { build_stubbed(:empty_project) } context "by milestone due date" do - #Correct order is: - #Issues/MRs with milestones ordered by date - #Issues/MRs with milestones without dates - #Issues/MRs without milestones + # Correct order is: + # Issues/MRs with milestones ordered by date + # Issues/MRs with milestones without dates + # Issues/MRs without milestones let!(:issue) { create(:issue, project: project) } let!(:early_milestone) { create(:milestone, project: project, due_date: 10.days.from_now) } diff --git a/spec/models/concerns/participable_spec.rb b/spec/models/concerns/participable_spec.rb new file mode 100644 index 00000000000..7e4ea0f2d66 --- /dev/null +++ b/spec/models/concerns/participable_spec.rb @@ -0,0 +1,83 @@ +require 'spec_helper' + +describe Participable, models: true do + let(:model) do + Class.new do + include Participable + end + end + + describe '.participant' do + it 'adds the participant attributes to the existing list' do + model.participant(:foo) + model.participant(:bar) + + expect(model.participant_attrs).to eq([:foo, :bar]) + end + end + + describe '#participants' do + it 'returns the list of participants' do + model.participant(:foo) + model.participant(:bar) + + user1 = build(:user) + user2 = build(:user) + user3 = build(:user) + project = build(:project, :public) + instance = model.new + + expect(instance).to receive(:foo).and_return(user2) + expect(instance).to receive(:bar).and_return(user3) + expect(instance).to receive(:project).twice.and_return(project) + + participants = instance.participants(user1) + + expect(participants).to include(user2) + expect(participants).to include(user3) + end + + it 'supports attributes returning another Participable' do + other_model = Class.new { include Participable } + + other_model.participant(:bar) + model.participant(:foo) + + instance = model.new + other = other_model.new + user1 = build(:user) + user2 = build(:user) + project = build(:project, :public) + + expect(instance).to receive(:foo).and_return(other) + expect(other).to receive(:bar).and_return(user2) + expect(instance).to receive(:project).twice.and_return(project) + + expect(instance.participants(user1)).to eq([user2]) + end + + context 'when using a Proc as an attribute' do + it 'calls the supplied Proc' do + user1 = build(:user) + project = build(:project, :public) + + user_arg = nil + ext_arg = nil + + model.participant -> (user, ext) do + user_arg = user + ext_arg = ext + end + + instance = model.new + + expect(instance).to receive(:project).twice.and_return(project) + + instance.participants(user1) + + expect(user_arg).to eq(user1) + expect(ext_arg).to be_an_instance_of(Gitlab::ReferenceExtractor) + end + end + end +end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 6540d77fbc0..87b3d8d650a 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -231,4 +231,42 @@ describe Issue, models: true do expect(issue.to_branch_name).to match /confidential-issue\z/ end end + + describe '#participants' do + context 'using a public project' do + let(:project) { create(:project, :public) } + let(:issue) { create(:issue, project: project) } + + let!(:note1) do + create(:note_on_issue, noteable: issue, project: project, note: 'a') + end + + let!(:note2) do + create(:note_on_issue, noteable: issue, project: project, note: 'b') + end + + it 'includes the issue author' do + expect(issue.participants).to include(issue.author) + end + + it 'includes the authors of the notes' do + expect(issue.participants).to include(note1.author, note2.author) + end + end + + context 'using a private project' do + it 'does not include mentioned users that do not have access to the project' do + project = create(:project) + user = create(:user) + issue = create(:issue, project: project) + + create(:note_on_issue, + noteable: issue, + project: project, + note: user.to_reference) + + expect(issue.participants).not_to include(user) + end + end + end end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 9f26d9eb5ce..9f13874b532 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -20,6 +20,48 @@ require 'spec_helper' describe ProjectMember, models: true do + describe 'associations' do + it { is_expected.to belong_to(:project).class_name('Project').with_foreign_key(:source_id) } + end + + describe 'validations' do + it { is_expected.to allow_value('Project').for(:source_type) } + it { is_expected.not_to allow_value('project').for(:source_type) } + end + + describe 'modules' do + it { is_expected.to include_module(Gitlab::ShellAdapter) } + end + + describe "#destroy" do + let(:owner) { create(:project_member, access_level: ProjectMember::OWNER) } + let(:project) { owner.project } + let(:master) { create(:project_member, project: project) } + + let(:owner_todos) { (0...2).map { create(:todo, user: owner.user, project: project) } } + let(:master_todos) { (0...3).map { create(:todo, user: master.user, project: project) } } + + before do + owner_todos + master_todos + end + + it "destroy itself and delete associated todos" do + expect(owner.user.todos.size).to eq(2) + expect(master.user.todos.size).to eq(3) + expect(Todo.count).to eq(5) + + master_todo_ids = master_todos.map(&:id) + master.destroy + + expect(owner.user.todos.size).to eq(2) + expect(Todo.count).to eq(2) + master_todo_ids.each do |id| + expect(Todo.exists?(id)).to eq(false) + end + end + end + describe :import_team do before do @abilities = Six.new diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 4b67c2facf3..118e1e22a78 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -414,4 +414,28 @@ describe MergeRequest, models: true do end end end + + describe '#participants' do + let(:project) { create(:project, :public) } + + let(:mr) do + create(:merge_request, source_project: project, target_project: project) + end + + let!(:note1) do + create(:note_on_merge_request, noteable: mr, project: project, note: 'a') + end + + let!(:note2) do + create(:note_on_merge_request, noteable: mr, project: project, note: 'b') + end + + it 'includes the merge request author' do + expect(mr.participants).to include(mr.author) + end + + it 'includes the authors of the notes' do + expect(mr.participants).to include(note1.author, note2.author) + end + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 8aad1b73add..b25150f7055 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -121,8 +121,19 @@ describe Note, models: true do let!(:note2) { create(:note_on_issue) } it "reads the rendered note body from the cache" do - expect(Banzai::Renderer).to receive(:render).with(note1.note, pipeline: :note, cache_key: [note1, "note"], project: note1.project) - expect(Banzai::Renderer).to receive(:render).with(note2.note, pipeline: :note, cache_key: [note2, "note"], project: note2.project) + expect(Banzai::Renderer).to receive(:render). + with(note1.note, + pipeline: :note, + cache_key: [note1, "note"], + project: note1.project, + author: note1.author) + + expect(Banzai::Renderer).to receive(:render). + with(note2.note, + pipeline: :note, + cache_key: [note2, "note"], + project: note2.project, + author: note2.author) note1.all_references note2.all_references @@ -139,6 +150,25 @@ describe Note, models: true do it 'returns notes with matching content regardless of the casing' do expect(described_class.search('WOW')).to eq([note]) end + + context "confidential issues" do + let(:user) { create :user } + let(:confidential_issue) { create(:issue, :confidential, author: user) } + let(:confidential_note) { create :note, note: "Random", noteable: confidential_issue } + + it "returns notes with matching content if user can see the issue" do + expect(described_class.search(confidential_note.note, as_user: user)).to eq([confidential_note]) + end + + it "does not return notes with matching content if user can not see the issue" do + user = create :user + expect(described_class.search(confidential_note.note, as_user: user)).to be_empty + end + + it "does not return notes with matching content for unauthenticated users" do + expect(described_class.search(confidential_note.note)).to be_empty + end + end end describe '.grouped_awards' do @@ -229,4 +259,14 @@ describe Note, models: true do expect { note.valid? }.to change(note, :line_code).to(nil) end end + + describe '#participants' do + it 'includes the note author' do + project = create(:project, :public) + issue = create(:issue, project: project) + note = create(:note_on_issue, noteable: issue, project: project) + + expect(note.participants).to include(note.author) + end + end end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 7a613e360d4..789816bf2c7 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -87,4 +87,31 @@ describe Snippet, models: true do expect(described_class.search_code('FOO')).to eq([snippet]) end end + + describe '#participants' do + let(:project) { create(:project, :public) } + let(:snippet) { create(:snippet, content: 'foo', project: project) } + + let!(:note1) do + create(:note_on_project_snippet, + noteable: snippet, + project: project, + note: 'a') + end + + let!(:note2) do + create(:note_on_project_snippet, + noteable: snippet, + project: project, + note: 'b') + end + + it 'includes the snippet author' do + expect(snippet.participants).to include(snippet.author) + end + + it 'includes the note authors' do + expect(snippet.participants).to include(note1.author, note2.author) + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9581990666b..548bec364f8 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -67,7 +67,7 @@ describe User, models: true do describe 'email' do context 'when no signup domains listed' do - before { allow(current_application_settings).to receive(:restricted_signup_domains).and_return([]) } + before { allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return([]) } it 'accepts any email' do user = build(:user, email: "info@example.com") expect(user).to be_valid @@ -75,7 +75,7 @@ describe User, models: true do end context 'when a signup domain is listed and subdomains are allowed' do - before { allow(current_application_settings).to receive(:restricted_signup_domains).and_return(['example.com', '*.example.com']) } + before { allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return(['example.com', '*.example.com']) } it 'accepts info@example.com' do user = build(:user, email: "info@example.com") expect(user).to be_valid @@ -93,7 +93,7 @@ describe User, models: true do end context 'when a signup domain is listed and subdomains are not allowed' do - before { allow(current_application_settings).to receive(:restricted_signup_domains).and_return(['example.com']) } + before { allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return(['example.com']) } it 'accepts info@example.com' do user = build(:user, email: "info@example.com") @@ -141,7 +141,7 @@ describe User, models: true do end describe '#confirm' do - before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(true) } + before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) } let(:user) { create(:user, confirmed_at: nil, unconfirmed_email: 'test@gitlab.com') } it 'returns unconfirmed' do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 40b24c125b5..a7690f430c4 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -20,7 +20,7 @@ describe API::API, api: true do end context "when authenticated" do - #These specs are written just in case API authentication is not required anymore + # These specs are written just in case API authentication is not required anymore context "when public level is restricted" do before do stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb index a97ac3ead45..71a0b8e2a12 100644 --- a/spec/services/groups/create_service_spec.rb +++ b/spec/services/groups/create_service_spec.rb @@ -13,7 +13,7 @@ describe Groups::CreateService, services: true do end context "cannot create group with restricted visibility level" do - before { allow(current_application_settings).to receive(:restricted_visibility_levels).and_return([Gitlab::VisibilityLevel::PUBLIC]) } + before { allow_any_instance_of(ApplicationSetting).to receive(:restricted_visibility_levels).and_return([Gitlab::VisibilityLevel::PUBLIC]) } it { is_expected.not_to be_persisted } end end diff --git a/spec/support/filter_spec_helper.rb b/spec/support/filter_spec_helper.rb index e849a9633b9..a8e454eb09e 100644 --- a/spec/support/filter_spec_helper.rb +++ b/spec/support/filter_spec_helper.rb @@ -40,8 +40,7 @@ module FilterSpecHelper filters = [ Banzai::Filter::AutolinkFilter, - described_class, - Banzai::Filter::ReferenceGathererFilter + described_class ] HTML::Pipeline.new(filters, context) diff --git a/spec/support/login_helpers.rb b/spec/support/login_helpers.rb index cd9fdc6f18e..7a0f078c72b 100644 --- a/spec/support/login_helpers.rb +++ b/spec/support/login_helpers.rb @@ -26,11 +26,13 @@ module LoginHelpers # Internal: Login as the specified user # - # user - User instance to login with - def login_with(user) + # user - User instance to login with + # remember - Whether or not to check "Remember me" (default: false) + def login_with(user, remember: false) visit new_user_session_path fill_in "user_login", with: user.email fill_in "user_password", with: "12345678" + check 'user_remember_me' if remember click_button "Sign in" Thread.current[:current_user] = user end diff --git a/spec/support/reference_parser_helpers.rb b/spec/support/reference_parser_helpers.rb new file mode 100644 index 00000000000..01689194eac --- /dev/null +++ b/spec/support/reference_parser_helpers.rb @@ -0,0 +1,5 @@ +module ReferenceParserHelpers + def empty_html_link + Nokogiri::HTML.fragment('<a></a>').children[0] + end +end diff --git a/spec/teaspoon_env.rb b/spec/teaspoon_env.rb index 58f45ff8610..69b2b9b6d5b 100644 --- a/spec/teaspoon_env.rb +++ b/spec/teaspoon_env.rb @@ -41,11 +41,11 @@ Teaspoon.configure do |config| suite.matcher = "{spec/javascripts,app/assets}/**/*_spec.{js,js.coffee,coffee}" # Load additional JS files, but requiring them in your spec helper is the preferred way to do this. - #suite.javascripts = [] + # suite.javascripts = [] # You can include your own stylesheets if you want to change how Teaspoon looks. # Note: Spec related CSS can and should be loaded using fixtures. - #suite.stylesheets = ["teaspoon"] + # suite.stylesheets = ["teaspoon"] # This suites spec helper, which can require additional support files. This file is loaded before any of your test # files are loaded. @@ -62,19 +62,19 @@ Teaspoon.configure do |config| # Hooks allow you to use `Teaspoon.hook("fixtures")` before, after, or during your spec run. This will make a # synchronous Ajax request to the server that will call all of the blocks you've defined for that hook name. - #suite.hook :fixtures, &proc{} + # suite.hook :fixtures, &proc{} # Determine whether specs loaded into the test harness should be embedded as individual script tags or concatenated - # into a single file. Similar to Rails' asset `debug: true` and `config.assets.debug = true` options. By default, + # into a single file. Similar to Rails' asset `debug: true` and `config.assets.debug = true` options. By default, # Teaspoon expands all assets to provide more valuable stack traces that reference individual source files. - #suite.expand_assets = true + # suite.expand_assets = true end # Example suite. Since we're just filtering to files already within the root test/javascripts, these files will also # be run in the default suite -- but can be focused into a more specific suite. - #config.suite :targeted do |suite| + # config.suite :targeted do |suite| # suite.matcher = "spec/javascripts/targeted/*_spec.{js,js.coffee,coffee}" - #end + # end # CONSOLE RUNNER SPECIFIC # @@ -94,45 +94,45 @@ Teaspoon.configure do |config| # PhantomJS: https://github.com/modeset/teaspoon/wiki/Using-PhantomJS # Selenium Webdriver: https://github.com/modeset/teaspoon/wiki/Using-Selenium-WebDriver # Capybara Webkit: https://github.com/modeset/teaspoon/wiki/Using-Capybara-Webkit - #config.driver = :phantomjs + # config.driver = :phantomjs # Specify additional options for the driver. # # PhantomJS: https://github.com/modeset/teaspoon/wiki/Using-PhantomJS # Selenium Webdriver: https://github.com/modeset/teaspoon/wiki/Using-Selenium-WebDriver # Capybara Webkit: https://github.com/modeset/teaspoon/wiki/Using-Capybara-Webkit - #config.driver_options = nil + # config.driver_options = nil # Specify the timeout for the driver. Specs are expected to complete within this time frame or the run will be # considered a failure. This is to avoid issues that can arise where tests stall. - #config.driver_timeout = 180 + # config.driver_timeout = 180 # Specify a server to use with Rack (e.g. thin, mongrel). If nil is provided Rack::Server is used. - #config.server = nil + # config.server = nil # Specify a port to run on a specific port, otherwise Teaspoon will use a random available port. - #config.server_port = nil + # config.server_port = nil # Timeout for starting the server in seconds. If your server is slow to start you may have to bump this, or you may # want to lower this if you know it shouldn't take long to start. - #config.server_timeout = 20 + # config.server_timeout = 20 # Force Teaspoon to fail immediately after a failing suite. Can be useful to make Teaspoon fail early if you have # several suites, but in environments like CI this may not be desirable. - #config.fail_fast = true + # config.fail_fast = true # Specify the formatters to use when outputting the results. # Note: Output files can be specified by using `"junit>/path/to/output.xml"`. # # Available: :dot, :clean, :documentation, :json, :junit, :pride, :rspec_html, :snowday, :swayze_or_oprah, :tap, :tap_y, :teamcity - #config.formatters = [:dot] + # config.formatters = [:dot] # Specify if you want color output from the formatters. - #config.color = true + # config.color = true # Teaspoon pipes all console[log/debug/error] to $stdout. This is useful to catch places where you've forgotten to # remove them, but in verbose applications this may not be desirable. - #config.suppress_log = false + # config.suppress_log = false # COVERAGE REPORTS / THRESHOLD ASSERTIONS # @@ -149,7 +149,7 @@ Teaspoon.configure do |config| # Specify that you always want a coverage configuration to be used. Otherwise, specify that you want coverage # on the CLI. # Set this to "true" or the name of your coverage config. - #config.use_coverage = nil + # config.use_coverage = nil # You can have multiple coverage configs by passing a name to config.coverage. # e.g. config.coverage :ci do |coverage| @@ -158,21 +158,21 @@ Teaspoon.configure do |config| # Which coverage reports Istanbul should generate. Correlates directly to what Istanbul supports. # # Available: text-summary, text, html, lcov, lcovonly, cobertura, teamcity - #coverage.reports = ["text-summary", "html"] + # coverage.reports = ["text-summary", "html"] # The path that the coverage should be written to - when there's an artifact to write to disk. # Note: Relative to `config.root`. - #coverage.output_path = "coverage" + # coverage.output_path = "coverage" # Assets to be ignored when generating coverage reports. Accepts an array of filenames or regular expressions. The # default excludes assets from vendor, gems and support libraries. - #coverage.ignore = [%r{/lib/ruby/gems/}, %r{/vendor/assets/}, %r{/support/}, %r{/(.+)_helper.}] + # coverage.ignore = [%r{/lib/ruby/gems/}, %r{/vendor/assets/}, %r{/support/}, %r{/(.+)_helper.}] # Various thresholds requirements can be defined, and those thresholds will be checked at the end of a run. If any # aren't met the run will fail with a message. Thresholds can be defined as a percentage (0-100), or nil. - #coverage.statements = nil - #coverage.functions = nil - #coverage.branches = nil - #coverage.lines = nil + # coverage.statements = nil + # coverage.functions = nil + # coverage.branches = nil + # coverage.lines = nil end end |