From 50aed508d00a280aed297085d2548870d1d8476b Mon Sep 17 00:00:00 2001 From: Simon Tharby Date: Mon, 16 Dec 2024 15:23:59 +0000 Subject: [PATCH 1/2] [TAN-3370] Revert BE changes --- .../policies/project_folders/folder_policy.rb | 4 ++- .../admin_publications_filtering_service.rb | 15 +++++++++ .../acceptance/admin_publications_spec.rb | 33 ++++++++++--------- .../policies/admin_publication_policy_spec.rb | 4 +-- .../project_folders/folder_policy_spec.rb | 8 ++--- ...min_publications_filtering_service_spec.rb | 22 ++++++++++++- 6 files changed, 62 insertions(+), 24 deletions(-) diff --git a/back/app/policies/project_folders/folder_policy.rb b/back/app/policies/project_folders/folder_policy.rb index 294fa75f3754..272df02185d2 100644 --- a/back/app/policies/project_folders/folder_policy.rb +++ b/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? diff --git a/back/app/services/admin_publications_filtering_service.rb b/back/app/services/admin_publications_filtering_service.rb index e84e0c841951..af394afb5fb5 100644 --- a/back/app/services/admin_publications_filtering_service.rb +++ b/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] diff --git a/back/spec/acceptance/admin_publications_spec.rb b/back/spec/acceptance/admin_publications_spec.rb index 4563c62b772d..632bbacfe823 100644 --- a/back/spec/acceptance/admin_publications_spec.rb +++ b/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 diff --git a/back/spec/policies/admin_publication_policy_spec.rb b/back/spec/policies/admin_publication_policy_spec.rb index 8f5b6d67b964..84c618de5162 100644 --- a/back/spec/policies/admin_publication_policy_spec.rb +++ b/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 diff --git a/back/spec/policies/project_folders/folder_policy_spec.rb b/back/spec/policies/project_folders/folder_policy_spec.rb index dd481592817e..3d030f7ca130 100644 --- a/back/spec/policies/project_folders/folder_policy_spec.rb +++ b/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 diff --git a/back/spec/services/admin_publications_filtering_service_spec.rb b/back/spec/services/admin_publications_filtering_service_spec.rb index c566674bc685..25ccd24cac3f 100644 --- a/back/spec/services/admin_publications_filtering_service_spec.rb +++ b/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 From f2de0282e01758b3156ee492dad8940b28dcfd6f Mon Sep 17 00:00:00 2001 From: Simon Tharby Date: Mon, 16 Dec 2024 15:40:26 +0000 Subject: [PATCH 2/2] [TAN-3370] Revert FE changes --- front/app/api/admin_publications/types.ts | 1 + front/app/api/admin_publications/useAdminPublications.ts | 2 ++ .../useAdminPublicationsStatusCounts.ts | 2 ++ front/app/components/ProjectAndFolderCards/index.tsx | 3 +++ .../ContentBuilder/components/Widgets/Published/index.tsx | 1 + .../containers/CustomPageShow/CustomPageProjectsAndEvents.tsx | 2 ++ .../Components/DesktopNavItems/AdminPublicationsNavbarItem.tsx | 1 + front/app/containers/SiteMap/ProjectsAndFoldersSection.tsx | 1 + 8 files changed, 13 insertions(+) diff --git a/front/app/api/admin_publications/types.ts b/front/app/api/admin_publications/types.ts index c3b7af1152f8..659230ddd8f1 100644 --- a/front/app/api/admin_publications/types.ts +++ b/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; diff --git a/front/app/api/admin_publications/useAdminPublications.ts b/front/app/api/admin_publications/useAdminPublications.ts index 2c396b668445..b27e019f8ef7 100644 --- a/front/app/api/admin_publications/useAdminPublications.ts +++ b/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, diff --git a/front/app/api/admin_publications_status_counts/useAdminPublicationsStatusCounts.ts b/front/app/api/admin_publications_status_counts/useAdminPublicationsStatusCounts.ts index cc5c8e145951..48098feb33b1 100644 --- a/front/app/api/admin_publications_status_counts/useAdminPublicationsStatusCounts.ts +++ b/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, diff --git a/front/app/components/ProjectAndFolderCards/index.tsx b/front/app/components/ProjectAndFolderCards/index.tsx index b11039744d20..5608c6af2415 100644 --- a/front/app/components/ProjectAndFolderCards/index.tsx +++ b/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, } ); diff --git a/front/app/containers/Admin/pagesAndMenu/containers/ContentBuilder/components/Widgets/Published/index.tsx b/front/app/containers/Admin/pagesAndMenu/containers/ContentBuilder/components/Widgets/Published/index.tsx index 1b61af06e9b1..9d25a6883fc8 100644 --- a/front/app/containers/Admin/pagesAndMenu/containers/ContentBuilder/components/Widgets/Published/index.tsx +++ b/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, }); diff --git a/front/app/containers/CustomPageShow/CustomPageProjectsAndEvents.tsx b/front/app/containers/CustomPageShow/CustomPageProjectsAndEvents.tsx index ee6e30caab99..d4f629e6b45b 100644 --- a/front/app/containers/CustomPageShow/CustomPageProjectsAndEvents.tsx +++ b/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, } ); diff --git a/front/app/containers/MainHeader/Components/DesktopNavItems/AdminPublicationsNavbarItem.tsx b/front/app/containers/MainHeader/Components/DesktopNavItems/AdminPublicationsNavbarItem.tsx index df904e3cd306..c69ce4f68f20 100644 --- a/front/app/containers/MainHeader/Components/DesktopNavItems/AdminPublicationsNavbarItem.tsx +++ b/front/app/containers/MainHeader/Components/DesktopNavItems/AdminPublicationsNavbarItem.tsx @@ -154,6 +154,7 @@ const AdminPublicationsNavbarItem = ({ { publicationStatusFilter: ['published', 'archived'], rootLevelOnly: true, + removeNotAllowedParents: true, }, { enabled: projectsDropdownOpened } ); diff --git a/front/app/containers/SiteMap/ProjectsAndFoldersSection.tsx b/front/app/containers/SiteMap/ProjectsAndFoldersSection.tsx index b6523bdf9175..c828c53d98e3 100644 --- a/front/app/containers/SiteMap/ProjectsAndFoldersSection.tsx +++ b/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();