Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IBX-5053: Handled deleted Locations in UDW-based limitations #2089

Merged
merged 6 commits into from
Mar 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/bundle/Resources/config/services/role_form_mappers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,10 @@ services:
ezplatform.content_forms.limitation.form_mapper.udw_based:
class: EzSystems\EzPlatformAdminUi\Limitation\Mapper\UDWBasedMapper
arguments:
- "@ezpublish.api.service.location"
- "@ezpublish.api.service.search"
$locationService: "@ezpublish.api.service.location"
$searchService: "@ezpublish.api.service.search"
$permissionResolver: '@eZ\Publish\API\Repository\PermissionResolver'
$repository: '@ezpublish.api.repository'
calls:
- [setFormTemplate, ["%ezplatform.content_forms.limitation.udw.template%"]]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,9 @@
};
const attachTagEventHandlers = (limitationBtn, tag) => {
const removeTagBtn = tag.querySelector('.ez-tag__remove-btn');

removeTagBtn.addEventListener('click', () => handleTagRemove(limitationBtn, tag), false);
if (removeTagBtn !== null) {
removeTagBtn.addEventListener('click', () => handleTagRemove(limitationBtn, tag), false);
}
};
const closeUDW = () => ReactDOM.unmountComponentAtNode(udwContainer);
const handleUdwConfirm = (limitationBtn, selectedItems) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@
<target state="new">Select Location(s) to use as Limitation</target>
<note>key: role.policy.limitation.location.udw_title</note>
</trans-unit>
<trans-unit id="290f48ccc1eb2889852b730ac62b2b148594c85f" resname="role.policy.limitation.location_deleted">
<source>Location deleted</source>
<target state="new">Location deleted</target>
<note>key: role.policy.limitation.location_deleted</note>
</trans-unit>
<trans-unit id="be8159ab7985797bd281cb0f7f58bb4d2c925159" resname="role.policy.limitation.not_implemented">
<source>Editing Limitations for '%limitationTypeIdentifier%' is not available.</source>
<target state="new">Editing Limitations for '%limitationTypeIdentifier%' is not available.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,18 @@
{% set content_breadcrumbs = content_breadcrumbs ~ ' / ' %}
{% endif %}
{% endfor %}

{{ include('@ezdesign/limitation/udw_limitation_value_list_item.html.twig', {
'content_breadcrumbs': content_breadcrumbs,
'location_id': limitationValue.id,
'is_loading_state': false
}) }}
{% else %}
<li class="mt-2">
<div class="ez-tag">
{{ "role.policy.limitation.location_deleted"|trans({}, "ezplatform_content_forms_role")|desc("Location deleted") }}
</div>
</li>
{% endif %}
{% endfor %}
</ul>
Expand Down
44 changes: 31 additions & 13 deletions src/lib/Form/DataTransformer/UDWBasedValueModelTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
namespace EzSystems\EzPlatformAdminUi\Form\DataTransformer;

use eZ\Publish\API\Repository\Exceptions\NotFoundException;
use eZ\Publish\API\Repository\Exceptions\UnauthorizedException;
use eZ\Publish\API\Repository\LocationService;
use eZ\Publish\API\Repository\PermissionResolver;
use eZ\Publish\API\Repository\Repository;
use eZ\Publish\API\Repository\Values\Content\Location;
use Symfony\Component\Form\DataTransformerInterface;
use Symfony\Component\Form\Exception\TransformationFailedException;

Expand All @@ -24,12 +26,20 @@ class UDWBasedValueModelTransformer implements DataTransformerInterface
/** @var \eZ\Publish\API\Repository\LocationService */
private $locationService;

/**
* @param \eZ\Publish\API\Repository\LocationService $locationService
*/
public function __construct(LocationService $locationService)
{
/** @var \eZ\Publish\API\Repository\PermissionResolver */
private $permissionResolver;

/** @var \eZ\Publish\API\Repository\Repository */
private $repository;

public function __construct(
LocationService $locationService,
PermissionResolver $permissionResolver,
Repository $repository
) {
$this->locationService = $locationService;
$this->permissionResolver = $permissionResolver;
$this->repository = $repository;
}

/**
Expand All @@ -43,16 +53,24 @@ public function transform($value): ?array
return null;
}

return array_map([$this, 'mapPathToLocation'], $value);
}

private function mapPathToLocation(string $path): ?Location
{
$locationId = $this->extractLocationIdFromPath($path);

try {
return array_map(function (string $path) {
return $this->locationService->loadLocation(
$this->extractLocationIdFromPath($path)
);
}, $value);
// Sudo is necessary as skipping non-accessible Locations
// will prevent an administrator from editing policies
return $this->permissionResolver->sudo(
function () use ($locationId): Location {
return $this->locationService->loadLocation($locationId);
},
$this->repository
);
} catch (NotFoundException $e) {
return null;
} catch (UnauthorizedException $e) {
throw new TransformationFailedException($e->getMessage(), $e->getCode(), $e);
}
}

Expand Down
11 changes: 11 additions & 0 deletions src/lib/Limitation/Mapper/SubtreeLimitationMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
namespace EzSystems\EzPlatformAdminUi\Limitation\Mapper;

use eZ\Publish\API\Repository\Exceptions\NotFoundException;
use eZ\Publish\API\Repository\Values\Content\LocationQuery;
use eZ\Publish\API\Repository\Values\Content\Query\Criterion\Ancestor;
use eZ\Publish\API\Repository\Values\Content\Query\SortClause\Location\Path;
Expand All @@ -32,6 +33,16 @@ public function mapLimitationValue(Limitation $limitation)
$values = [];

foreach ($limitation->limitationValues as $pathString) {
$pathParts = explode('/', trim($pathString, '/'));
$locationId = (int) array_pop($pathParts);

try {
$this->locationService->loadLocation($locationId);
} catch (NotFoundException $e) {
// Skip generating limitation value as Location doesn't exist at this point
continue;
}

$query = new LocationQuery([
'filter' => new Ancestor($pathString),
'sortClauses' => [new Path()],
Expand Down
31 changes: 23 additions & 8 deletions src/lib/Limitation/Mapper/UDWBasedMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
namespace EzSystems\EzPlatformAdminUi\Limitation\Mapper;

use eZ\Publish\API\Repository\LocationService;
use eZ\Publish\API\Repository\PermissionResolver;
use eZ\Publish\API\Repository\Repository;
use eZ\Publish\API\Repository\SearchService;
use eZ\Publish\API\Repository\Values\Content\LocationQuery;
use eZ\Publish\API\Repository\Values\Content\Query\Criterion\Ancestor;
Expand Down Expand Up @@ -42,15 +44,22 @@ class UDWBasedMapper implements LimitationFormMapperInterface, LimitationValueMa
*/
private $template;

/**
* UDWBasedMapper constructor.
*
* @param \eZ\Publish\API\Repository\SearchService $searchService
*/
public function __construct(LocationService $locationService, SearchService $searchService)
{
/** @var \eZ\Publish\API\Repository\PermissionResolver */
private $permissionResolver;

/** @var \eZ\Publish\API\Repository\Repository */
private $repository;

public function __construct(
LocationService $locationService,
SearchService $searchService,
PermissionResolver $permissionResolver,
Repository $repository
) {
$this->locationService = $locationService;
$this->searchService = $searchService;
$this->permissionResolver = $permissionResolver;
$this->repository = $repository;
}

public function setFormTemplate($template)
Expand All @@ -74,7 +83,13 @@ public function mapLimitationForm(FormInterface $form, Limitation $data)
'label' => LimitationTranslationExtractor::identifierToLabel($data->getIdentifier()),
])
->addViewTransformer(new UDWBasedValueViewTransformer($this->locationService))
->addModelTransformer(new UDWBasedValueModelTransformer($this->locationService))
->addModelTransformer(
new UDWBasedValueModelTransformer(
$this->locationService,
$this->permissionResolver,
$this->repository
)
)
// Deactivate auto-initialize as we're not on the root form.
->setAutoInitialize(false)->getForm()
);
Expand Down
11 changes: 10 additions & 1 deletion src/lib/Tests/Limitation/Mapper/SubtreeLimitationMapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
namespace EzSystems\EzPlatformAdminUi\Tests\Limitation\Mapper;

use eZ\Publish\API\Repository\LocationService;
use eZ\Publish\API\Repository\PermissionResolver;
use eZ\Publish\API\Repository\Repository;
use eZ\Publish\API\Repository\SearchService;
use eZ\Publish\API\Repository\Values\Content\ContentInfo;
use eZ\Publish\API\Repository\Values\Content\Location;
Expand Down Expand Up @@ -44,6 +46,8 @@ public function testMapLimitationValue()

$locationServiceMock = $this->createMock(LocationService::class);
$searchServiceMock = $this->createMock(SearchService::class);
$permissionResolverMock = $this->createMock(PermissionResolver::class);
$repositoryMock = $this->createMock(Repository::class);

foreach ($values as $i => $pathString) {
$query = new LocationQuery([
Expand All @@ -58,7 +62,12 @@ public function testMapLimitationValue()
->willReturn($this->createSearchResultsMock($expected[$i]));
}

$mapper = new SubtreeLimitationMapper($locationServiceMock, $searchServiceMock);
$mapper = new SubtreeLimitationMapper(
$locationServiceMock,
$searchServiceMock,
$permissionResolverMock,
$repositoryMock
);
$result = $mapper->mapLimitationValue(new SubtreeLimitation([
'limitationValues' => $values,
]));
Expand Down
21 changes: 15 additions & 6 deletions src/lib/Tests/Limitation/Mapper/UDWBasedMapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
namespace EzSystems\EzPlatformAdminUi\Tests\Limitation\Mapper;

use eZ\Publish\API\Repository\LocationService;
use eZ\Publish\API\Repository\Repository;
use eZ\Publish\API\Repository\SearchService;
use eZ\Publish\API\Repository\Values\Content\ContentInfo;
use eZ\Publish\API\Repository\Values\Content\LocationQuery;
Expand All @@ -15,13 +16,14 @@
use eZ\Publish\API\Repository\Values\Content\Search\SearchHit;
use eZ\Publish\API\Repository\Values\Content\Search\SearchResult;
use eZ\Publish\API\Repository\Values\User\Limitation\SubtreeLimitation;
use eZ\Publish\Core\Repository\Permission\PermissionResolver;
use eZ\Publish\Core\Repository\Values\Content\Location;
use EzSystems\EzPlatformAdminUi\Limitation\Mapper\UDWBasedMapper;
use PHPUnit\Framework\TestCase;

class UDWBasedMapperTest extends TestCase
{
public function testMapLimitationValue()
public function testMapLimitationValue(): void
{
$values = [5, 7, 11];
$expected = [
Expand All @@ -44,6 +46,8 @@ public function testMapLimitationValue()

$locationServiceMock = $this->createMock(LocationService::class);
$searchServiceMock = $this->createMock(SearchService::class);
$permissionResolverMock = $this->createMock(PermissionResolver::class);
$repositoryMock = $this->createMock(Repository::class);

foreach ($values as $i => $id) {
$location = new Location([
Expand All @@ -62,27 +66,32 @@ public function testMapLimitationValue()
]);

$searchServiceMock
->expects($this->at($i))
->expects(self::at($i))
->method('findLocations')
->with($query)
->willReturn($this->createSearchResultsMock($expected[$i]));
}

$mapper = new UDWBasedMapper($locationServiceMock, $searchServiceMock);
$mapper = new UDWBasedMapper(
$locationServiceMock,
$searchServiceMock,
$permissionResolverMock,
$repositoryMock
);
$result = $mapper->mapLimitationValue(new SubtreeLimitation([
'limitationValues' => $values,
]));

$this->assertEquals($expected, $result);
self::assertEquals($expected, $result);
}

private function createSearchResultsMock($expected)
private function createSearchResultsMock(array $expected): SearchResult
{
$hits = [];
foreach ($expected as $contentInfo) {
$locationMock = $this->createMock(Location::class);
$locationMock
->expects($this->atLeastOnce())
->expects(self::atLeastOnce())
->method('getContentInfo')
->willReturn($contentInfo);

Expand Down