[9.x] Ability to attach an array of files in MailMessage

[Illusionist3886]

Passing a filepath or an array of filepaths will now attach one or multiple files in mail.

$files = ['file.jpeg', 'file2.jpeg'];

Then in build method

return $this->view('welcome')
        ->attach($files)
        ->with('order', $this->order);

[timacdonald]

Although I can absolutely get behind the intended functionality of this PR and think it is a nice addition to the framework, I don't feel this is the best way to handle it.

Take the following example...

$mailable->attach(['forge.png', 'vapor.jpg'], [
    'as' => 'logo',
    'mime' => 'image/jpg',
]);

This means that you cannot assign intended names / mime types to the individual elements in the array. Instead of trying to pipe this all through the existing method, I would personally prefer to see an attachMany method, potentially with the following API...

$mailable->attachMany([
    'forge.png' => ['as' => 'forge-logo', 'mime' => 'image/png'],
    'vapor.jpg' => ['as' => 'vapor-logo', 'mime' => 'image/jpeg'],
    'envoyer.svg', // just a filename
    'nova.svg', // just a filename
    $echo, // instance of attachable 
    $horizon, // instance of attachable 
]);

If this is wanted for the framework, I would be happy to assist @Illusionist3886 getting this one in shape and tested.

[Illusionist3886]

@tmacdonald
I also think your approach will be much cleaner. And give the developer more power to manage attachables

[timacdonald]

@Illusionist3886 would you like me to jump into this and make the necessary changes? Or did you want to have a go at it first?

[Illusionist3886]

Let me try a few days more. I can surely use your help about writing tests.

[timacdonald]

No worries at all. Ping me and I can jump in, otherwise I'll check back with you in a few days.

[Illusionist3886]

@timacdonald
I've implemented the new method attachMany.
Thanks for your patience, support and ideas.
We can pass filename, filenames array, attachable object through this method.

$items = ['index.php', 'robots.txt'];

$item = 'forge.svg';

$item_details = [
                 'dark-gold.jpeg' => ['as' => 'Gold Dust.jpeg', 'mime' => 'image/jpeg'],
                 'forge.svg', // just a filename
                 'vapor.svg', // just a filename
                ];

We can pass any of this and also the attachable object too.

Can you confirm and write some tests for this, please?

[timacdonald]

Awesome stuff @Illusionist3886 Gonna take a look at this today and then will get some tests written up and sent to your branch.

[timacdonald]

@Illusionist3886 I've just pushed some tests + some formatting changes. I've removed the options array based on feedback from @BrandonSurowiec

Would love to hear your thoughts after seeing my adjustments. If you are happy with this now I think you can open it back up for review 😀

I'm not sure if we need to consider things attachFromStorage and attachData with this new function. These are all supported via the new "attachables" concept in conjunction with the attachMany func, so I think we can just lean on that instead of making the "low-level" versions a first class citizen in the attachMany function.

I've tested this new function against a live mail server to confirm that the attachments are being attached to emails as expected.

[Illusionist3886]

Thanks for the support ☺️.
Do you think we should add a check if the passed items are iterable or not?
May act as a failsafe. Other than that, I think we are ready for review.

[timacdonald]

No troubles at all. Always excited to see new contributors making things happen in the framework!

Do you think we should add a check if the passed items are iterable or not?

I don't believe we should add an additional type check. We have specified the type in the docblock, which is the Laravel convention.

[taylorotwell]

So, I'm just curious, can you not just call attach multiple times?

[Illusionist3886]

I could do for Mailable's but when using MailMessage from Notification, I couldn't find a cleaner way myself specially when items are dynamic.

[taylorotwell]

Sorry - I don't follow. You can't just call attach multiple times with notification mail messages? /cc @timacdonald

[Illusionist3886]

Yes, we can definitely attach multiple times.

return $this->view('welcome')
        ->attach('goat.png')
        ->attach('dark-gold.jpeg')
        ->attach('forge.svg')
        ->with('order', $this->order);

But if we have unknown amount of items to be attached we need to do a foreach loop like below.

$item_details = [
                   'goat.png',
                   'dark-gold.jpeg',
                   'forge.svg', // just a filename
                   'vapor.svg', // just a filename
               ];

foreach ($item_details as $item) {
            $this->attach($item);
        }

return $this->view('welcome')
            ->with('order', $this->order);

But problem comes when $item_details has option too in some items and not in other items. As the developer will have to handle this manually when iterating through the items array.
For example:

 $item_details = [
                   'goat.png' => ['as' => 'Eid Mubarak.png', 'mime' => 'image/png'],
                   'dark-gold.jpeg' => ['as' => 'Gold Dust.jpeg', 'mime' => 'image/jpeg'],
                   'forge.svg', // just a filename
                   'vapor.svg', // just a filename
                ];

These are all for build() method. But for toMail() method I don't know if there's any suitable way.

So, I thought why not create a attachMany method which can help to write more cleaner code. Just my opinion.