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

[BC BREAK] emails persisted between reboots #9

Merged
merged 1 commit into from
Oct 4, 2021
Merged

Conversation

kbond
Copy link
Member

@kbond kbond commented Sep 30, 2021

Closes #5.

BC BREAKS

  1. For each test in a test case using InteractsWithMailer, emails are now persisted between kernel reboots. This will cause BC breaks for tests that have kernel reboots (ie using zenstruck/browser) and checking emails multiple times.
  2. A lightweight bundle is now included and required. If this package is already installed in your app, when you upgrade, symfony/flex will not enable this bundle and you'll have to do this manually.

@kbond kbond added the enhancement New feature or request label Sep 30, 2021
@kbond kbond changed the title [BC BREAK] emails persisted between reboots (closes #5) [BC BREAK] emails persisted between reboots Sep 30, 2021
@kbond
Copy link
Member Author

kbond commented Sep 30, 2021

Ping @OskarStark, @silasjoisten. Would you be able to review/test in your app? Some questions/thoughts:

  1. Should this behaviour be configurable? If so, what should be the default and per test/test case or globally (bundle config)?
  2. Currently, there is no way to manually reset the collected messages within a test. Think this is a valid requirement?

@OskarStark
Copy link
Contributor

OskarStark commented Oct 1, 2021

Hi Kevin,

thanks for the ping.
We only use it once in our tests, so it would not break I think.

About the bundle, do you only use it to a service with a tag? In this case I would rather create a flex recipe instead of a bundle and add some (three) lines of documentation, in case one is not using flex and recipes.

I did this for my twig extension lib:
https://github.com/OskarStark/env-var-extension

with the following recipe:
https://github.com/symfony/recipes-contrib/blob/master/oskarstark/env-var-extension/1.0/config/packages/oskarstark_env_var_extension.yaml

WDYT?

@kbond
Copy link
Member Author

kbond commented Oct 1, 2021

We only use it once in our tests, so it would not break I think.

I thought I recalled some code where you "stop" browsing with browser, check emails, resume browsing, then check emails again.

About the bundle, do you only use it to a service with a tag? In this case I would rather create a flex recipe instead of a bundle

I suppose this is an option but I feel like it would make it harder to possibly add bundle config in the future. Especially now that I discovered that flex doesn't auto-configure bundles during an update. I feel like it's better to get the bundle added now pre-1.0 so there isn't a little BC break post 1.0.

@OskarStark
Copy link
Contributor

Indeed, but I can't remember exactly where.

@OskarStark
Copy link
Contributor

I think you can release an we can test the change with the new release.
From the code POV this makes sense and is much better without the profiler

@kbond kbond merged commit 2399e87 into zenstruck:1.x Oct 4, 2021
@kbond kbond deleted the bundle branch October 4, 2021 15:12
@kbond
Copy link
Member Author

kbond commented Oct 4, 2021

Released in v0.2.0.

@OskarStark
Copy link
Contributor

@silasjoisten please require it and enable the bundle in a PR an check if our test suite is green

Thanks

@silasjoisten
Copy link

silasjoisten commented Oct 5, 2021

@kbond in our test case we have following:

public function aLoggedInVerifiedUserCanAddNewAboByEnteringTheAktenzeichenAndTheOwnerCanConfirmAndRevokeTheAccessAfterwards(): void
    {
        $faker = self::faker();
        $email = $faker->email();

        $akte = self::Akte([
            'email' => $email,
            'owner' => null,
        ]);

        $owner = self::Abonnent([
            'email' => $email,
        ]);

        $browser = $this->browser()
            ->login($owner)
            ->requestAboForAkte($akte)
            ->assertOn('/user/dashboard')
            ->assertSuccessful();

        $messenger = $this->messenger('async')->process();

        $messenger->dispatched()
            ->assertContains(NewAboForUserAndAkteByAktenzeichen::class, 1)
            ->assertContains(NewAudit::class, 3)
            ->assertContains(SetAboConfirmationDecisionByOwner::class, 1)
            ->assertContains(SetOwnerForAkte::class, 1)
            ->assertContains(SetNewConfirmationStatus::class, 1)
            ->assertContains(SetValueNutzerMandantencockpit::class, 1)
            ->assertNotContains(SendEmailToAkteOwnerForAboConfirmation::class);

        $messenger->reset();

        $requester = self::VerifiedUser();

        $browser
            ->login($requester)
            ->requestAboForAkte($akte)
            ->assertOn('/abo/neu')
            ->assertSuccessful()
            ->assertSeeIn('h3', 'Um welchen Fall geht es?')
            ->assertSeeSuccessFlashmessage('Wir haben eine E-Mail an die in dem Fall hinterlegte E-Mail-Adresse gesendet. Nach der Bestätigung können Sie den Fall einsehen.');

        $messenger = $this->messenger('async')->process();

        $messenger->dispatched()
            ->assertContains(NewAboForUserAndAkteByAktenzeichen::class, 1)
            ->assertContains(NewAudit::class, 2)
            ->assertNotContains(SetAboConfirmationDecisionByOwner::class)
            ->assertContains(SetNewConfirmationStatus::class, 1)
            ->assertNotContains(SetValueNutzerMandantencockpit::class)
            ->assertContains(SendEmailToAkteOwnerForAboConfirmation::class, 1);

        $messenger->reset();

        $mailer = $this->mailer();
        $mailer->assertSentEmailCount(1);
        $mailer->assertEmailSentTo(
            $owner->getEmail(),
            sprintf('Bitte erlauben Sie die Einsicht in Ihren Fall %s', $akte->getAktenzeichen()->toString())
        );

        $abonnent = self::refreshUserFromDatabase($requester->getId());

        /** @var Abo $abo */
        $abo = $abonnent->getAbos()->first();

        $now = new DateTimeImmutable();

        $this->browser()
            ->login($owner)
            ->visit(sprintf('/abo/confirm/%s', $abo->getSecret()->toString()))
            ->assertSuccessful()
            ->assertSeeIn('h3', 'Fall freigeben')
            ->assertSee(sprintf(
                '%s möchte Ihren Fall mit dem Aktenzeichen %s einsehen und verwalten.',
                $abo->getUser()->getUserIdentifier(),
                $abo->getAkte()->getAktenzeichen()->toString()
            ))
            ->assertSee(sprintf(
                'Bitte beachten Sie, dass %s für Sie ebenfalls Dokumente übermitteln und Dateneingaben vornehmen darf. Die Eingaben von %s gelten damit als von Ihnen selbst eingegeben. Darüber hinaus wird die Kanzlei im Umfang des freigegebenen Falls von der anwaltlichen Schweigepflicht gegenüber %s entbunden.',
                $abo->getUser()->getUserIdentifier(),
                $abo->getUser()->getUserIdentifier(),
                $abo->getUser()->getUserIdentifier(),
            ))
            ->click('Erlauben');

        $messenger = $this->messenger('async')->process();

        $messenger->dispatched()
            ->assertContains(SetAboConfirmationDecisionByOwner::class, 1)
            ->assertContains(SetNewConfirmationStatus::class, 1)
            ->assertContains(SetValueNutzerMandantencockpit::class, 1)
            ->assertContains(SendEmailToAboUserWithDecision::class, 1);

        $messenger->reset();

        $mailer = $this->mailer();
        $mailer->assertSentEmailCount(1);
        $mailer->assertEmailSentTo(
            $abonnent->getEmail(),
            sprintf('Einsicht in Akte %s gewährt', $akte->getAktenzeichen()->toString())
        );

        $this->browser()
            ->login($owner)
            ->visit('/user/einstellungen')
            ->assertSee('Wer hat Einsicht in Ihre Fälle')
            ->assertSee($akte->getAktenzeichen()->toString())
            ->assertSee($abonnent->getUserIdentifier())
            ->assertSee($now->format('d.m.Y'))
            ->assertSee('Erlaubnis widerrufen');
    }

CleanShot 2021-10-05 at 08 02 15

in general it is correct that there 2 Mails sent. Is there any way to reset the mailer? I want to keep asserting Mails after Interaction. Means after each step i want to assert that a mail was sent.

@kbond
Copy link
Member Author

kbond commented Oct 5, 2021

Ok, a couple of thoughts/ideas:

  1. You could remove the $mailer->assertSentEmailCount(1); calls and I think this test will pass
  2. You could make all your email assertions at the end of the test

I think though, adding a global mailer reset as discussed here is best.

@kbond kbond mentioned this pull request Oct 5, 2021
@kbond
Copy link
Member Author

kbond commented Oct 5, 2021

I think though, adding a global mailer reset as discussed here is best.

This is available in v0.3.0.

@silasjoisten
Copy link

@kbond with the reset sounds good to me. Indeed i could assert this at the end. But as you can see the workflow we test is more complex. To ensure that only one mail is sent after a specific User interaction the assertCount is important to me. Thank you! i will try the v0.3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

Sent emails should be persisted between kernel reboots
3 participants