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

[9.x] Add ability to determine if attachments exist #43967

Merged

Conversation

macbookandrew
Copy link
Contributor

This adds the ability to determine if the given attachment(s) are included in the mailable, similar to the existing $mail->hasTo(), $mail->hasSubject(), etc. methods.

This is particularly useful in tests; example:

use App\Mail\InvoicePaid;
use App\Models\User;
 
public function test_mailable_content()
{
    $user = User::factory()->create();
 
    $mailable = new InvoicePaid($user);
 
    $mailable->assertSeeInHtml($user->email);
    $mailable->assertSeeInHtml('Invoice Paid');
    $mailable->assertSeeInOrderInHtml(['Invoice Paid', 'Thanks']);
 
    $mailable->assertSeeInText($user->email);
    $mailable->assertSeeInOrderInText(['Invoice Paid', 'Thanks']);

    // Test normal attachment.
    $this->assertTrue(
        $mail->hasAttachment('Receipt.pdf')
    );

    // Test attachment from storage disk.
    $this->assertTrue(
        $mail->hasAttachmentFromStorageDisk('s3', 'invoices', $user->latest_invoice->name)
    );

    // Test raw attachment.
    $this->assertTrue(
        $mail->hasAttachedData('12345', 'confirmation.txt')
    );
}

@nunomaduro nunomaduro changed the title add ability to determine if attachments exist [9.x] Aad ability to determine if attachments exist Sep 1, 2022
@taylorotwell
Copy link
Member

Should this work with attachable objects?

@taylorotwell taylorotwell changed the title [9.x] Aad ability to determine if attachments exist [9.x] Add ability to determine if attachments exist Sep 5, 2022
@timacdonald timacdonald marked this pull request as draft September 6, 2022 23:48
@timacdonald
Copy link
Member

Converting to draft and working on some improvements to support attachments / attachables and adding some tests for us as well.

@timacdonald
Copy link
Member

timacdonald commented Sep 7, 2022

  • Now supports implementations of Attachable and Attachment objects.
  • Added tests
  • Tweaked the implementation so that you need to be exact about the options passed, i.e.
$mail->attach('style', ['mime' => 'text/css']);


$mail->hasAttachment('style');
// false

$mail->hasAttachment('style', ['mime' => 'text/css']);
// true

One thing to note however, is that when attaching an Attachable or Attachment, you need to use the $mail->hasAttachment() method rather than the data / storage check.

$mail->attach(Attachment::fromStorageDisk('s3', 'foo.png'));


$mail->hasAttachment(Attachment::fromStorage('s3', 'foo.png'));
// true

$mail->hasAttachmentFromStorageDisk('s3', 'foo.png');
// false

This is because of the implementation of the attachment, where it really acts as a data attachment whether it is from storage or from data.

I also added some assertions to compliment the existing assertions.

$mail->assertHasAttachment(...);
$mail->assertHasAttachedData(...);
$mail->assertHasAttachmentFromStorage(...);
$mail->assertHasAttachmentFromStorageDisk(...);

@timacdonald timacdonald marked this pull request as ready for review September 7, 2022 04:33
@taylorotwell taylorotwell marked this pull request as draft September 7, 2022 16:26
@timacdonald timacdonald marked this pull request as ready for review September 7, 2022 23:57
@macbookandrew
Copy link
Contributor Author

@timacdonald thanks!! I wasn’t sure where/how to test the new methods.

Here’s a PR with documentation as well: laravel/docs#8195

@taylorotwell taylorotwell merged commit 50a9486 into laravel:9.x Sep 8, 2022
@timacdonald
Copy link
Member

No troubles at all. Thanks for PR'ing the feature ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants