From faef4d70a84e782cf646d926d761c0a9733eac19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Mon, 14 Sep 2026 08:25:05 +0200 Subject: [PATCH] feat: add two- and four-weekly series and drop the weekday picker --- assets/controllers/bookingform_controller.js | 11 ++- migrations/Version20260914090000.php | 35 +++++++++ src/Booking/RecurrenceRule.php | 33 +++++++++ src/Controller/BookingController.php | 3 +- src/Entity/BookingSeries.php | 42 +++-------- src/Form/BookingRequest.php | 7 -- src/Form/BookingType.php | 8 --- templates/booking/_fields.html.twig | 4 -- tests/Booking/BookingManagerTest.php | 75 ++++++++++++++------ tests/Controller/BookingControllerTest.php | 51 +++++++------ 10 files changed, 166 insertions(+), 103 deletions(-) create mode 100644 migrations/Version20260914090000.php diff --git a/assets/controllers/bookingform_controller.js b/assets/controllers/bookingform_controller.js index 7aca69c..7621cb5 100644 --- a/assets/controllers/bookingform_controller.js +++ b/assets/controllers/bookingform_controller.js @@ -2,24 +2,21 @@ import { Controller } from '@hotwired/stimulus'; /* * Shows and hides the parts of the booking form that only apply in some states: the time fields - * when "ganzer Tag" is off, the recurrence fields when a repetition is picked, and the weekday - * boxes only for a weekly rule. + * when "ganzer Tag" is off, and the recurrence fields when a repetition is picked. The weekday a + * series runs on is derived from its start date, so there is nothing to toggle for it. * * Purely client-side, so it is Stimulus rather than an htmx form refresh -- there is nothing the * server needs to decide here. The server still validates every one of these rules, because a * hidden field is not an absent one. */ export default class extends Controller { - static targets = ['times', 'repeat', 'weekdays']; + static targets = ['times', 'repeat']; toggleTimes(event) { this.timesTarget.classList.toggle('hidden', event.target.checked); } toggleRepeat(event) { - const rule = event.target.value; - - this.repeatTarget.classList.toggle('hidden', rule === ''); - this.weekdaysTarget.classList.toggle('hidden', rule !== 'weekly'); + this.repeatTarget.classList.toggle('hidden', event.target.value === ''); } } diff --git a/migrations/Version20260914090000.php b/migrations/Version20260914090000.php new file mode 100644 index 0000000..61d3f7d --- /dev/null +++ b/migrations/Version20260914090000.php @@ -0,0 +1,35 @@ +addSql('ALTER TABLE booking_series DROP weekdays'); + } + + public function down(Schema $schema): void + { + // Lossy on purpose, and not fixable: which weekdays a multi-day weekly series used to + // run on is gone once the column is dropped. Every restored row comes back as "[]", + // which the old code read as "every calendar day" for DAILY and "no days at all" for + // WEEKLY -- so a rolled-back weekly series would materialise nothing. + $this->addSql("ALTER TABLE booking_series ADD weekdays JSON NOT NULL DEFAULT '[]'"); + $this->addSql('ALTER TABLE booking_series ALTER weekdays DROP DEFAULT'); + } +} diff --git a/src/Booking/RecurrenceRule.php b/src/Booking/RecurrenceRule.php index d6f5eff..5e6fd22 100644 --- a/src/Booking/RecurrenceRule.php +++ b/src/Booking/RecurrenceRule.php @@ -4,16 +4,49 @@ declare(strict_types=1); namespace App\Booking; +/** + * How often a series repeats. + * + * Every rule is a whole number of days, which is what lets BookingSeries::occurrences() be a + * plain stride from the start date. Every stride that is a multiple of 7 therefore keeps the + * weekday fixed by construction -- that is why a series needs no weekday field at all. + * + * There is deliberately no calendar-monthly rule: "the 31st of each month" has no counterpart in + * five months of the year, and it would not hold the weekday. FOUR_WEEKLY is the nearest thing + * that behaves, at the cost of drifting against the calendar -- 13 occurrences a year, not 12, + * which is why it is not labelled "Monatlich". + */ enum RecurrenceRule: string { case DAILY = 'daily'; case WEEKLY = 'weekly'; + case TWO_WEEKLY = 'two_weekly'; + case FOUR_WEEKLY = 'four_weekly'; public function label(): string { return match ($this) { self::DAILY => 'Täglich', self::WEEKLY => 'Wöchentlich', + self::TWO_WEEKLY => 'Alle zwei Wochen', + self::FOUR_WEEKLY => 'Alle vier Wochen', }; } + + /** Days between two occurrences. */ + public function intervalDays(): int + { + return match ($this) { + self::DAILY => 1, + self::WEEKLY => 7, + self::TWO_WEEKLY => 14, + self::FOUR_WEEKLY => 28, + }; + } + + /** Whether the rule lands on the same weekday every time, i.e. whether naming it adds anything. */ + public function keepsWeekday(): bool + { + return 0 === $this->intervalDays() % 7; + } } diff --git a/src/Controller/BookingController.php b/src/Controller/BookingController.php index 25c009c..73827d5 100644 --- a/src/Controller/BookingController.php +++ b/src/Controller/BookingController.php @@ -132,7 +132,7 @@ final class BookingController extends AbstractController ]); // The day and any recurrence belong to the original booking; only the span is editable // here, so a series is never silently rewritten by editing one of its occurrences. - $form->remove('date')->remove('repeat')->remove('weekdays')->remove('until'); + $form->remove('date')->remove('repeat')->remove('until'); $form->handleRequest($request); if ($form->isSubmitted() && $form->isValid()) { @@ -207,7 +207,6 @@ final class BookingController extends AbstractController $data->date, $data->until, $data->timeRange(), - $data->weekdays, )); } diff --git a/src/Entity/BookingSeries.php b/src/Entity/BookingSeries.php index 4465833..a6fb065 100644 --- a/src/Entity/BookingSeries.php +++ b/src/Entity/BookingSeries.php @@ -41,15 +41,6 @@ class BookingSeries #[ORM\Column(enumType: RecurrenceRule::class)] private RecurrenceRule $rule; - /** - * ISO weekdays (1 = Monday) the series falls on. Empty for a DAILY rule, which means - * every calendar day. - * - * @var list - */ - #[ORM\Column] - private array $weekdays = []; - #[ORM\Column(type: 'date_immutable')] private \DateTimeImmutable $startDate; @@ -69,9 +60,6 @@ class BookingSeries #[ORM\OneToMany(targetEntity: Booking::class, mappedBy: 'series')] private Collection $bookings; - /** - * @param list $weekdays - */ public function __construct( User $user, Resource $resource, @@ -79,7 +67,6 @@ class BookingSeries \DateTimeImmutable $startDate, \DateTimeImmutable $endDate, TimeRange $time, - array $weekdays = [], ) { $this->user = $user; $this->resource = $resource; @@ -88,7 +75,6 @@ class BookingSeries $this->endDate = $endDate->setTime(0, 0); $this->startMinute = $time->startMinute; $this->endMinute = $time->endMinute; - $this->weekdays = RecurrenceRule::WEEKLY === $rule ? array_values(array_unique($weekdays)) : []; $this->createdAt = new \DateTimeImmutable(); $this->bookings = new ArrayCollection(); } @@ -96,17 +82,18 @@ class BookingSeries /** * Every day this series falls on, in order. * + * A rule is only a step size, so this is a stride from the start date. For any rule whose + * interval is a multiple of a week that means every occurrence lands on the same weekday as + * the start -- the series never has to be told which weekday it runs on. + * * @return list<\DateTimeImmutable> */ public function occurrences(): array { $dates = []; + $step = \sprintf('+%d days', $this->rule->intervalDays()); - for ($day = $this->startDate; $day <= $this->endDate; $day = $day->modify('+1 day')) { - if (RecurrenceRule::WEEKLY === $this->rule && !\in_array((int) $day->format('N'), $this->weekdays, true)) { - continue; - } - + for ($day = $this->startDate; $day <= $this->endDate; $day = $day->modify($step)) { $dates[] = $day; } @@ -133,14 +120,6 @@ class BookingSeries return $this->rule; } - /** - * @return list - */ - public function getWeekdays(): array - { - return $this->weekdays; - } - public function getStartDate(): \DateTimeImmutable { return $this->startDate; @@ -178,12 +157,11 @@ class BookingSeries { $description = $this->rule->label(); - if (RecurrenceRule::WEEKLY === $this->rule && [] !== $this->weekdays) { + // Naming the weekday only says something for a rule that actually holds it fixed; for a + // daily series it would just repeat the start date back at the reader. + if ($this->rule->keepsWeekday()) { $names = ['', 'Mo', 'Di', 'Mi', 'Do', 'Fr', 'Sa', 'So']; - // Sort the numbers, not the names -- alphabetical would read "Di, Do, Fr, Mi, Mo". - $weekdays = $this->weekdays; - sort($weekdays); - $description .= ' ('.implode(', ', array_map(static fn (int $day): string => $names[$day], $weekdays)).')'; + $description .= ' ('.$names[(int) $this->startDate->format('N')].')'; } return $description.', '.$this->getTimeRange(); diff --git a/src/Form/BookingRequest.php b/src/Form/BookingRequest.php index 332ef9b..785d3b5 100644 --- a/src/Form/BookingRequest.php +++ b/src/Form/BookingRequest.php @@ -28,9 +28,6 @@ final class BookingRequest public ?RecurrenceRule $repeat = null; - /** @var list ISO weekdays, 1 = Monday */ - public array $weekdays = []; - public ?\DateTimeImmutable $until = null; public function __construct(?\DateTimeImmutable $date = null, ?TimeRange $time = null) @@ -92,10 +89,6 @@ final class BookingRequest return; } - if (RecurrenceRule::WEEKLY === $this->repeat && [] === $this->weekdays) { - $context->buildViolation('Bitte mindestens einen Wochentag wählen.')->atPath('weekdays')->addViolation(); - } - if (null === $this->until) { $context->buildViolation('Bitte ein Enddatum für die Serie angeben.')->atPath('until')->addViolation(); diff --git a/src/Form/BookingType.php b/src/Form/BookingType.php index eec84c4..743dffc 100644 --- a/src/Form/BookingType.php +++ b/src/Form/BookingType.php @@ -7,7 +7,6 @@ namespace App\Form; use App\Booking\RecurrenceRule; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type\CheckboxType; -use Symfony\Component\Form\Extension\Core\Type\ChoiceType; use Symfony\Component\Form\Extension\Core\Type\DateType; use Symfony\Component\Form\Extension\Core\Type\EnumType; use Symfony\Component\Form\Extension\Core\Type\TimeType; @@ -49,13 +48,6 @@ final class BookingType extends AbstractType 'placeholder' => 'Einmalig', 'choice_label' => static fn (RecurrenceRule $rule): string => $rule->label(), ]) - ->add('weekdays', ChoiceType::class, [ - 'label' => 'Wochentage', - 'choices' => ['Mo' => 1, 'Di' => 2, 'Mi' => 3, 'Do' => 4, 'Fr' => 5, 'Sa' => 6, 'So' => 7], - 'multiple' => true, - 'expanded' => true, - 'required' => false, - ]) ->add('until', DateType::class, [ 'label' => 'Serie bis', 'widget' => 'single_text', diff --git a/templates/booking/_fields.html.twig b/templates/booking/_fields.html.twig index e20a7f0..616fa4d 100644 --- a/templates/booking/_fields.html.twig +++ b/templates/booking/_fields.html.twig @@ -27,10 +27,6 @@
-
- {{ form_row(form.weekdays, { row_attr: { class: 'pb-2' } }) }} -
{{ form_row(form.until) }}

Belegte Tage werden übersprungen und danach aufgelistet. diff --git a/tests/Booking/BookingManagerTest.php b/tests/Booking/BookingManagerTest.php index 0731bdb..ac35e32 100644 --- a/tests/Booking/BookingManagerTest.php +++ b/tests/Booking/BookingManagerTest.php @@ -101,27 +101,62 @@ class BookingManagerTest extends KernelTestCase public function testASeriesMaterialisesOneBookingPerOccurrence(): void { - // 2026-10-05 is a Monday. - $series = new BookingSeries( - $this->user('a@ep-reisen.de'), - $this->resource('D1'), - RecurrenceRule::WEEKLY, - new \DateTimeImmutable('2026-10-05'), - new \DateTimeImmutable('2026-10-30'), - TimeRange::fromMinutes(540, 720), - [1, 4], - ); + // 2026-10-05 is a Monday; the Mondays up to the 30th are the 5th, 12th, 19th and 26th. + $series = $this->weeklySeries(); $result = $this->manager->bookSeries($series); - self::assertSame(8, $result->createdCount()); + self::assertSame(4, $result->createdCount()); self::assertSame(0, $result->skippedCount()); foreach ($result->created as $booking) { - self::assertContains((int) $booking->getDate()->format('N'), [1, 4]); + self::assertSame(1, (int) $booking->getDate()->format('N')); self::assertSame($series, $booking->getSeries()); } } + public function testATwoWeeklySeriesSkipsEveryOtherWeek(): void + { + $result = $this->manager->bookSeries(new BookingSeries( + $this->user('a@ep-reisen.de'), + $this->resource('D1'), + RecurrenceRule::TWO_WEEKLY, + new \DateTimeImmutable('2026-10-05'), + new \DateTimeImmutable('2026-11-30'), + TimeRange::fromMinutes(540, 720), + )); + + self::assertSame(5, $result->createdCount()); + self::assertSame( + ['2026-10-05', '2026-10-19', '2026-11-02', '2026-11-16', '2026-11-30'], + array_map(static fn (Booking $booking): string => $booking->getDate()->format('Y-m-d'), $result->created), + ); + } + + /** + * The property the weekday field was removed in favour of: a stride that is a whole number of + * weeks lands on the same weekday every time, so nothing has to store which one it is. A + * four-weekly series is the one where this is easy to get wrong by reaching for months. + */ + public function testAFourWeeklySeriesAlwaysLandsOnTheSameWeekday(): void + { + $result = $this->manager->bookSeries(new BookingSeries( + $this->user('a@ep-reisen.de'), + $this->resource('D1'), + RecurrenceRule::FOUR_WEEKLY, + new \DateTimeImmutable('2026-10-05'), + new \DateTimeImmutable('2026-12-31'), + TimeRange::fromMinutes(540, 720), + )); + + self::assertSame(4, $result->createdCount()); + foreach ($result->created as $booking) { + self::assertSame(1, (int) $booking->getDate()->format('N'), 'every occurrence is a Monday'); + } + // Crossing a month boundary is where a "monthly" reading would diverge: 28 days after + // 2 November is 30 November, not 2 December. + self::assertSame('2026-11-30', $result->created[2]->getDate()->format('Y-m-d')); + } + public function testADailySeriesCoversEveryDay(): void { $result = $this->manager->bookSeries(new BookingSeries( @@ -142,7 +177,8 @@ class BookingManagerTest extends KernelTestCase public function testASeriesSkipsDaysThatAreAlreadyTaken(): void { $desk = $this->resource('D1'); - $this->manager->book($this->user('b@ep-reisen.de'), $desk, new \DateTimeImmutable('2026-10-08'), TimeRange::fromMinutes(600, 660)); + // The 12th is one of the series' own Mondays. + $this->manager->book($this->user('b@ep-reisen.de'), $desk, new \DateTimeImmutable('2026-10-12'), TimeRange::fromMinutes(600, 660)); $result = $this->manager->bookSeries(new BookingSeries( $this->user('a@ep-reisen.de'), @@ -151,11 +187,10 @@ class BookingManagerTest extends KernelTestCase new \DateTimeImmutable('2026-10-05'), new \DateTimeImmutable('2026-10-30'), TimeRange::fromMinutes(540, 720), - [1, 4], )); - self::assertSame(7, $result->createdCount()); - self::assertSame(['8.10.'], $result->skippedDates()); + self::assertSame(3, $result->createdCount()); + self::assertSame(['12.10.'], $result->skippedDates()); self::assertFalse($result->isCompletelyBlocked()); } @@ -232,7 +267,7 @@ class BookingManagerTest extends KernelTestCase $removed = $this->manager->endSeries($series, new \DateTimeImmutable('2026-10-05')); - self::assertSame(8, $removed); + self::assertSame(4, $removed); self::assertSame([], $this->em->getRepository(Booking::class)->findBy(['series' => $id])); // Nothing of the series is left, so the series row goes too -- note $series has no id // any more at this point, which is why the lookup goes through the captured one. @@ -244,10 +279,11 @@ class BookingManagerTest extends KernelTestCase $series = $this->weeklySeries(); $this->manager->bookSeries($series); + // Leaves the Mondays on the 5th and 12th, removes the 19th and 26th. $removed = $this->manager->endSeries($series, new \DateTimeImmutable('2026-10-15')); - self::assertSame(5, $removed); - self::assertCount(3, $this->em->getRepository(Booking::class)->findBy(['series' => $series])); + self::assertSame(2, $removed); + self::assertCount(2, $this->em->getRepository(Booking::class)->findBy(['series' => $series])); self::assertSame('2026-10-14', $series->getEndDate()->format('Y-m-d')); } @@ -260,7 +296,6 @@ class BookingManagerTest extends KernelTestCase new \DateTimeImmutable('2026-10-05'), new \DateTimeImmutable('2026-10-30'), TimeRange::fromMinutes(540, 720), - [1, 4], ); } diff --git a/tests/Controller/BookingControllerTest.php b/tests/Controller/BookingControllerTest.php index ec49b35..6060b1a 100644 --- a/tests/Controller/BookingControllerTest.php +++ b/tests/Controller/BookingControllerTest.php @@ -322,18 +322,37 @@ class BookingControllerTest extends WebTestCase $this->client->loginUser($this->user('anna@ep-reisen.de')); $desk = $this->resource('D1'); - // 2026-10-05 is a Monday. + // 2026-10-05 is a Monday, and the weekday is taken from it rather than asked for -- + // the Mondays up to the 30th are the 5th, 12th, 19th and 26th. $response = $this->submitBookingModal($desk, '2026-10-05', [ 'startTime' => '09:00', 'endTime' => '12:00', 'repeat' => 'weekly', - 'weekdays' => ['1', '4'], 'until' => '2026-10-30', ]); self::assertSame(Response::HTTP_OK, $response->getStatusCode()); - self::assertCount(8, $this->em->getRepository(Booking::class)->findBy(['resource' => $desk])); - self::assertStringContainsString('8 Termine gebucht', (string) $response->getContent()); + $occurrences = $this->em->getRepository(Booking::class)->findBy(['resource' => $desk]); + self::assertCount(4, $occurrences); + self::assertStringContainsString('4 Termine gebucht', (string) $response->getContent()); + foreach ($occurrences as $booking) { + self::assertSame(1, (int) $booking->getDate()->format('N')); + } + } + + public function testAFourWeeklySeriesIsCreatedAndReported(): void + { + $this->client->loginUser($this->user('anna@ep-reisen.de')); + $desk = $this->resource('D1'); + + $response = $this->submitBookingModal($desk, '2026-10-05', [ + 'allDay' => '1', + 'repeat' => 'four_weekly', + 'until' => '2026-12-31', + ]); + + self::assertSame(Response::HTTP_OK, $response->getStatusCode()); + self::assertCount(4, $this->em->getRepository(Booking::class)->findBy(['resource' => $desk])); } public function testASeriesWithoutAnEndDateIsRejected(): void @@ -349,20 +368,6 @@ class BookingControllerTest extends WebTestCase self::assertStringContainsString('Bitte ein Enddatum für die Serie angeben.', (string) $response->getContent()); } - public function testAWeeklySeriesWithoutWeekdaysIsRejected(): void - { - $this->client->loginUser($this->user('anna@ep-reisen.de')); - - $response = $this->submitBookingModal($this->resource('D1'), '2026-10-05', [ - 'allDay' => '1', - 'repeat' => 'weekly', - 'until' => '2026-10-30', - ]); - - self::assertSame(Response::HTTP_UNPROCESSABLE_ENTITY, $response->getStatusCode()); - self::assertStringContainsString('Bitte mindestens einen Wochentag wählen.', (string) $response->getContent()); - } - public function testASeriesRunningTooFarOutIsRejected(): void { $this->client->loginUser($this->user('anna@ep-reisen.de')); @@ -417,7 +422,6 @@ class BookingControllerTest extends WebTestCase 'startTime' => '09:00', 'endTime' => '12:00', 'repeat' => 'weekly', - 'weekdays' => ['1'], 'until' => '2026-10-26', ]); @@ -451,10 +455,11 @@ class BookingControllerTest extends WebTestCase /** * Renders the modal, reads its CSRF token, and posts the given booking fields back to it. * - * Posted explicitly rather than through DomCrawler's form(): the weekday checkboxes are an - * expanded multiple choice rendered as booking[weekdays][], which the crawler's field - * registry indexes positionally, so assigning ['1', '4'] sets the second checkbox instead of - * the fourth. The token still comes off the rendered form, so the round trip is real. + * Posted explicitly rather than through DomCrawler's form(), which lets a test name only the + * fields it cares about and leave the rest at their rendered defaults. The token still comes + * off the rendered form, so the round trip is real. (This originally worked around the + * weekday checkboxes, which the crawler indexed positionally; they are gone, but posting + * directly is still the simpler way to drive the modal.) * * @param array $fields */