From 10239afabdbcb0d2576d442d5658e539736a7dce Mon Sep 17 00:00:00 2001 From: Jim Safley Date: Thu, 14 Mar 2024 14:26:16 -0400 Subject: [PATCH 1/7] Filter default sites for items user setting (fix #2166) --- .../src/Form/Element/AbstractGroupByOwnerSelect.php | 13 ++++++++++--- application/src/Form/UserForm.php | 9 +++++++++ .../view/omeka/admin/item/manage-sites.phtml | 4 ++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/application/src/Form/Element/AbstractGroupByOwnerSelect.php b/application/src/Form/Element/AbstractGroupByOwnerSelect.php index f32242558b..e5c94902dd 100644 --- a/application/src/Form/Element/AbstractGroupByOwnerSelect.php +++ b/application/src/Form/Element/AbstractGroupByOwnerSelect.php @@ -50,12 +50,19 @@ public function getValueOptions(): array $query = []; } - $response = $this->getApiManager()->search($this->getResourceName(), $query); + $resourceReps = $this->getApiManager()->search($this->getResourceName(), $query)->getContent(); + + // Provide a way to filter the resource representations prior to + // building the value options. + $callback = $this->getOption('filter_resource_representations'); + if (is_callable($callback)) { + $resourceReps = $callback($resourceReps); + } if ($this->getOption('disable_group_by_owner')) { // Group alphabetically by resource label without grouping by owner. $resources = []; - foreach ($response->getContent() as $resource) { + foreach ($resourceReps as $resource) { $resources[$this->getValueLabel($resource)][] = $resource->id(); } ksort($resources); @@ -68,7 +75,7 @@ public function getValueOptions(): array } else { // Group alphabetically by owner email. $resourceOwners = []; - foreach ($response->getContent() as $resource) { + foreach ($resourceReps as $resource) { $owner = $resource->owner(); $index = $owner ? $owner->email() : null; $resourceOwners[$index]['owner'] = $owner; diff --git a/application/src/Form/UserForm.php b/application/src/Form/UserForm.php index f89463706c..26c39ab782 100644 --- a/application/src/Form/UserForm.php +++ b/application/src/Form/UserForm.php @@ -199,6 +199,15 @@ public function init() 'options' => [ 'label' => 'Default sites for items', // @translate 'empty_option' => '', + 'filter_resource_representations' => function ($sites) { + // The user must have permission to assign items to the site. + foreach ($sites as $index => $site) { + if (!$site->userIsAllowed('can-assign-items')) { + unset($sites[$index]); + } + } + return $sites; + }, ], ]); $settingsFieldset->add([ diff --git a/application/view/omeka/admin/item/manage-sites.phtml b/application/view/omeka/admin/item/manage-sites.phtml index 2fbef150e2..09dc2849ca 100644 --- a/application/view/omeka/admin/item/manage-sites.phtml +++ b/application/view/omeka/admin/item/manage-sites.phtml @@ -13,6 +13,10 @@ if ($item) { $sites = []; foreach ($siteRepresentations as $siteRepresentation) { + // The user must have permission to assign items to the site. + if (!$siteRepresentation->userIsAllowed('can-assign-items')) { + continue; + } $sites[] = [ 'id' => $siteRepresentation->id(), ]; From 305ecc043f36727fbb89f67079909ea5cba05878 Mon Sep 17 00:00:00 2001 From: Jim Safley Date: Fri, 15 Mar 2024 13:02:12 -0400 Subject: [PATCH 2/7] Account for sites set to auto-assign but the user does not have permission to assign --- application/asset/js/global.js | 4 ++ .../view/omeka/admin/item/manage-sites.phtml | 65 ++++++++++++++----- 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/application/asset/js/global.js b/application/asset/js/global.js index 4b0c96e7f7..d2b7442734 100644 --- a/application/asset/js/global.js +++ b/application/asset/js/global.js @@ -235,6 +235,10 @@ var Omeka = { var resource = selector.find('[data-resource-id="' + id + '"]'); var resourceParent = resource.parents('.selector-parent'); var childCount = resourceParent.find('.selector-child-count').first(); + // Update the count only when the resource exists in the selector. + if (!selector.find(`.selector-child[data-resource-id="${id}"]`).length) { + return; + } if (resource.hasClass('added')) { var newTotalCount = parseInt(selectorCount.text()) - 1; var newChildCount = parseInt(childCount.text()) - 1; diff --git a/application/view/omeka/admin/item/manage-sites.phtml b/application/view/omeka/admin/item/manage-sites.phtml index 09dc2849ca..e2ad1fa5b6 100644 --- a/application/view/omeka/admin/item/manage-sites.phtml +++ b/application/view/omeka/admin/item/manage-sites.phtml @@ -1,25 +1,43 @@ sites(); + // This is an existing item. + + $sites = $item->sites(); + foreach ($sites as $site) { + if ($site->userIsAllowed('can-assign-items')) { + $canAssignItemsSites[] = ['id' => $site->id()]; + } else { + $cannotAssignItemsSites[$site->id()] = $site; + } + } } else { - $siteRepresentations = $this->api()->search('sites', ['assign_new_items' => true])->getContent(); + // This is a new item. - $userDefaultSites = $this->userSetting('default_item_sites'); - if (is_array($userDefaultSites)) { - $userDefaultSiteRepresentations = $this->api()->search('sites', ['id' => $userDefaultSites])->getContent(); - $siteRepresentations = array_merge($siteRepresentations, $userDefaultSiteRepresentations); + // Get all sites that allow item assignment in site settings. + $sites = $this->api()->search('sites', ['assign_new_items' => true])->getContent(); + foreach ($sites as $site) { + if ($site->userIsAllowed('can-assign-items')) { + $canAssignItemsSites[] = ['id' => $site->id()]; + } else { + $cannotAssignItemsSites[$site->id()] = $site; + } } -} -$sites = []; -foreach ($siteRepresentations as $siteRepresentation) { - // The user must have permission to assign items to the site. - if (!$siteRepresentation->userIsAllowed('can-assign-items')) { - continue; + // Get all sites that the user set as default in user settings. + $defaultItemSites = $this->userSetting('default_item_sites'); + $sites = is_array($defaultItemSites) + ? $this->api()->search('sites', ['id' => $defaultItemSites])->getContent() + : []; + foreach ($sites as $site) { + if ($site->userIsAllowed('can-assign-items')) { + $canAssignItemsSites[] = ['id' => $site->id()]; + } else { + $cannotAssignItemsSites[$site->id()] = $site; + } } - $sites[] = [ - 'id' => $siteRepresentation->id(), - ]; } $siteTemplate = ' @@ -34,7 +52,7 @@ $siteTemplate = ' '; ?> - +
@@ -42,7 +60,20 @@ $siteTemplate = ' - + + + + + + + + +
translate('Title'); ?>
escapeHtml($site->title()); ?>escapeHtml($site->owner()->email()); ?> +
    +
  • hyperlink('', '#', ['class' => 'o-icon-delete', 'title' => $this->translate('Unassign from site')]); ?>
    • +
+ +
From cded28af3e9381c5244e467853b274e085f5ddab Mon Sep 17 00:00:00 2001 From: Jim Safley Date: Fri, 15 Mar 2024 14:28:32 -0400 Subject: [PATCH 3/7] Append to the correct element (tbody) --- application/asset/js/global.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/asset/js/global.js b/application/asset/js/global.js index d2b7442734..97db7c2e8b 100644 --- a/application/asset/js/global.js +++ b/application/asset/js/global.js @@ -227,7 +227,7 @@ var Omeka = { tableRowCell.text(tableRowValue); }); selectorRow.addClass('added'); - table.append(tableRow).removeClass('empty').trigger('appendRow'); + table.children('.resource-rows').append(tableRow).removeClass('empty').trigger('appendRow'); updateResourceCount(id); } From d97dd804b9810fb59607f439f8cf583e699e4893 Mon Sep 17 00:00:00 2001 From: Jim Safley Date: Fri, 15 Mar 2024 14:29:59 -0400 Subject: [PATCH 4/7] Add site ID to hidden inputs --- application/view/omeka/admin/item/manage-sites.phtml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/view/omeka/admin/item/manage-sites.phtml b/application/view/omeka/admin/item/manage-sites.phtml index e2ad1fa5b6..4d03fa3d46 100644 --- a/application/view/omeka/admin/item/manage-sites.phtml +++ b/application/view/omeka/admin/item/manage-sites.phtml @@ -69,7 +69,7 @@ $siteTemplate = '
  • hyperlink('', '#', ['class' => 'o-icon-delete', 'title' => $this->translate('Unassign from site')]); ?>
- + From 5b0c39c62a33ee64e0e6664fef7d8445acaa201b Mon Sep 17 00:00:00 2001 From: Jim Safley Date: Fri, 15 Mar 2024 14:30:50 -0400 Subject: [PATCH 5/7] On create users can assign item to sites if they do not otherwise have permission --- application/src/Api/Adapter/ItemAdapter.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/application/src/Api/Adapter/ItemAdapter.php b/application/src/Api/Adapter/ItemAdapter.php index d85bebd435..fda745ddd5 100644 --- a/application/src/Api/Adapter/ItemAdapter.php +++ b/application/src/Api/Adapter/ItemAdapter.php @@ -254,7 +254,11 @@ public function hydrate(Request $request, EntityInterface $entity, if (!$site) { // Assign site that was not already assigned. $site = $siteAdapter->findEntity($siteId); - if ($acl->userIsAllowed($site, 'can-assign-items')) { + if ($isCreate) { + // Only on create can users assign to sites if they don't + // otherwise have permission to do so. + $sites->set($site->getId(), $site); + } elseif ($acl->userIsAllowed($site, 'can-assign-items')) { $sites->set($site->getId(), $site); } } From 7bfe4185e0778b0d00699bbc1cfe29287381b3f0 Mon Sep 17 00:00:00 2001 From: Jim Safley Date: Mon, 18 Mar 2024 10:00:04 -0400 Subject: [PATCH 6/7] Prevent unflagged sites from sneaking in on create --- application/src/Api/Adapter/ItemAdapter.php | 26 +++++++++++++-------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/application/src/Api/Adapter/ItemAdapter.php b/application/src/Api/Adapter/ItemAdapter.php index fda745ddd5..8abff9ba81 100644 --- a/application/src/Api/Adapter/ItemAdapter.php +++ b/application/src/Api/Adapter/ItemAdapter.php @@ -1,6 +1,7 @@ getValue('o:site'))) { - // On CREATE and when no "o:site" array is passed, assign this item - // to all sites where assignNewItems=true. - $dql = ' - SELECT site + if ($isCreate) { + // On CREATE we will need sites where assignNewItems=true. + $dql = 'SELECT site FROM Omeka\Entity\Site site WHERE site.assignNewItems = true'; $query = $this->getEntityManager()->createQuery($dql); + $assignNewItemsSites = new ArrayCollection($query->getResult()); + } + if ($isCreate && !is_array($request->getValue('o:site'))) { + // On CREATE and when no "o:site" array is passed, assign this item + // to all sites where assignNewItems=true. $sites = $entity->getSites(); - foreach ($query->getResult() as $site) { + foreach ($assignNewItemsSites as $site) { $sites->set($site->getId(), $site); } } elseif ($this->shouldHydrate($request, 'o:site')) { @@ -254,11 +258,13 @@ public function hydrate(Request $request, EntityInterface $entity, if (!$site) { // Assign site that was not already assigned. $site = $siteAdapter->findEntity($siteId); - if ($isCreate) { - // Only on create can users assign to sites if they don't - // otherwise have permission to do so. + if ($acl->userIsAllowed($site, 'can-assign-items')) { + // A user with the "can-assign-items" privilege can assign + // a site at any time. $sites->set($site->getId(), $site); - } elseif ($acl->userIsAllowed($site, 'can-assign-items')) { + } elseif ($isCreate && $assignNewItemsSites->contains($site)) { + // A user without the "can-assign-items" privilege can assign + // a site only on CREATE when the site has assignNewItems=true. $sites->set($site->getId(), $site); } } From d68b6aeb66e67b9c8a0b1d38a05882a400947df3 Mon Sep 17 00:00:00 2001 From: Jim Safley Date: Thu, 28 Mar 2024 13:53:24 -0400 Subject: [PATCH 7/7] Clarify if condition --- application/src/Api/Adapter/ItemAdapter.php | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/application/src/Api/Adapter/ItemAdapter.php b/application/src/Api/Adapter/ItemAdapter.php index 8abff9ba81..3710094929 100644 --- a/application/src/Api/Adapter/ItemAdapter.php +++ b/application/src/Api/Adapter/ItemAdapter.php @@ -258,13 +258,11 @@ public function hydrate(Request $request, EntityInterface $entity, if (!$site) { // Assign site that was not already assigned. $site = $siteAdapter->findEntity($siteId); - if ($acl->userIsAllowed($site, 'can-assign-items')) { + if ($acl->userIsAllowed($site, 'can-assign-items') || ($isCreate && $assignNewItemsSites->contains($site))) { // A user with the "can-assign-items" privilege can assign - // a site at any time. - $sites->set($site->getId(), $site); - } elseif ($isCreate && $assignNewItemsSites->contains($site)) { - // A user without the "can-assign-items" privilege can assign - // a site only on CREATE when the site has assignNewItems=true. + // a site at any time. A user without the "can-assign-items" + // privilege can assign a site only on CREATE when the site + // has assignNewItems=true. $sites->set($site->getId(), $site); } }