Commit 75eed4eb834919a8cb5a47b8a05335fd2e74f99e
1 parent
9f20580e
Exists in
spb-stable
and in
3 other branches
Implement project collection service
Main purpose is move big amount of methods from user, group, project models and place filtering logic in one place. It also fixes 500 error on group page for PostgreSQL Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Showing
5 changed files
with
132 additions
and
5 deletions
Show diff stats
app/controllers/groups_controller.rb
| ... | ... | @@ -99,7 +99,7 @@ class GroupsController < ApplicationController |
| 99 | 99 | end |
| 100 | 100 | |
| 101 | 101 | def projects |
| 102 | - @projects ||= group.projects_accessible_to(current_user).sorted_by_activity | |
| 102 | + @projects ||= Projects::CollectService.new.execute(current_user, group: group) | |
| 103 | 103 | end |
| 104 | 104 | |
| 105 | 105 | def project_ids | ... | ... |
app/models/ability.rb
| ... | ... | @@ -50,7 +50,7 @@ class Ability |
| 50 | 50 | else |
| 51 | 51 | nil |
| 52 | 52 | end |
| 53 | - | |
| 53 | + | |
| 54 | 54 | if group && group.has_projects_accessible_to?(nil) |
| 55 | 55 | [:read_group] |
| 56 | 56 | else |
| ... | ... | @@ -184,7 +184,7 @@ class Ability |
| 184 | 184 | def group_abilities user, group |
| 185 | 185 | rules = [] |
| 186 | 186 | |
| 187 | - if user.admin? || group.users.include?(user) || group.has_projects_accessible_to?(user) | |
| 187 | + if user.admin? || Projects::CollectService.new.execute(user, group: group).any? | |
| 188 | 188 | rules << :read_group |
| 189 | 189 | end |
| 190 | 190 | ... | ... |
app/models/project.rb
| ... | ... | @@ -116,21 +116,28 @@ class Project < ActiveRecord::Base |
| 116 | 116 | scope :personal, ->(user) { where(namespace_id: user.namespace_id) } |
| 117 | 117 | scope :joined, ->(user) { where("namespace_id != ?", user.namespace_id) } |
| 118 | 118 | |
| 119 | + scope :public_only, -> { where(visibility_level: Project::PUBLIC) } | |
| 120 | + scope :public_and_internal_only, -> { where(visibility_level: Project.public_and_internal_levels) } | |
| 121 | + | |
| 119 | 122 | scope :non_archived, -> { where(archived: false) } |
| 120 | 123 | |
| 121 | 124 | enumerize :issues_tracker, in: (Gitlab.config.issues_tracker.keys).append(:gitlab), default: :gitlab |
| 122 | 125 | |
| 123 | 126 | class << self |
| 127 | + def public_and_internal_levels | |
| 128 | + [Project::PUBLIC, Project::INTERNAL] | |
| 129 | + end | |
| 130 | + | |
| 124 | 131 | def abandoned |
| 125 | 132 | where('projects.last_activity_at < ?', 6.months.ago) |
| 126 | 133 | end |
| 127 | - | |
| 134 | + | |
| 128 | 135 | def publicish(user) |
| 129 | 136 | visibility_levels = [Project::PUBLIC] |
| 130 | 137 | visibility_levels += [Project::INTERNAL] if user |
| 131 | 138 | where(visibility_level: visibility_levels) |
| 132 | 139 | end |
| 133 | - | |
| 140 | + | |
| 134 | 141 | def accessible_to(user) |
| 135 | 142 | accessible_ids = publicish(user).pluck(:id) |
| 136 | 143 | accessible_ids += user.authorized_projects.pluck(:id) if user | ... | ... |
| ... | ... | @@ -0,0 +1,69 @@ |
| 1 | +# Projects::CollectService class | |
| 2 | +# | |
| 3 | +# Used to collect projects user has access to | |
| 4 | +# | |
| 5 | +module Projects | |
| 6 | + class CollectService | |
| 7 | + def execute(current_user, options) | |
| 8 | + group = options[:group] | |
| 9 | + | |
| 10 | + if group | |
| 11 | + group_projects(current_user, group) | |
| 12 | + else | |
| 13 | + all_projects(current_user) | |
| 14 | + end | |
| 15 | + end | |
| 16 | + | |
| 17 | + private | |
| 18 | + | |
| 19 | + def group_projects(current_user, group) | |
| 20 | + if current_user | |
| 21 | + if group.users.include?(current_user) | |
| 22 | + # User is group member | |
| 23 | + # | |
| 24 | + # Return ALL group projects | |
| 25 | + group.projects | |
| 26 | + else | |
| 27 | + projects_members = UsersProject.where( | |
| 28 | + project_id: group.projects, | |
| 29 | + user_id: current_user | |
| 30 | + ) | |
| 31 | + | |
| 32 | + if projects_members.any? | |
| 33 | + # User is a project member | |
| 34 | + # | |
| 35 | + # Return only: | |
| 36 | + # public projects | |
| 37 | + # internal projects | |
| 38 | + # joined projects | |
| 39 | + # | |
| 40 | + group.projects.where( | |
| 41 | + "projects.id IN (?) OR projects.visibility_level IN (?)", | |
| 42 | + projects_members.pluck(:project_id), | |
| 43 | + Project.public_and_internal_levels | |
| 44 | + ) | |
| 45 | + else | |
| 46 | + # User has no access to group or group projects | |
| 47 | + # | |
| 48 | + # Return only: | |
| 49 | + # public projects | |
| 50 | + # internal projects | |
| 51 | + # | |
| 52 | + group.projects.public_and_internal_only | |
| 53 | + end | |
| 54 | + end | |
| 55 | + else | |
| 56 | + # Not authenticated | |
| 57 | + # | |
| 58 | + # Return only: | |
| 59 | + # public projects | |
| 60 | + group.projects.public_only | |
| 61 | + end | |
| 62 | + end | |
| 63 | + end | |
| 64 | + | |
| 65 | + def all_projects | |
| 66 | + # TODO: implement | |
| 67 | + raise 'Not implemented yet' | |
| 68 | + end | |
| 69 | +end | ... | ... |
| ... | ... | @@ -0,0 +1,51 @@ |
| 1 | +require 'spec_helper' | |
| 2 | + | |
| 3 | +describe Projects::CollectService do | |
| 4 | + let(:user) { create :user } | |
| 5 | + let(:group) { create :group } | |
| 6 | + | |
| 7 | + let(:project1) { create(:empty_project, group: group, visibility_level: Project::PUBLIC) } | |
| 8 | + let(:project2) { create(:empty_project, group: group, visibility_level: Project::INTERNAL) } | |
| 9 | + let(:project3) { create(:empty_project, group: group, visibility_level: Project::PRIVATE) } | |
| 10 | + let(:project4) { create(:empty_project, group: group, visibility_level: Project::PRIVATE) } | |
| 11 | + | |
| 12 | + context 'non authenticated' do | |
| 13 | + subject { Projects::CollectService.new.execute(nil, group: group) } | |
| 14 | + | |
| 15 | + it { should include(project1) } | |
| 16 | + it { should_not include(project2) } | |
| 17 | + it { should_not include(project3) } | |
| 18 | + it { should_not include(project4) } | |
| 19 | + end | |
| 20 | + | |
| 21 | + context 'authenticated' do | |
| 22 | + subject { Projects::CollectService.new.execute(user, group: group) } | |
| 23 | + | |
| 24 | + it { should include(project1) } | |
| 25 | + it { should include(project2) } | |
| 26 | + it { should_not include(project3) } | |
| 27 | + it { should_not include(project4) } | |
| 28 | + end | |
| 29 | + | |
| 30 | + context 'authenticated, project member' do | |
| 31 | + before { project3.team << [user, :developer] } | |
| 32 | + | |
| 33 | + subject { Projects::CollectService.new.execute(user, group: group) } | |
| 34 | + | |
| 35 | + it { should include(project1) } | |
| 36 | + it { should include(project2) } | |
| 37 | + it { should include(project3) } | |
| 38 | + it { should_not include(project4) } | |
| 39 | + end | |
| 40 | + | |
| 41 | + context 'authenticated, group member' do | |
| 42 | + before { group.add_user(user, Gitlab::Access::DEVELOPER) } | |
| 43 | + | |
| 44 | + subject { Projects::CollectService.new.execute(user, group: group) } | |
| 45 | + | |
| 46 | + it { should include(project1) } | |
| 47 | + it { should include(project2) } | |
| 48 | + it { should include(project3) } | |
| 49 | + it { should include(project4) } | |
| 50 | + end | |
| 51 | +end | ... | ... |