Merge pull request #9801 from CitizenLabDotCo/TAN-3370-revert-recent-…

…changes-to-folder-visibility

[TAN-3370] Revert recent changes to folder visibility in legacy projects (and folders) homepage widget

back/app/policies/project_folders/folder_policy.rb

@@ -19,8 +19,10 @@ def published_folders
     def show?
       return true if user && UserRoleService.new.can_moderate?(record, user)
       return false if record.admin_publication.publication_status == 'draft'
+      return true if record.projects.empty?
 
-      true
+      # We check if the user has access to at least one of the projects in the folder
+      scope_for(record.projects).exists?
     end
 
     def by_slug?

back/app/services/admin_publications_filtering_service.rb

@@ -20,6 +20,21 @@ def for_homepage_filter(scope)
     scope.where(publication_type: Project.name)
   end
 
+  # This filter removes AdminPublications that represent folders,
+  # which contain *only* projects which should not be visible to the current user.
+  add_filter('remove_not_allowed_parents') do |scope, options|
+    next scope unless ['true', true, '1'].include? options[:remove_not_allowed_parents]
+
+    # We remove parents that have only draft projects
+    parents_with_visible_children = scope.where(id: scope.not_draft.filter_map(&:parent_id).uniq)
+    parents_without_any_children  = scope.where(children_allowed: true, children_count: 0)
+    non_parents                   = scope.where(children_allowed: false)
+
+    parents_with_visible_children
+      .or(parents_without_any_children)
+      .or(non_parents)
+  end
+
   add_filter('by_publication_status') do |scope, options|
     publication_status = options[:publication_statuses]
 

back/spec/acceptance/admin_publications_spec.rb

@@ -39,6 +39,7 @@
       parameter :search, 'Search text of title, description, preview, and slug', required: false
       parameter :publication_statuses, 'Return only publications with the specified publication statuses (i.e. given an array of publication statuses); always includes folders; returns all publications by default (OR)', required: false
       parameter :folder, 'Filter by folder (project folder id)', required: false
+      parameter :remove_not_allowed_parents, 'Filter out folders which contain only projects that are not visible to the user', required: false
       parameter :only_projects, 'Include projects only (no folders)', required: false
       parameter :filter_can_moderate, 'Filter out the projects the current_user is not allowed to moderate. False by default', required: false
       parameter :filter_is_moderator_of, 'Filter out the publications the current_user is not moderator of. False by default', required: false
@@ -405,38 +406,35 @@
       end
       parameter :topics, 'Filter by topics (AND)', required: false
       parameter :areas, 'Filter by areas (AND)', required: false
+      parameter :remove_not_allowed_parents, 'Filter out folders with no visible children for the current user', required: false
       parameter :publication_statuses, 'Return only publications with the specified publication statuses (i.e. given an array of publication statuses); always includes folders; returns all publications by default', required: false
       parameter :folder, 'Filter by folder (project folder id)', required: false
       parameter :include_publications, 'Include the related publications and associated items', required: false
 
       example 'Listed admin publications have correct visible children count', document: false do
-        do_request(folder: nil)
+        do_request(folder: nil, remove_not_allowed_parents: true)
         expect(status).to eq(200)
         json_response = json_parse(response_body)
         # Only 3 of initial 6 projects are not in folder
-        expect(json_response[:data].size).to eq 4
-        # Only 2 folders expected - Draft folder created at top of file is not visible to resident,
-        expect(json_response[:data].map { |d| d.dig(:relationships, :publication, :data, :type) }.count('folder')).to eq 2
+        expect(json_response[:data].size).to eq 3
+        # Only 1 folder expected - Draft folder created at top of file is not visible to resident,
+        # nor should a folder with only a draft project in it
+        expect(json_response[:data].map { |d| d.dig(:relationships, :publication, :data, :type) }.count('folder')).to eq 1
         # 3 projects are inside folder, 3 top-level projects remain, of which 1 is not visible (draft)
         expect(json_response[:data].map { |d| d.dig(:relationships, :publication, :data, :type) }.count('project')).to eq 2
-
-        folders_data = (json_response[:data].select { |d| d.dig(:relationships, :publication, :data, :type) == 'folder' })
-        # Only the two non-draft projects (in a folder) are visible to resident
-        expect(folders_data.map { |d| d.dig(:attributes, :visible_children_count) }).to match_array [0, 2]
+        # Only the two non-draft projects are visible to resident
+        expect(json_response[:data].find { |d| d.dig(:relationships, :publication, :data, :type) == 'folder' }.dig(:attributes, :visible_children_count)).to eq 2
       end
 
       example 'Visible children count should take account of applied filters', document: false do
         projects.first.admin_publication.update! publication_status: 'archived'
-        do_request(folder: nil, publication_statuses: ['published'])
+        do_request(folder: nil, publication_statuses: ['published'], remove_not_allowed_parents: true)
         expect(status).to eq(200)
         json_response = json_parse(response_body)
-        expect(json_response[:data].size).to eq 3
-        expect(json_response[:data].map { |d| d.dig(:relationships, :publication, :data, :type) }.count('folder')).to eq 2
+        expect(json_response[:data].size).to eq 2
+        expect(json_response[:data].map { |d| d.dig(:relationships, :publication, :data, :type) }.count('folder')).to eq 1
         expect(json_response[:data].map { |d| d.dig(:relationships, :publication, :data, :type) }.count('project')).to eq 1
-
-        folders_data = (json_response[:data].select { |d| d.dig(:relationships, :publication, :data, :type) == 'folder' })
-        # Only the one non-draft project (in a folder) is visible to resident
-        expect(folders_data.map { |d| d.dig(:attributes, :visible_children_count) }).to match_array [0, 1]
+        expect(json_response[:data].find { |d| d.dig(:relationships, :publication, :data, :type) == 'folder' }.dig(:attributes, :visible_children_count)).to eq 1
       end
 
       context 'search param' do
@@ -626,6 +624,7 @@
       parameter :search, 'Search text of title, description, preview, and slug', required: false
       parameter :publication_statuses, 'Return only publications with the specified publication statuses (i.e. given an array of publication statuses); always includes folders; returns all publications by default (OR)', required: false
       parameter :folder, 'Filter by folder (project folder id)', required: false
+      parameter :remove_not_allowed_parents, 'Filter out folders which contain only projects that are not visible to the user', required: false
       parameter :only_projects, 'Include projects only (no folders)', required: false
       parameter :filter_can_moderate, 'Filter out the projects the user is allowed to moderate. False by default', required: false
       parameter :filter_is_moderator_of, 'Filter out the publications the user is not moderator of. False by default', required: false
@@ -675,6 +674,7 @@
       parameter :search, 'Search text of title, description, preview, and slug', required: false
       parameter :publication_statuses, 'Return only publications with the specified publication statuses (i.e. given an array of publication statuses); always includes folders; returns all publications by default (OR)', required: false
       parameter :folder, 'Filter by folder (project folder id)', required: false
+      parameter :remove_not_allowed_parents, 'Filter out folders which contain only projects that are not visible to the user', required: false
       parameter :only_projects, 'Include projects only (no folders)', required: false
       parameter :filter_can_moderate, 'Filter out the projects the user is allowed to moderate. False by default', required: false
       parameter :filter_is_moderator_of, 'Filter out the publications the user is not moderator of. False by default', required: false
@@ -806,6 +806,7 @@
         end
         parameter :depth, 'Filter by depth', required: false
         parameter :publication_statuses, 'Return only publications with the specified publication statuses (i.e. given an array of publication statuses); always includes folders; returns all publications by default (OR)', required: false
+        parameter :remove_not_allowed_parents, 'Filter out folders which contain only projects that are not visible to the user', required: false
         parameter :include_publications, 'Include the related publications and associated items', required: false
 
         example_request 'Index action does not invoke unnecessary queries' do
@@ -830,7 +831,7 @@
               publication_statuses: %w[published archived],
               include_publications: 'true'
             )
-          end.not_to exceed_query_limit(122)
+          end.not_to exceed_query_limit(123)
 
           assert_status 200
         end

back/spec/policies/admin_publication_policy_spec.rb

@@ -114,13 +114,13 @@
     context 'when visitor' do
       let(:user) { nil }
 
-      it { is_expected.to permit(:show) }
+      it { is_expected.not_to permit(:show) }
     end
 
     context 'when regular user' do
       let(:user) { create(:user) }
 
-      it { is_expected.to permit(:show) }
+      it { is_expected.not_to permit(:show) }
     end
 
     context 'when admin' do

back/spec/policies/project_folders/folder_policy_spec.rb

@@ -187,13 +187,13 @@
     context 'when visitor' do
       let(:user) { nil }
 
-      it { is_expected.to permit(:show) }
+      it { is_expected.not_to permit(:show) }
     end
 
     context 'when not member' do
       let(:user) { create(:user) }
 
-      it { is_expected.to permit(:show) }
+      it { is_expected.not_to permit(:show) }
     end
 
     context 'when member' do
@@ -278,13 +278,13 @@
     context 'when visitor' do
       let(:user) { nil }
 
-      it { is_expected.to permit(:show) }
+      it { is_expected.not_to permit(:show) }
     end
 
     context 'when regular user' do
       let(:user) { create(:user) }
 
-      it { is_expected.to permit(:show) }
+      it { is_expected.not_to permit(:show) }
     end
 
     context 'when admin' do

back/spec/services/admin_publications_filtering_service_spec.rb

@@ -24,16 +24,36 @@
       expect(result.ids).not_to include(*tree_mock.other.where(publication_status: %w[draft]).ids)
     end
 
+    it 'does not include a parent if all its children are in draft' do
+      expect(result.ids).not_to include(tree_mock.published_parent_with_draft_children.id)
+    end
+
     it 'does not include the draft children of a published parent' do
       expect(result.ids).not_to include(*tree_mock.published_parent_with_draft_children.children.ids)
     end
   end
 
   context 'when a normal user searching from the home page' do
-    let(:options) { { depth: 0, publication_statuses: %w[archived published] } }
+    let(:options) { { depth: 0, remove_not_allowed_parents: true, publication_statuses: %w[archived published] } }
     let(:base_scope) { Pundit.policy_scope(create(:user), AdminPublication.includes(:parent)) }
 
     include_examples 'when a user searching from the home page'
+
+    it 'does not include parents when the user has no access to its children' do
+      expect(result.ids).not_to include(*tree_mock.admin_only_parents.ids)
+    end
+  end
+
+  context 'when an admin searching from the home page' do
+    let(:options) { { depth: '0', remove_not_allowed_parents: 'true', publication_statuses: %w[archived published] } }
+    let(:base_scope) { Pundit.policy_scope(create(:admin), AdminPublication.includes(:parent)) }
+
+    include_examples 'when a user searching from the home page'
+
+    it 'does include parents when the admin has access to its children' do
+      # we use not_draft because we filter out draft projects by publication_statuses param
+      expect(result.ids).to include(*tree_mock.admin_only_parents.not_draft.ids)
+    end
   end
 
   context 'when an admin searching from the admin dashboard' do

front/app/api/admin_publications/types.ts

@@ -17,6 +17,7 @@ export interface IQueryParameters {
   pageNumber?: number;
   pageSize?: number;
   rootLevelOnly?: boolean;
+  removeNotAllowedParents?: boolean;
   onlyProjects?: boolean;
   filter_is_moderator_of?: boolean;
   filter_user_is_moderator_of?: string;

front/app/api/admin_publications/useAdminPublications.ts

@@ -16,6 +16,7 @@ export const fetchAdminPublications = (filters: IQueryParameters) => {
     pageNumber,
     pageSize,
     rootLevelOnly,
+    removeNotAllowedParents,
     publicationStatusFilter,
     childrenOfId,
     topicIds,
@@ -30,6 +31,7 @@ export const fetchAdminPublications = (filters: IQueryParameters) => {
       'page[number]': pageNumber || 1,
       'page[size]': pageSize || 1000,
       depth: rootLevelOnly ? 0 : undefined,
+      remove_not_allowed_parents: removeNotAllowedParents,
       publication_statuses: publicationStatusFilter,
       folder: childrenOfId,
       topics: topicIds,

front/app/api/admin_publications_status_counts/useAdminPublicationsStatusCounts.ts

@@ -10,6 +10,7 @@ import { IStatusCounts, AdminPublicationsStatusCountsKeys } from './types';
 
 export const fetchStatusCounts = ({
   rootLevelOnly,
+  removeNotAllowedParents,
   publicationStatusFilter,
   childrenOfId,
   topicIds,
@@ -25,6 +26,7 @@ export const fetchStatusCounts = ({
       'page[size]': undefined,
       'page[number]': undefined,
       depth: rootLevelOnly ? 0 : undefined,
+      remove_not_allowed_parents: removeNotAllowedParents,
       publication_statuses: publicationStatusFilter,
       folder: childrenOfId,
       topics: topicIds,

front/app/components/ProjectAndFolderCards/index.tsx

@@ -67,6 +67,7 @@ const ProjectAndFolderCards = ({
       {
         publicationStatusFilter: PUBLICATION_STATUSES,
         rootLevelOnly,
+        removeNotAllowedParents: true,
         topicIds,
         areaIds,
         search,
@@ -85,6 +86,7 @@ const ProjectAndFolderCards = ({
     pageSize: 6,
     publicationStatusFilter: getPublicationStatuses(currentTab),
     rootLevelOnly,
+    removeNotAllowedParents: true,
     topicIds,
     areaIds,
     search,
@@ -134,6 +136,7 @@ const ProjectAndFolderCardsWrapper = (props: Props) => {
     {
       publicationStatusFilter: PUBLICATION_STATUSES,
       rootLevelOnly: true,
+      removeNotAllowedParents: true,
     }
   );
 

front/app/containers/Admin/pagesAndMenu/containers/ContentBuilder/components/Widgets/Published/index.tsx

@@ -25,6 +25,7 @@ const Published = ({ titleMultiloc }: Props) => {
       pageSize: 6,
       publicationStatusFilter: ['published'],
       rootLevelOnly: true,
+      removeNotAllowedParents: true,
       include_publications: true,
     });
 

front/app/containers/CustomPageShow/CustomPageProjectsAndEvents.tsx

@@ -73,6 +73,7 @@ const CustomPageProjectsAndEvents = ({
     areaIds,
     publicationStatusFilter: getPublicationStatuses(currentTab),
     rootLevelOnly: false,
+    removeNotAllowedParents: true,
     onlyProjects: true,
   });
 
@@ -137,6 +138,7 @@ const CustomPageProjectsAndEventsWrapper = ({ page }: Props) => {
       areaIds,
       publicationStatusFilter: PUBLICATION_STATUSES,
       rootLevelOnly: false,
+      removeNotAllowedParents: true,
       onlyProjects: true,
     }
   );

front/app/containers/MainHeader/Components/DesktopNavItems/AdminPublicationsNavbarItem.tsx

@@ -154,6 +154,7 @@ const AdminPublicationsNavbarItem = ({
     {
       publicationStatusFilter: ['published', 'archived'],
       rootLevelOnly: true,
+      removeNotAllowedParents: true,
     },
     { enabled: projectsDropdownOpened }
   );

front/app/containers/SiteMap/ProjectsAndFoldersSection.tsx

@@ -27,6 +27,7 @@ const ProjectsAndFoldersSection = ({ projectsSectionRef }: Props) => {
   const { data } = useAdminPublications({
     publicationStatusFilter: ['draft', 'published', 'archived'],
     rootLevelOnly: true,
+    removeNotAllowedParents: true,
   });
 
   const adminPublications = data?.pages.map((page) => page.data).flat();