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

Revert "chore(performance): use subresources & uriTemplate property to avoid …" #4963

Merged
merged 1 commit into from
Apr 14, 2024
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
21 changes: 2 additions & 19 deletions api/src/Entity/Day.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
use ApiPlatform\Metadata\ApiResource;
use ApiPlatform\Metadata\Get;
use ApiPlatform\Metadata\GetCollection;
use ApiPlatform\Metadata\Link;
use App\Repository\DayRepository;
use App\Serializer\Normalizer\RelatedCollectionLink;
use Doctrine\Common\Collections\ArrayCollection;
Expand All @@ -34,13 +33,6 @@
normalizationContext: self::COLLECTION_NORMALIZATION_CONTEXT,
security: 'is_authenticated()'
),
new GetCollection(
uriTemplate: self::PERIOD_SUBRESOURCE_URI_TEMPLATE,
uriVariables: [
'periodId' => new Link(toProperty: 'period', fromClass: Period::class),
],
security: 'is_fully_authenticated()',
),
],
denormalizationContext: ['groups' => ['write']],
normalizationContext: ['groups' => ['read']],
Expand All @@ -51,8 +43,6 @@
#[ORM\Entity(repositoryClass: DayRepository::class)]
#[ORM\UniqueConstraint(name: 'offset_period_idx', columns: ['periodId', 'dayOffset'])]
class Day extends BaseEntity implements BelongsToCampInterface {
public const PERIOD_SUBRESOURCE_URI_TEMPLATE = '/periods/{periodId}/days{._format}';

public const ITEM_NORMALIZATION_CONTEXT = [
'groups' => [
'read',
Expand All @@ -71,11 +61,7 @@ class Day extends BaseEntity implements BelongsToCampInterface {
/**
* The list of people who have a whole-day responsibility on this day.
*/
#[ApiProperty(
writable: false,
uriTemplate: DayResponsible::DAY_SUBRESOURCE_URI_TEMPLATE,
example: '/days/1a2b3c4d/day_responsibles'
)]
#[ApiProperty(writable: false, example: '["/day_responsibles/1a2b3c4d"]')]
#[Groups(['read'])]
#[ORM\OneToMany(targetEntity: DayResponsible::class, mappedBy: 'day', orphanRemoval: true)]
public Collection $dayResponsibles;
Expand Down Expand Up @@ -175,10 +161,7 @@ public function getEnd(): ?\DateTime {
/**
* @return DayResponsible[]
*/
#[ApiProperty(
readableLink: true,
uriTemplate: DayResponsible::DAY_SUBRESOURCE_URI_TEMPLATE,
)]
#[ApiProperty(readableLink: true)]
#[SerializedName('dayResponsibles')]
#[Groups(['Day:DayResponsibles'])]
public function getEmbeddedDayResponsibles(): array {
Expand Down
10 changes: 0 additions & 10 deletions api/src/Entity/DayResponsible.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
use ApiPlatform\Metadata\Delete;
use ApiPlatform\Metadata\Get;
use ApiPlatform\Metadata\GetCollection;
use ApiPlatform\Metadata\Link;
use ApiPlatform\Metadata\Post;
use App\Repository\DayResponsibleRepository;
use App\Validator\AssertBelongsToSameCamp;
Expand All @@ -32,13 +31,6 @@
new GetCollection(
security: 'is_authenticated()'
),
new GetCollection(
uriTemplate: self::DAY_SUBRESOURCE_URI_TEMPLATE,
uriVariables: [
'dayId' => new Link(toProperty: 'day', fromClass: Day::class),
],
security: 'is_fully_authenticated()',
),
new Post(
securityPostDenormalize: 'is_granted("CAMP_MEMBER", object) or is_granted("CAMP_MANAGER", object)'
),
Expand All @@ -54,8 +46,6 @@
#[ORM\Entity(repositoryClass: DayResponsibleRepository::class)]
#[ORM\UniqueConstraint(name: 'day_campCollaboration_unique', columns: ['dayId', 'campCollaborationId'])]
class DayResponsible extends BaseEntity implements BelongsToCampInterface {
public const DAY_SUBRESOURCE_URI_TEMPLATE = '/days/{dayId}/day_responsibles{._format}';

/**
* The day on which the person is responsible.
*/
Expand Down
17 changes: 3 additions & 14 deletions api/src/Entity/Period.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,7 @@ class Period extends BaseEntity implements BelongsToCampInterface {
/**
* The days in this time period. These are generated automatically.
*/
#[ApiProperty(
writable: false,
uriTemplate: Day::PERIOD_SUBRESOURCE_URI_TEMPLATE,
example: '/periods/1a2b3c4d/days'
)]
#[ApiProperty(writable: false, example: '["/days?period=/periods/1a2b3c4d"]')]
#[Groups(['read'])]
#[ORM\OneToMany(targetEntity: Day::class, mappedBy: 'period', orphanRemoval: true, cascade: ['persist'])]
#[ORM\OrderBy(['dayOffset' => 'ASC'])]
Expand All @@ -84,11 +80,7 @@ class Period extends BaseEntity implements BelongsToCampInterface {
*
* @var Collection<int, ScheduleEntry>
*/
#[ApiProperty(
writable: false,
uriTemplate: ScheduleEntry::PERIOD_SUBRESOURCE_URI_TEMPLATE,
example: '/periods/1a2b3c4d/schedule_entries'
)]
#[ApiProperty(writable: false, example: '["/schedule_entries/1a2b3c4d"]')]
#[Groups(['read'])]
#[ORM\OneToMany(targetEntity: ScheduleEntry::class, mappedBy: 'period')]
#[ORM\OrderBy(['startOffset' => 'ASC', 'left' => 'ASC', 'endOffset' => 'DESC', 'id' => 'ASC'])]
Expand Down Expand Up @@ -186,10 +178,7 @@ public function __construct() {
/**
* @return Day[]
*/
#[ApiProperty(
readableLink: true,
uriTemplate: Day::PERIOD_SUBRESOURCE_URI_TEMPLATE,
)]
#[ApiProperty(readableLink: true)]
#[SerializedName('days')]
#[Groups('Period:Days')]
public function getEmbeddedDays(): array {
Expand Down
10 changes: 0 additions & 10 deletions api/src/Entity/ScheduleEntry.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
use ApiPlatform\Metadata\Delete;
use ApiPlatform\Metadata\Get;
use ApiPlatform\Metadata\GetCollection;
use ApiPlatform\Metadata\Link;
use ApiPlatform\Metadata\Patch;
use ApiPlatform\Metadata\Post;
use App\Doctrine\Filter\ExpressionDateTimeFilter;
Expand Down Expand Up @@ -43,13 +42,6 @@
new GetCollection(
security: 'is_authenticated()'
),
new GetCollection(
uriTemplate: self::PERIOD_SUBRESOURCE_URI_TEMPLATE,
uriVariables: [
'periodId' => new Link(toProperty: 'period', fromClass: Period::class),
],
security: 'is_fully_authenticated()',
),
new Post(
denormalizationContext: ['groups' => ['write', 'create']],
normalizationContext: self::ITEM_NORMALIZATION_CONTEXT,
Expand All @@ -69,8 +61,6 @@
#[ORM\Index(columns: ['startOffset'])]
#[ORM\Index(columns: ['endOffset'])]
class ScheduleEntry extends BaseEntity implements BelongsToCampInterface {
public const PERIOD_SUBRESOURCE_URI_TEMPLATE = '/periods/{periodId}/schedule_entries{._format}';

public const ITEM_NORMALIZATION_CONTEXT = [
'groups' => ['read', 'ScheduleEntry:Activity'],
'swagger_definition_name' => 'read',
Expand Down
30 changes: 0 additions & 30 deletions api/tests/Api/DayResponsibles/ListDayResponsiblesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,34 +101,4 @@ public function testListDayResponsiblesFilteredByDayInCampPrototypeIsAllowedForU
['href' => $this->getIriFor('dayResponsible1day1period1campPrototype')],
], $response->toArray()['_links']['items']);
}

public function testListDayResponsiblesAsDaySubresourceIsAllowedForCollaborator() {
$day = static::getFixture('day1period1');
$response = static::createClientWithCredentials()->request('GET', '/days/'.$day->getId().'/day_responsibles');
$this->assertResponseStatusCodeSame(200);
$this->assertJsonContains([
'totalItems' => 1,
'_links' => [
'items' => [],
],
'_embedded' => [
'items' => [],
],
]);
$this->assertEqualsCanonicalizing([
['href' => $this->getIriFor('dayResponsible1')],
], $response->toArray()['_links']['items']);
}

public function testListDayResponsiblesAsDaySubresourceIsDeniedForUnrelatedUser() {
$day = static::getFixture('day1period1');
$response = static::createClientWithCredentials(['email' => static::$fixtures['user4unrelated']->getEmail()])
->request('GET', '/days/'.$day->getId().'/day_responsibles')
;

$this->assertResponseStatusCodeSame(200);

$this->assertJsonContains(['totalItems' => 0]);
$this->assertArrayNotHasKey('items', $response->toArray()['_links']);
}
}
32 changes: 0 additions & 32 deletions api/tests/Api/Days/ListDaysTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,36 +122,4 @@ public function testListDaysFilteredByPeriodInCampPrototypeIsAllowedForCollabora
['href' => $this->getIriFor('day1period1campPrototype')],
], $response->toArray()['_links']['items']);
}

public function testListDaysAsPeriodSubresourceIsAllowedForCollaborator() {
$period = static::getFixture('period1');
$response = static::createClientWithCredentials()->request('GET', '/periods/'.$period->getId().'/days');
$this->assertResponseStatusCodeSame(200);
$this->assertJsonContains([
'totalItems' => 3,
'_links' => [
'items' => [],
],
'_embedded' => [
'items' => [],
],
]);
$this->assertEqualsCanonicalizing([
['href' => $this->getIriFor('day1period1')],
['href' => $this->getIriFor('day2period1')],
['href' => $this->getIriFor('day3period1')],
], $response->toArray()['_links']['items']);
}

public function testListDaysAsPeriodSubresourceIsDeniedForUnrelatedUser() {
$period = static::getFixture('period1');
$response = static::createClientWithCredentials(['email' => static::$fixtures['user4unrelated']->getEmail()])
->request('GET', '/periods/'.$period->getId().'/days')
;

$this->assertResponseStatusCodeSame(200);

$this->assertJsonContains(['totalItems' => 0]);
$this->assertArrayNotHasKey('items', $response->toArray()['_links']);
}
}
8 changes: 4 additions & 4 deletions api/tests/Api/Days/ReadDayTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public function testGetSingleDayIsAllowedForGuest() {
'_links' => [
'period' => ['href' => $this->getIriFor('period1')],
'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$day->period->getId().'&start%5Bstrictly_before%5D='.urlencode($end).'&end%5Bafter%5D='.urlencode($start)],
'dayResponsibles' => ['href' => '/days/'.$day->getId().'/day_responsibles'],
'dayResponsibles' => ['href' => '/day_responsibles?day=%2Fdays%2F'.$day->getId()],
],
]);
}
Expand All @@ -83,7 +83,7 @@ public function testGetSingleDayIsAllowedForMember() {
'_links' => [
'period' => ['href' => $this->getIriFor('period1')],
'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$day->period->getId().'&start%5Bstrictly_before%5D='.urlencode($end).'&end%5Bafter%5D='.urlencode($start)],
'dayResponsibles' => ['href' => '/days/'.$day->getId().'/day_responsibles'],
'dayResponsibles' => ['href' => '/day_responsibles?day=%2Fdays%2F'.$day->getId()],
],
]);
}
Expand All @@ -102,7 +102,7 @@ public function testGetSingleDayIsAllowedForManager() {
'_links' => [
'period' => ['href' => $this->getIriFor('period1')],
'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$day->period->getId().'&start%5Bstrictly_before%5D='.urlencode($end).'&end%5Bafter%5D='.urlencode($start)],
'dayResponsibles' => ['href' => '/days/'.$day->getId().'/day_responsibles'],
'dayResponsibles' => ['href' => '/day_responsibles?day=%2Fdays%2F'.$day->getId()],
],
]);
}
Expand All @@ -121,7 +121,7 @@ public function testGetSingleDayFromCampPrototypeIsAllowedForUnrelatedUser() {
'_links' => [
'period' => ['href' => $this->getIriFor('period1campPrototype')],
'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$day->period->getId().'&start%5Bstrictly_before%5D='.urlencode($end).'&end%5Bafter%5D='.urlencode($start)],
'dayResponsibles' => ['href' => '/days/'.$day->getId().'/day_responsibles'],
'dayResponsibles' => ['href' => '/day_responsibles?day=%2Fdays%2F'.$day->getId()],
],
]);
}
Expand Down
12 changes: 6 additions & 6 deletions api/tests/Api/Periods/ReadPeriodTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ public function testGetSinglePeriodIsAllowedForGuest() {
'_links' => [
'camp' => ['href' => $this->getIriFor('camp1')],
'materialItems' => ['href' => '/material_items?period=%2Fperiods%2F'.$period->getId()],
'days' => ['href' => '/periods/'.$period->getId().'/days'],
'scheduleEntries' => ['href' => '/periods/'.$period->getId().'/schedule_entries'],
'days' => ['href' => '/days?period=%2Fperiods%2F'.$period->getId()],
'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$period->getId()],
'contentNodes' => ['href' => '/content_nodes?period=%2Fperiods%2F'.$period->getId()],
'dayResponsibles' => ['href' => '/day_responsibles?day.period=%2Fperiods%2F'.$period->getId()],
],
Expand All @@ -86,8 +86,8 @@ public function testGetSinglePeriodIsAllowedForMember() {
'_links' => [
'camp' => ['href' => $this->getIriFor('camp1')],
'materialItems' => ['href' => '/material_items?period=%2Fperiods%2F'.$period->getId()],
'days' => ['href' => '/periods/'.$period->getId().'/days'],
'scheduleEntries' => ['href' => '/periods/'.$period->getId().'/schedule_entries'],
'days' => ['href' => '/days?period=%2Fperiods%2F'.$period->getId()],
'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$period->getId()],
],
]);
}
Expand All @@ -105,8 +105,8 @@ public function testGetSinglePeriodIsAllowedForManager() {
'_links' => [
'camp' => ['href' => $this->getIriFor('camp1')],
'materialItems' => ['href' => '/material_items?period=%2Fperiods%2F'.$period->getId()],
'days' => ['href' => '/periods/'.$period->getId().'/days'],
'scheduleEntries' => ['href' => '/periods/'.$period->getId().'/schedule_entries'],
'days' => ['href' => '/days?period=%2Fperiods%2F'.$period->getId()],
'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$period->getId()],
],
]);
}
Expand Down
32 changes: 0 additions & 32 deletions api/tests/Api/ScheduleEntries/ListScheduleEntriesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -375,36 +375,4 @@ public function testListScheduleEntriesFilteredByInvalidEndDoesntFilter() {
['href' => $this->getIriFor('scheduleEntry2period1campPrototype')],
], $response->toArray()['_links']['items']);
}

public function testListScheduleEntriesAsPeriodSubresourceIsAllowedForCollaborator() {
$period = static::getFixture('period1');
$response = static::createClientWithCredentials()->request('GET', '/periods/'.$period->getId().'/schedule_entries');
$this->assertResponseStatusCodeSame(200);
$this->assertJsonContains([
'totalItems' => 3,
'_links' => [
'items' => [],
],
'_embedded' => [
'items' => [],
],
]);
$this->assertEqualsCanonicalizing([
['href' => $this->getIriFor('scheduleEntry1')],
['href' => $this->getIriFor('scheduleEntry2')],
['href' => $this->getIriFor('scheduleEntry1period1camp1')],
], $response->toArray()['_links']['items']);
}

public function testListScheduleEntriesAsPeriodSubresourceIsDeniedForUnrelatedUser() {
$period = static::getFixture('period1');
$response = static::createClientWithCredentials(['email' => static::$fixtures['user4unrelated']->getEmail()])
->request('GET', '/periods/'.$period->getId().'/schedule_entries')
;

$this->assertResponseStatusCodeSame(200);

$this->assertJsonContains(['totalItems' => 0]);
$this->assertArrayNotHasKey('items', $response->toArray()['_links']);
}
}
Loading
Loading