Merge branch 'master' into 4009-external-users

CHANGELOG

@@ -31,12 +31,14 @@ v 8.6.0 (unreleased)
   - Add ability to show archived projects on dashboard, explore and group pages
   - Move group activity to separate page
   - Create external users which are excluded of internal and private projects unless access was explicitly granted
+  - Continue parameters are checked to ensure redirection goes to the same instance
 v 8.5.5
   - Ensure removing a project removes associated Todo entries
   - Prevent a 500 error in Todos when author was removed
   - Fix pagination for filtered dashboard and explore pages
   - Fix "Show all" link behavior
+  - Add #upcoming filter to Milestone filter (Tiago Botelho)
 v 8.5.4
   - Do not cache requests for badges (including builds badge)

GITLAB_SHELL_VERSION

-2.6.10
+2.6.11

app/assets/javascripts/dispatcher.js.coffee

@@ -104,6 +104,8 @@ class Dispatcher
         new ProjectFork()
       when 'projects:artifacts:browse'
         new BuildArtifacts()
+      when 'projects:group_links:index'
+        new GroupsSelect()
     switch path.first()
       when 'admin'

app/assets/javascripts/gl_dropdown.js.coffee

@@ -238,13 +238,15 @@ class GitLabDropdown
         selectedObject = @renderedData[selectedIndex]
       value = if @options.id then @options.id(selectedObject, el) else selectedObject.id
+      if !value?
+        field.remove()
+
       if @options.multiSelect
         oldValue = field.val()
         if oldValue
           value = "#{oldValue},#{value}"
       else
         @dropdown.find(ACTIVE_CLASS).removeClass ACTIVE_CLASS
-        field.remove()
       # Toggle active class for the tick mark
       el.toggleClass "is-active"

app/controllers/concerns/continue_params.rb

+module ContinueParams
+  extend ActiveSupport::Concern
+
+  def continue_params
+    continue_params = params[:continue]
+    return nil unless continue_params
+
+    continue_params = continue_params.permit(:to, :notice, :notice_now)
+    return unless continue_params[:to] && continue_params[:to].start_with?('/')
+
+    continue_params
+  end
+end

app/controllers/groups_controller.rb

@@ -46,6 +46,8 @@ class GroupsController < Groups::ApplicationController
     @projects = @projects.sort(@sort = params[:sort])
     @projects = @projects.page(params[:page]).per(PER_PAGE) if params[:filter_projects].blank?
+    @shared_projects = @group.shared_projects
+
     respond_to do |format|
       format.html
@@ -133,7 +135,7 @@ class GroupsController < Groups::ApplicationController
   end
   def group_params
-    params.require(:group).permit(:name, :description, :path, :avatar, :public)
+    params.require(:group).permit(:name, :description, :path, :avatar, :public, :share_with_group_lock)
   end
   def load_events

app/controllers/projects/forks_controller.rb

 class Projects::ForksController < Projects::ApplicationController
+  include ContinueParams
+
   # Authorize
   before_action :require_non_empty_project
   before_action :authorize_download_code!
@@ -53,15 +55,4 @@ class Projects::ForksController < Projects::ApplicationController
       render :error
     end
   end
-
-  private
-
-  def continue_params
-    continue_params = params[:continue]
-    if continue_params
-      continue_params.permit(:to, :notice, :notice_now)
-    else
-      nil
-    end
-  end
 end

app/controllers/projects/group_links_controller.rb

+class Projects::GroupLinksController < Projects::ApplicationController
+  layout 'project_settings'
+  before_action :authorize_admin_project!
+
+  def index
+    @group_links = project.project_group_links.all
+  end
+
+  def create
+    link = project.project_group_links.new
+    link.group_id = params[:link_group_id]
+    link.group_access = params[:link_group_access]
+    link.save
+
+    redirect_to namespace_project_group_links_path(project.namespace, project)
+  end
+
+  def destroy
+    project.project_group_links.find(params[:id]).destroy
+
+    redirect_to namespace_project_group_links_path(project.namespace, project)
+  end
+end

app/controllers/projects/imports_controller.rb

 class Projects::ImportsController < Projects::ApplicationController
+  include ContinueParams
+
   # Authorize
   before_action :authorize_admin_project!
   before_action :require_no_repo, only: [:new, :create]
@@ -44,16 +46,6 @@ class Projects::ImportsController < Projects::ApplicationController
   private
-  def continue_params
-    continue_params = params[:continue]
-
-    if continue_params
-      continue_params.permit(:to, :notice, :notice_now)
-    else
-      nil
-    end
-  end
-
   def finished_notice
     if @project.forked?
       'The project was successfully forked.'

app/controllers/projects/project_members_controller.rb

@@ -27,6 +27,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
     end
     @project_member = @project.project_members.new
+    @project_group_links = @project.project_group_links
   end
   def create

app/finders/issuable_finder.rb

@@ -244,10 +244,17 @@ class IssuableFinder
     items
   end
+  def filter_by_upcoming_milestone?
+    params[:milestone_title] == '#upcoming'
+  end
+
   def by_milestone(items)
     if milestones?
       if filter_by_no_milestone?
         items = items.where(milestone_id: [-1, nil])
+      elsif filter_by_upcoming_milestone?
+        upcoming = Milestone.where(project_id: projects).upcoming
+        items = items.joins(:milestone).where(milestones: { title: upcoming.title })
       else
         items = items.joins(:milestone).where(milestones: { title: params[:milestone_title] })

app/finders/projects_finder.rb

@@ -42,16 +42,14 @@ class ProjectsFinder
   def group_projects(current_user, group)
     return [group.projects.public_only] unless current_user
+    user_group_projects = [
+       group_projects_for_user(current_user, group),
+       group.shared_projects.visible_to_user(current_user)
+    ]
     if current_user.external?
-      [
-        group_projects_for_user(current_user, group),
-        group.projects.public_only
-      ]
+      user_group_projects.push(group.projects.public_only)
     else
-      [
-        group_projects_for_user(current_user, group),
-        group.projects.public_and_internal_only
-      ]
+      user_group_projects.push(group.projects.public_and_internal_only)
     end
   end

app/helpers/milestones_helper.rb

@@ -59,6 +59,7 @@ module MilestonesHelper
     grouped_milestones = grouped_milestones.sort_by { |x| x.due_date.nil? ? epoch : x.due_date }
     grouped_milestones.unshift(Milestone::None)
     grouped_milestones.unshift(Milestone::Any)
+    grouped_milestones.unshift(Milestone::Upcoming)
     options_from_collection_for_select(grouped_milestones, 'name', 'title', params[:milestone_title])
   end

app/models/group.rb

@@ -23,6 +23,8 @@ class Group < Namespace
   has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember'
   alias_method :members, :group_members
   has_many :users, through: :group_members
+  has_many :project_group_links, dependent: :destroy
+  has_many :shared_projects, through: :project_group_links, source: :project
   validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? }
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }

app/models/milestone.rb

@@ -19,6 +19,7 @@ class Milestone < ActiveRecord::Base
   MilestoneStruct = Struct.new(:title, :name, :id)
   None = MilestoneStruct.new('No Milestone', 'No Milestone', 0)
   Any = MilestoneStruct.new('Any Milestone', '', -1)
+  Upcoming = MilestoneStruct.new('Upcoming', '#upcoming', -2)
   include InternalId
   include Sortable
@@ -81,6 +82,10 @@ class Milestone < ActiveRecord::Base
     super("milestones", /(?<milestone>\d+)/)
   end
+  def self.upcoming
+    self.where('due_date > ?', Time.now).order(due_date: :asc).first
+  end
+
   def to_reference(from_project = nil)
     escaped_title = self.title.gsub("]", "\\]")

app/models/note.rb

@@ -173,26 +173,29 @@ class Note < ActiveRecord::Base
     Note.where(noteable_id: noteable_id, noteable_type: noteable_type, line_code: line_code).last.try(:diff)
   end
-  # Check if such line of code exists in merge request diff
-  # If exists - its active discussion
-  # If not - its outdated diff
+  # Check if this note is part of an "active" discussion
+  #
+  # This will always return true for anything except MergeRequest noteables,
+  # which have special logic.
+  #
+  # If the note's current diff cannot be matched in the MergeRequest's current
+  # diff, it's considered inactive.
   def active?
     return true unless self.diff
     return false unless noteable
     return @active if defined?(@active)
-    diffs = noteable.diffs(Commit.max_diff_options)
-    notable_diff = diffs.find { |d| d.new_path == self.diff.new_path }
-    return @active = false if notable_diff.nil?
-    parsed_lines = Gitlab::Diff::Parser.new.parse(notable_diff.diff.each_line)
-    # We cannot use ||= because @active may be false
-    @active = parsed_lines.any? { |line_obj| line_obj.text == diff_line }
-  end
-  def outdated?
-    !active?
+    noteable_diff = find_noteable_diff
+    if noteable_diff
+      parsed_lines = Gitlab::Diff::Parser.new.parse(noteable_diff.diff.each_line)
+      @active = parsed_lines.any? { |line_obj| line_obj.text == diff_line }
+    else
+      @active = false
+    end
+    @active
   end
   def diff_file_index
@@ -380,6 +383,12 @@ class Note < ActiveRecord::Base
     self.line_code = nil if self.line_code.blank?
   end
+  # Find the diff on noteable that matches our own
+  def find_noteable_diff
+    diffs = noteable.diffs(Commit.max_diff_options)
+    diffs.find { |d| d.new_path == self.diff.new_path }
+  end
+
   def awards_supported?
     (for_issue? || for_merge_request?) && !for_diff_line?
   end

app/models/project.rb

@@ -151,6 +151,8 @@ class Project < ActiveRecord::Base
   has_many :releases, dependent: :destroy
   has_many :lfs_objects_projects, dependent: :destroy
   has_many :lfs_objects, through: :lfs_objects_projects
+  has_many :project_group_links, dependent: :destroy
+  has_many :invited_groups, through: :project_group_links, source: :group
   has_many :todos, dependent: :destroy
   has_one :import_data, dependent: :destroy, class_name: "ProjectImportData"
@@ -893,6 +895,10 @@ class Project < ActiveRecord::Base
     jira_tracker? && jira_service.active
   end
+  def allowed_to_share_with_group?
+    !namespace.share_with_group_lock
+  end
+
   def ci_commit(sha)
     ci_commits.find_by(sha: sha)
   end

app/models/project_group_link.rb

+class ProjectGroupLink < ActiveRecord::Base
+  GUEST     = 10
+  REPORTER  = 20
+  DEVELOPER = 30
+  MASTER    = 40
+
+  belongs_to :project
+  belongs_to :group
+
+  validates :project_id, presence: true
+  validates :group_id, presence: true
+  validates :group_id, uniqueness: { scope: [:project_id], message: "already shared with this group" }
+  validates :group_access, presence: true
+  validates :group_access, inclusion: { in: Gitlab::Access.values }, presence: true
+  validate :different_group
+
+  def self.access_options
+    Gitlab::Access.options
+  end
+
+  def self.default_access
+    DEVELOPER
+  end
+
+  def human_access
+    self.class.access_options.key(self.group_access)
+  end
+
+  private 
+
+  def different_group
+    if self.group && self.project && self.project.group == self.group
+      errors.add(:base, "Project cannot be shared with the project it is in.")
+    end
+  end
+end

app/models/project_team.rb

@@ -160,7 +160,27 @@ class ProjectTeam
       end
     end
-    access.max
+    if project.invited_groups.any? && project.allowed_to_share_with_group?
+      access << max_invited_level(user_id)
+    end
+
+    access.compact.max
+  end
+
+
+  def max_invited_level(user_id)
+    project.project_group_links.map do |group_link|
+      invited_group = group_link.group
+      access = invited_group.group_members.find_by(user_id: user_id).try(:access_field)
+
+      # If group member has higher access level we should restrict it
+      # to max allowed access level
+      if access && access > group_link.group_access
+        access = group_link.group_access
+      end
+
+      access
+    end.compact.max
   end
   private
@@ -168,6 +188,35 @@ class ProjectTeam
   def fetch_members(level = nil)
     project_members = project.project_members
     group_members = group ? group.group_members : []
+    invited_members = []
+
+    if project.invited_groups.any? && project.allowed_to_share_with_group?
+      project.project_group_links.each do |group_link|
+        invited_group = group_link.group
+        im = invited_group.group_members
+
+        if level
+          int_level = GroupMember.access_level_roles[level.to_s.singularize.titleize]
+
+          # Skip group members if we ask for masters
+          # but max group access is developers
+          next if int_level > group_link.group_access
+
+          # If we ask for developers and max
+          # group access is developers we need to provide
+          # both group master, developers as devs
+          if int_level == group_link.group_access
+            im.where("access_level >= ?)", group_link.group_access)
+          else
+            im.send(level)
+          end
+        end
+
+        invited_members << im
+      end
+
+      invited_members = invited_members.flatten.compact
+    end
     if level
       project_members = project_members.send(level)
@@ -175,6 +224,7 @@ class ProjectTeam
     end
     user_ids = project_members.pluck(:user_id)
+    user_ids.push(*invited_members.map(&:user_id)) if invited_members.any?
     user_ids.push(*group_members.pluck(:user_id)) if group
     User.where(id: user_ids)

app/models/user.rb

@@ -838,7 +838,8 @@ class User < ActiveRecord::Base
   def projects_union
     Gitlab::SQL::Union.new([personal_projects.select(:id),
                             groups_projects.select(:id),
-                            projects.select(:id)])
+                            projects.select(:id),
+                            groups.joins(:shared_projects).select(:project_id)])
   end
   def ci_projects_union