Commit a105101c4566479baa00ef98b6bf58f8a5516b12
Exists in
spb-stable
and in
2 other branches
Merge branch 'improve-projects-collecting' into 'master'
Improve projects collecting
Refactor code that cause unexpected AR behaviour.
For example this works fine:
@projects = Project.personal(@user).accessible_to(current_user)
@groups = @user.groups.accessible_to(current_user) # OK
But
@projects = Project.all
@groups = @user.groups.accessible_to(current_user) # PG:Error
Showing
11 changed files
with
91 additions
and
32 deletions
Show diff stats
app/controllers/users_controller.rb
| ... | ... | @@ -4,15 +4,24 @@ class UsersController < ApplicationController |
| 4 | 4 | |
| 5 | 5 | def show |
| 6 | 6 | @user = User.find_by_username!(params[:username]) |
| 7 | - @projects = Project.personal(@user).accessible_to(current_user) | |
| 8 | 7 | |
| 9 | 8 | unless current_user || @user.public_profile? |
| 10 | 9 | return authenticate_user! |
| 11 | 10 | end |
| 12 | 11 | |
| 13 | - @groups = @user.groups.accessible_to(current_user) | |
| 14 | - accessible_projects = @user.authorized_projects.accessible_to(current_user) | |
| 15 | - @events = @user.recent_events.where(project_id: accessible_projects.pluck(:id)).limit(20) | |
| 12 | + # Projects user can view | |
| 13 | + authorized_projects_ids = ProjectsFinder.new.execute(current_user).pluck(:id) | |
| 14 | + | |
| 15 | + @projects = @user.personal_projects. | |
| 16 | + where(id: authorized_projects_ids) | |
| 17 | + | |
| 18 | + # Collect only groups common for both users | |
| 19 | + @groups = @user.groups & GroupsFinder.new.execute(current_user) | |
| 20 | + | |
| 21 | + # Get user activity feed for projects common for both users | |
| 22 | + @events = @user.recent_events. | |
| 23 | + where(project_id: authorized_projects_ids).limit(20) | |
| 24 | + | |
| 16 | 25 | @title = @user.name |
| 17 | 26 | end |
| 18 | 27 | ... | ... |
app/finders/base_finder.rb
| ... | ... | @@ -49,7 +49,7 @@ class BaseFinder |
| 49 | 49 | elsif current_user && params[:authorized_only].presence |
| 50 | 50 | klass.of_projects(current_user.authorized_projects).references(:project) |
| 51 | 51 | else |
| 52 | - klass.of_projects(Project.accessible_to(current_user)).references(:project) | |
| 52 | + klass.of_projects(ProjectsFinder.new.execute(current_user)).references(:project) | |
| 53 | 53 | end |
| 54 | 54 | end |
| 55 | 55 | ... | ... |
| ... | ... | @@ -0,0 +1,38 @@ |
| 1 | +class GroupsFinder | |
| 2 | + def execute(current_user, options = {}) | |
| 3 | + all_groups(current_user) | |
| 4 | + end | |
| 5 | + | |
| 6 | + private | |
| 7 | + | |
| 8 | + def all_groups(current_user) | |
| 9 | + if current_user | |
| 10 | + if current_user.authorized_groups.any? | |
| 11 | + # User has access to groups | |
| 12 | + # | |
| 13 | + # Return only: | |
| 14 | + # groups with public projects | |
| 15 | + # groups with internal projects | |
| 16 | + # groups with joined projects | |
| 17 | + # | |
| 18 | + group_ids = Project.public_and_internal_only.pluck(:namespace_id) + | |
| 19 | + current_user.authorized_groups.pluck(:id) | |
| 20 | + Group.where(id: group_ids) | |
| 21 | + else | |
| 22 | + # User has no group membership | |
| 23 | + # | |
| 24 | + # Return only: | |
| 25 | + # groups with public projects | |
| 26 | + # groups with internal projects | |
| 27 | + # | |
| 28 | + Group.where(id: Project.public_and_internal_only.pluck(:namespace_id)) | |
| 29 | + end | |
| 30 | + else | |
| 31 | + # Not authenticated | |
| 32 | + # | |
| 33 | + # Return only: | |
| 34 | + # groups with public projects | |
| 35 | + Group.where(id: Project.public_only.pluck(:namespace_id)) | |
| 36 | + end | |
| 37 | + end | |
| 38 | +end | ... | ... |
app/finders/projects_finder.rb
| 1 | 1 | class ProjectsFinder |
| 2 | - def execute(current_user, options) | |
| 2 | + def execute(current_user, options = {}) | |
| 3 | 3 | group = options[:group] |
| 4 | 4 | |
| 5 | 5 | if group |
| ... | ... | @@ -56,8 +56,36 @@ class ProjectsFinder |
| 56 | 56 | end |
| 57 | 57 | end |
| 58 | 58 | |
| 59 | - def all_projects | |
| 60 | - # TODO: implement | |
| 61 | - raise 'Not implemented yet' | |
| 59 | + def all_projects(current_user) | |
| 60 | + if current_user | |
| 61 | + if current_user.authorized_projects.any? | |
| 62 | + # User has access to private projects | |
| 63 | + # | |
| 64 | + # Return only: | |
| 65 | + # public projects | |
| 66 | + # internal projects | |
| 67 | + # joined projects | |
| 68 | + # | |
| 69 | + Project.where( | |
| 70 | + "projects.id IN (?) OR projects.visibility_level IN (?)", | |
| 71 | + current_user.authorized_projects.pluck(:id), | |
| 72 | + Project.public_and_internal_levels | |
| 73 | + ) | |
| 74 | + else | |
| 75 | + # User has no access to private projects | |
| 76 | + # | |
| 77 | + # Return only: | |
| 78 | + # public projects | |
| 79 | + # internal projects | |
| 80 | + # | |
| 81 | + Project.public_and_internal_only | |
| 82 | + end | |
| 83 | + else | |
| 84 | + # Not authenticated | |
| 85 | + # | |
| 86 | + # Return only: | |
| 87 | + # public projects | |
| 88 | + Project.public_only | |
| 89 | + end | |
| 62 | 90 | end |
| 63 | 91 | end | ... | ... |
app/helpers/search_helper.rb
| ... | ... | @@ -81,7 +81,7 @@ module SearchHelper |
| 81 | 81 | |
| 82 | 82 | # Autocomplete results for the current user's projects |
| 83 | 83 | def projects_autocomplete(term, limit = 5) |
| 84 | - Project.accessible_to(current_user).search_by_title(term).non_archived.limit(limit).map do |p| | |
| 84 | + ProjectsFinder.new.execute(current_user).search_by_title(term).non_archived.limit(limit).map do |p| | |
| 85 | 85 | { |
| 86 | 86 | label: "project: #{search_result_sanitize(p.name_with_namespace)}", |
| 87 | 87 | url: project_path(p) | ... | ... |
app/models/ability.rb
app/models/group.rb
| ... | ... | @@ -27,12 +27,6 @@ class Group < Namespace |
| 27 | 27 | |
| 28 | 28 | mount_uploader :avatar, AttachmentUploader |
| 29 | 29 | |
| 30 | - def self.accessible_to(user) | |
| 31 | - accessible_ids = Project.accessible_to(user).pluck(:namespace_id) | |
| 32 | - accessible_ids += user.groups.pluck(:id) if user | |
| 33 | - where(id: accessible_ids) | |
| 34 | - end | |
| 35 | - | |
| 36 | 30 | def human_name |
| 37 | 31 | name |
| 38 | 32 | end |
| ... | ... | @@ -77,4 +71,8 @@ class Group < Namespace |
| 77 | 71 | self.errors.add :avatar, "only images allowed" |
| 78 | 72 | end |
| 79 | 73 | end |
| 74 | + | |
| 75 | + def public_profile? | |
| 76 | + projects.public_only.any? | |
| 77 | + end | |
| 80 | 78 | end | ... | ... |
app/models/namespace.rb
| ... | ... | @@ -47,14 +47,6 @@ class Namespace < ActiveRecord::Base |
| 47 | 47 | def self.global_id |
| 48 | 48 | 'GLN' |
| 49 | 49 | end |
| 50 | - | |
| 51 | - def projects_accessible_to(user) | |
| 52 | - projects.accessible_to(user) | |
| 53 | - end | |
| 54 | - | |
| 55 | - def has_projects_accessible_to?(user) | |
| 56 | - projects_accessible_to(user).present? | |
| 57 | - end | |
| 58 | 50 | |
| 59 | 51 | def to_param |
| 60 | 52 | path | ... | ... |
app/models/project.rb
| ... | ... | @@ -164,12 +164,6 @@ class Project < ActiveRecord::Base |
| 164 | 164 | where(visibility_level: visibility_levels) |
| 165 | 165 | end |
| 166 | 166 | |
| 167 | - def accessible_to(user) | |
| 168 | - accessible_ids = publicish(user).pluck(:id) | |
| 169 | - accessible_ids += user.authorized_projects.pluck(:id) if user | |
| 170 | - where(id: accessible_ids) | |
| 171 | - end | |
| 172 | - | |
| 173 | 167 | def with_push |
| 174 | 168 | includes(:events).where('events.action = ?', Event::PUSHED) |
| 175 | 169 | end | ... | ... |
app/services/search/global_service.rb
| ... | ... | @@ -12,7 +12,7 @@ module Search |
| 12 | 12 | return result unless query.present? |
| 13 | 13 | |
| 14 | 14 | group = Group.find_by(id: params[:group_id]) if params[:group_id].present? |
| 15 | - projects = Project.accessible_to(current_user) | |
| 15 | + projects = ProjectsFinder.new.execute(current_user) | |
| 16 | 16 | projects = projects.where(namespace_id: group.id) if group |
| 17 | 17 | project_ids = projects.pluck(:id) |
| 18 | 18 | ... | ... |