Fix: Missing Projects in Projects Dashboard where user is a member (#295

)

* chore: Update Project Policy default scope to include Projects where a user is a direct member.

* chore: update project policy scope to include projects where the user is a member of a group

* chore: solve finding direct project access differently in project policy

* chore: update tests relating to members after updating seed data

app/controllers/dashboard/projects_controller.rb

@@ -5,14 +5,12 @@ module Dashboard
   class ProjectsController < ApplicationController
     before_action :current_page
 
-    def index # rubocop:disable Metrics/AbcSize
+    def index
       @q = Project.ransack(params[:q])
       set_default_sort
       respond_to do |format|
         format.html do
-          @has_projects = Project.joins(:namespace).exists?(namespace: { parent: current_user.namespace }) ||
-                          Project.joins(:namespace)
-                                 .exists?(namespace: { parent: current_user.groups.self_and_descendant_ids })
+          @has_projects = load_projects(params).count.positive?
         end
         format.turbo_stream do
           @pagy, @projects = pagy(@q.result.where(id: load_projects(params).select(:id))

app/policies/project_policy.rb

@@ -114,9 +114,14 @@ def transfer_sample_into_project?
   end
 
   scope_for :relation do |relation|
-    relation.where(namespace: { parent: user.groups.self_and_descendant_ids })
-            .include_route.or(relation.where(namespace: { parent: user.namespace })
-            .include_route)
+    relation
+      .where(namespace: { parent: user.groups.self_and_descendant_ids }).include_route
+      .or(relation.where(
+        namespace: user.members.joins(:namespace).where(
+          namespace: { type: Namespaces::ProjectNamespace.sti_name }
+        ).select(:namespace_id)
+      ).include_route)
+      .or(relation.where(namespace: { parent: user.namespace }).include_route)
   end
 
   scope_for :relation, :manageable do |relation|

test/controllers/concerns/projects_membership_actions_concern_test.rb

@@ -14,7 +14,6 @@ class ProjectsMembershipActionsConcernTest < ActionDispatch::IntegrationTest
     get namespace_project_members_path(namespace, project)
 
     assert_response :success
-    assert_equal 4, project.namespace.project_members.count
   end
 
   test 'project members index invalid route get' do
@@ -32,7 +31,6 @@ class ProjectsMembershipActionsConcernTest < ActionDispatch::IntegrationTest
     get new_namespace_project_member_path(namespace, project, format: :turbo_stream)
 
     assert_response :success
-    assert_equal 4, project.namespace.project_members.count
   end
 
   test 'project members new invalid route get' do
@@ -52,14 +50,15 @@ class ProjectsMembershipActionsConcernTest < ActionDispatch::IntegrationTest
     user = users(:jane_doe)
     curr_user = users(:john_doe)
 
-    post namespace_project_members_path(namespace, project),
-         params: { member: { user_id: user.id,
-                             namespace_id: proj_namespace,
-                             created_by_id: curr_user.id,
-                             access_level: Member::AccessLevel::OWNER }, format: :turbo_stream }
+    assert_difference -> { project.namespace.project_members.count } => 1 do
+      post namespace_project_members_path(namespace, project),
+           params: { member: { user_id: user.id,
+                               namespace_id: proj_namespace,
+                               created_by_id: curr_user.id,
+                               access_level: Member::AccessLevel::OWNER }, format: :turbo_stream }
+    end
 
     assert_response :success
-    assert_equal 5, project.namespace.project_members.count
   end
 
   test 'project members create invalid route post' do
@@ -82,10 +81,11 @@ class ProjectsMembershipActionsConcernTest < ActionDispatch::IntegrationTest
 
     project_member = members(:project_two_member_james_doe)
 
-    delete namespace_project_member_path(namespace, project, project_member, format: :turbo_stream)
+    assert_difference -> { project.namespace.project_members.count } => -1 do
+      delete namespace_project_member_path(namespace, project, project_member, format: :turbo_stream)
+    end
 
     assert_response :ok
-    assert_equal 3, project.namespace.project_members.count
   end
 
   test 'project members destroy invalid route delete' do
@@ -137,14 +137,15 @@ class ProjectsMembershipActionsConcernTest < ActionDispatch::IntegrationTest
 
     user_new = users(:jane_doe)
 
-    post namespace_project_members_path(namespace, project),
-         params: { member: { user_id: user_new.id,
-                             namespace_id: proj_namespace,
-                             created_by_id: user.id,
-                             access_level: Member::AccessLevel::ANALYST }, format: :turbo_stream }
+    assert_difference -> { project.namespace.project_members.count } => 1 do
+      post namespace_project_members_path(namespace, project),
+           params: { member: { user_id: user_new.id,
+                               namespace_id: proj_namespace,
+                               created_by_id: user.id,
+                               access_level: Member::AccessLevel::ANALYST }, format: :turbo_stream }
+    end
 
     assert_response :success
-    assert_equal 2, project.namespace.project_members.count
   end
 
   test 'update project member access role with current user as owner' do

test/fixtures/members.yml

@@ -116,6 +116,12 @@ project_two_member_joan_doe:
   namespace_id: <%= ActiveRecord::FixtureSet.identify(:john_doe_project2_namespace) %>
   access_level: <%= Member::AccessLevel::MAINTAINER %>
 
+project_two_member_jean_doe:
+  user_id: <%= ActiveRecord::FixtureSet.identify(:jean_doe) %>
+  created_by_id: <%= ActiveRecord::FixtureSet.identify(:john_doe) %>
+  namespace_id: <%= ActiveRecord::FixtureSet.identify(:john_doe_project2_namespace) %>
+  access_level: <%= Member::AccessLevel::MAINTAINER %>
+
 project_two_member_ryan_doe:
   user_id: <%= ActiveRecord::FixtureSet.identify(:ryan_doe) %>
   created_by_id: <%= ActiveRecord::FixtureSet.identify(:john_doe) %>

test/fixtures/namespaces/user_namespaces.yml

@@ -12,6 +12,12 @@ jane_doe_namespace:
   type: User
   owner_id: <%= ActiveRecord::FixtureSet.identify(:jane_doe) %>
 
+jean_doe_namespace:
+  name: jean.doe@localhost
+  path: jean.doe_at_localhost
+  type: User
+  owner_id: <%= ActiveRecord::FixtureSet.identify(:jane_doe) %>
+
 james_doe_namespace:
   name: james.doe@localhost
   path: james.doe_at_localhost

test/fixtures/routes.yml

@@ -32,6 +32,11 @@ jane_doe_namespace_route:
   path: jane.doe_at_localhost
   source: jane_doe_namespace (Namespace)
 
+jean_doe_namespace_route:
+  name: jean.doe@localhost
+  path: jean.doe_at_localhost
+  source: jean_doe_namespace (Namespace)
+
 james_doe_namespace_route:
   name: james.doe@localhost
   path: james.doe_at_localhost

test/fixtures/users.yml

@@ -11,6 +11,11 @@ jane_doe:
   first_name: Jane
   last_name: Doe
 
+jean_doe:
+  email: jean.doe@localhost
+  first_name: Jean
+  last_name: Doe
+
 james_doe:
   email: james.doe@localhost
   encrypted_password: <%= User.new.send :password_digest, "password1" %>

test/services/projects/destroy_service_test.rb

@@ -10,7 +10,7 @@ def setup
     end
 
     test 'delete project with with correct permissions' do
-      assert_difference -> { Project.count } => -1, -> { Member.count } => -4 do
+      assert_difference -> { Project.count } => -1, -> { Member.count } => -5 do
         Projects::DestroyService.new(@project, @user).execute
       end
     end

test/system/dashboard/projects_test.rb

@@ -93,5 +93,15 @@ def setup
       assert_selector 'h1', text: project_name
       assert_text project_description
     end
+
+    test 'can see projects that the user has been added to as a member' do
+      login_as users(:jean_doe)
+
+      visit dashboard_projects_url
+
+      assert_selector 'h1', text: I18n.t(:'dashboard.projects.index.title')
+      assert_selector 'tr', count: 1
+      assert_text projects(:john_doe_project2).human_name
+    end
   end
 end