diff --git a/.env b/.env index 69ee53c..643c347 100644 --- a/.env +++ b/.env @@ -114,7 +114,6 @@ FEATURE_SANITIZE_UPLOADS=false FEATURE_STAMP_INVOICES=false FEATURE_DATEV_EMAIL=false FEATURE_MYEP_LOGIN=false -FEATURE_APPLICATION_FORM_TEAMER_PROVISIONING=false UPLOAD_REPLACEMENT_FILE=assets/pdf/chicken.pdf diff --git a/config/packages/flagception.yaml b/config/packages/flagception.yaml index 4c28445..4d658e4 100644 --- a/config/packages/flagception.yaml +++ b/config/packages/flagception.yaml @@ -13,8 +13,3 @@ flagception: myep_login: default: false env: FEATURE_MYEP_LOGIN - # Provisioning writes to the production CRM and cannot be rehearsed: the teamer selection - # is only changeable there. Ships off. - application_form_teamer_provisioning: - default: false - env: FEATURE_APPLICATION_FORM_TEAMER_PROVISIONING diff --git a/src/Controller/Admin/ApplicationFormSubmission/CreateTeamerController.php b/src/Controller/Admin/ApplicationFormSubmission/CreateTeamerController.php index 6f72eeb..8424369 100644 --- a/src/Controller/Admin/ApplicationFormSubmission/CreateTeamerController.php +++ b/src/Controller/Admin/ApplicationFormSubmission/CreateTeamerController.php @@ -7,7 +7,6 @@ use App\Htmx\HxRedirectResponse; use App\Model\TeamerProvisioningResult; use App\Service\Teamer\TeamerProvisioner; use App\Service\Teamer\TeamerProvisioningException; -use Flagception\Manager\FeatureManagerInterface; use Psr\Log\LoggerInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\Request; @@ -19,14 +18,14 @@ use Symfony\Component\Security\Http\Attribute\IsGranted; * Makes a teamer out of an applicant who passed the interview: one click, because the submission * carries everything the CRM needs and there is nothing left for the reviewer to type. * - * Behind a feature flag, because provisioning writes to the production CRM and cannot be rehearsed - * anywhere else - the teamer selection is only changeable there. + * The click writes to the CRM for real - three writes, none of them undoable from here - so what + * goes to BusPro is shown in the confirmation before it goes, and TeamerProvisioner refuses a + * submission it cannot provision before sending anything. */ class CreateTeamerController extends AbstractController { public function __construct( private readonly TeamerProvisioner $provisioner, - private readonly FeatureManagerInterface $featureManager, private readonly LoggerInterface $logger, ) { } @@ -35,10 +34,6 @@ class CreateTeamerController extends AbstractController #[IsGranted('ROLE_TEAM_ADMIN')] public function index(ApplicationFormSubmission $submission, Request $request): Response { - if (false === $this->featureManager->isActive('application_form_teamer_provisioning')) { - throw $this->createNotFoundException('Teamer provisioning is not enabled'); - } - if (false === $request->isMethod(Request::METHOD_POST)) { return $this->render('admin/application_form_submission/modal_create_teamer.html.twig', [ 'submission' => $submission, diff --git a/templates/admin/application_form_submission/detail.html.twig b/templates/admin/application_form_submission/detail.html.twig index 36945c4..32ae36d 100644 --- a/templates/admin/application_form_submission/detail.html.twig +++ b/templates/admin/application_form_submission/detail.html.twig @@ -106,41 +106,39 @@ {{ form_rest(callsForm) }} {{ form_end(callsForm) }} - {% if feature('application_form_teamer_provisioning') or submission.teamerCreatedAt %} -
-
- {{ _self.row('übernommen am', submission.teamerCreatedAt ? submission.teamerCreatedAt|date('d.m.Y, H:i') : null) }} -
-
- {% if submission.user and submission.user.teamer %} - - Teamer:innenprofil öffnen - - {% elseif submission.email %} - - {% if submission.busProAddressId %} - {# Step 1 is done and the rest is not: naming it here is what makes a - retry a resume rather than a guess. #} -
- Die BusPro-Adresse {{ submission.busProAddressId }} wurde schon angelegt, - die Übernahme wurde aber nicht abgeschlossen. -
- {% endif %} - {% else %} -
- Ohne E-Mail-Adresse kann kein Teamer:innenkonto angelegt werden. +
+
+ {{ _self.row('übernommen am', submission.teamerCreatedAt ? submission.teamerCreatedAt|date('d.m.Y, H:i') : null) }} +
+
+ {% if submission.user and submission.user.teamer %} + + Teamer:innenprofil öffnen + + {% elseif submission.email %} + + {% if submission.busProAddressId %} + {# Step 1 is done and the rest is not: naming it here is what makes a + retry a resume rather than a guess. #} +
+ Die BusPro-Adresse {{ submission.busProAddressId }} wurde schon angelegt, + die Übernahme wurde aber nicht abgeschlossen.
{% endif %} -
+ {% else %} +
+ Ohne E-Mail-Adresse kann kein Teamer:innenkonto angelegt werden. +
+ {% endif %}
- {% endif %} +

diff --git a/tests/Controller/Admin/ApplicationFormSubmission/CreateTeamerControllerTest.php b/tests/Controller/Admin/ApplicationFormSubmission/CreateTeamerControllerTest.php index a5b0198..4701f21 100644 --- a/tests/Controller/Admin/ApplicationFormSubmission/CreateTeamerControllerTest.php +++ b/tests/Controller/Admin/ApplicationFormSubmission/CreateTeamerControllerTest.php @@ -12,13 +12,13 @@ use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; use Symfony\Component\HttpFoundation\Response; /** - * The feature gate, which is the only part of provisioning a test can exercise: everything behind - * it writes to the production CRM, where the teamer selection is changeable - the test database - * answers aenderbar="False" for it (.ddev/plans/buspro-crm-selection-write.md). + * Everything around the provisioning click that can be exercised without writing to BusPro. * - * With the flag off the route must not exist at all, not merely hide its button: the modal is - * reachable by url, and a reviewer who bookmarked it would otherwise write to BusPro from a - * release where the feature is meant to be dormant. + * The write itself cannot: the teamer CRM selection is only changeable in the production database + * (.ddev/plans/buspro-crm-selection-write.md), and a POST that gets as far as the api client would + * open a socket to it from a test. TeamerProvisionerTest covers the flow against a mocked client; + * what is left here is the route, the confirmation and the one POST that is safe to make - a + * submission TeamerProvisioner refuses before it sends anything. */ class CreateTeamerControllerTest extends WebTestCase { @@ -37,51 +37,115 @@ class CreateTeamerControllerTest extends WebTestCase $this->client->loginUser($this->reviewer()); } - public function testTheDisabledFeatureHidesTheRouteEntirely(): void + public function testTheDetailPageOffersTheProvisioningButton(): void { - $this->client->request('GET', $this->url()); + $submission = $this->submission(); - $this->assertSame(Response::HTTP_NOT_FOUND, $this->client->getResponse()->getStatusCode()); - } - - /** - * The POST is the one that writes, so it is pinned separately rather than trusted to share the - * guard with the GET. - */ - public function testTheDisabledFeatureRefusesThePostAsWell(): void - { - $this->client->request('POST', $this->url()); - - $this->assertSame(Response::HTTP_NOT_FOUND, $this->client->getResponse()->getStatusCode()); - $this->assertNull($this->submission()->getTeamerCreatedAt()); - } - - public function testTheDetailPageOffersNoProvisioningWhileTheFeatureIsOff(): void - { $crawler = $this->client->request( 'GET', - sprintf('/admin/application-form-submission/detail/%s', $this->submission()->getUuid()) + sprintf('/admin/application-form-submission/detail/%s', $submission->getUuid()), + ['r' => rawurlencode('/admin/application-form-submission')] ); $this->assertSame(Response::HTTP_OK, $this->client->getResponse()->getStatusCode()); - $this->assertCount(0, $crawler->filter('button[hx-get*="create-teamer"]')); + + $button = $crawler->filter('button[hx-get*="create-teamer"]'); + + $this->assertCount(1, $button); + // Like every other action on that page: it hands on the return url it was given, so "zurück" + // keeps pointing at the list. + $this->assertStringContainsString('r='.rawurlencode('/admin/application-form-submission'), (string) $button->attr('hx-get')); } - private function url(): string + /** + * The three writes cannot be undone from here, so the confirmation names what goes to BusPro + * rather than asking "are you sure". + */ + public function testTheConfirmationNamesWhatGoesToBusPro(): void { - return sprintf('/admin/application-form-submission/create-teamer/%s', $this->submission()->getUuid()); + $crawler = $this->client->request('GET', $this->url($this->submission())); + + $this->assertSame(Response::HTTP_OK, $this->client->getResponse()->getStatusCode()); + + $modal = $crawler->filter('#htmx-confirmation-modal')->text(); + + $this->assertStringContainsString('Anna Muster', $modal); + $this->assertStringContainsString('01.01.1990', $modal); + $this->assertStringContainsString('Musterstraße 1, 10115 Berlin', $modal); + } + + /** + * A row from before the tightened endpoint contract. The confirmation says so before the + * reviewer clicks, because BusPro would answer the same thing only after the attempt. + */ + public function testTheConfirmationFlagsAnIncompleteSubmission(): void + { + $crawler = $this->client->request('GET', $this->url($this->incompleteSubmission())); + + $modal = $crawler->filter('#htmx-confirmation-modal')->text(); + + $this->assertStringContainsString('ohne Geburtsdatum', $modal); + $this->assertStringContainsString('ohne Straße, PLZ und Ort', $modal); + } + + /** + * The only POST a test may make: TeamerProvisioner refuses an unprovisionable submission before + * it touches the api client, so nothing here reaches BusPro. It covers what the reviewer + * actually sees when provisioning fails - the refusal's own wording, not "es ist ein Fehler + * aufgetreten". + */ + public function testAnUnprovisionableSubmissionFailsWithoutWritingAnything(): void + { + $submission = $this->incompleteSubmission(); + + $this->client->request('POST', $this->url($submission)); + + $response = $this->client->getResponse(); + + $this->assertSame(Response::HTTP_OK, $response->getStatusCode()); + $this->assertStringContainsString('detail', (string) $response->headers->get('HX-Redirect')); + + $flashes = $this->client->getRequest()->getSession()->getFlashBag()->peekAll(); + + $this->assertArrayHasKey('error', $flashes); + $this->assertStringContainsString('Geburtsdatum', $flashes['error'][0]); + + $this->entityManager->clear(); + $stored = $this->entityManager->getRepository(ApplicationFormSubmission::class)->find($submission->getId()); + + $this->assertNotNull($stored); + $this->assertFalse($stored->isProvisioned()); + $this->assertNull($stored->getBusProAddressId()); + $this->assertNull($stored->getUser()); + } + + private function url(ApplicationFormSubmission $submission): string + { + return sprintf('/admin/application-form-submission/create-teamer/%s', $submission->getUuid()); } private function submission(): ApplicationFormSubmission { - $repository = $this->entityManager->getRepository(ApplicationFormSubmission::class); - $submission = $repository->findOneBy([], ['id' => 'DESC']); + return $this->persist( + $this->newSubmission() + ->setBirthDate('1990-01-01') + ->setStreet('Musterstraße 1') + ->setZipCode('10115') + ->setCity('Berlin') + ); + } - if (null !== $submission) { - return $submission; - } + /** + * Everything the contract now requires, missing - which only a row stored before it can be. + */ + private function incompleteSubmission(): ApplicationFormSubmission + { + return $this->persist($this->newSubmission()->setBirthDate('01.01.1990')); + } - $submission = (new ApplicationFormSubmission( + private function newSubmission(): ApplicationFormSubmission + { + return (new ApplicationFormSubmission( 'applicationform', 'Bewerbungsformular', new \DateTimeImmutable('2026-09-22T12:30:00+02:00'), @@ -90,9 +154,11 @@ class CreateTeamerControllerTest extends WebTestCase ->setFirstName('Anna') ->setLastName('Muster') ->setEmail('anna@example.com') - ->setBirthDate('1990-01-01') ; + } + private function persist(ApplicationFormSubmission $submission): ApplicationFormSubmission + { $this->entityManager->persist($submission); $this->entityManager->flush();