From 66e59d6e48efa9cd85066ef9c91898b65db3ed88 Mon Sep 17 00:00:00 2001 From: danilopolani Date: Tue, 20 Feb 2024 11:18:30 +0100 Subject: [PATCH 1/2] fix attachments on retry email feature --- src/Actions/SendMail.php | 7 +-- src/Dto/AttachmentDto.php | 30 ++++++---- src/Dto/RecipientDto.php | 2 +- src/Dto/SendMailDto.php | 2 +- .../Controllers/MailCarrierController.php | 20 ++++++- src/Models/Log.php | 15 +++++ src/Resources/LogResource.php | 55 ++++++++++++++++--- .../TemplateResource/Pages/EditTemplate.php | 2 +- .../Logs/CreateFromGenericMailTest.php | 6 +- tests/Feature/SendMailTest.php | 19 ++++--- 10 files changed, 115 insertions(+), 43 deletions(-) diff --git a/src/Actions/SendMail.php b/src/Actions/SendMail.php index 50d2073..50b4c57 100644 --- a/src/Actions/SendMail.php +++ b/src/Actions/SendMail.php @@ -2,11 +2,9 @@ namespace MailCarrier\Actions; -use Illuminate\Http\UploadedFile; use Illuminate\Support\Facades\Config; use Illuminate\Support\Str; use Laravel\SerializableClosure\SerializableClosure; -use MailCarrier\Dto\AttachmentDto; use MailCarrier\Dto\GenericMailDto; use MailCarrier\Dto\RecipientDto; use MailCarrier\Dto\SendMailDto; @@ -125,10 +123,7 @@ protected function send(RecipientDto $recipient): void cc: $recipient->cc, bcc: $recipient->bcc, subject: $this->params->subject, - attachments: array_map( - fn (UploadedFile $file) => new AttachmentDto($file), - $recipient->attachments - ), + attachments: $recipient->attachments, remoteAttachments: $recipient->remoteAttachments, template: $this->template, variables: $recipient->variables, diff --git a/src/Dto/AttachmentDto.php b/src/Dto/AttachmentDto.php index 832c38b..3880cbf 100644 --- a/src/Dto/AttachmentDto.php +++ b/src/Dto/AttachmentDto.php @@ -3,20 +3,26 @@ namespace MailCarrier\Dto; use Illuminate\Http\UploadedFile; -use Spatie\DataTransferObject\DataTransferObject; -class AttachmentDto extends DataTransferObject +class AttachmentDto { - public readonly string $name; - - public readonly string $content; - - public readonly int $size; - - public function __construct(UploadedFile $file) + public static function fromUploadedFile(UploadedFile $file): static { - $this->name = $file->getClientOriginalName(); - $this->content = base64_encode($file->getContent()); - $this->size = $file->getSize(); + return new static( + name: $file->getClientOriginalName(), + content: base64_encode($file->getContent()), + size: $file->getSize(), + ); + } + + /** + * @param string $content Base64 encoded file content + */ + public function __construct( + public readonly string $name, + public readonly string $content, + public readonly int $size + ) { + // } } diff --git a/src/Dto/RecipientDto.php b/src/Dto/RecipientDto.php index 8121b97..2a7d28c 100644 --- a/src/Dto/RecipientDto.php +++ b/src/Dto/RecipientDto.php @@ -24,7 +24,7 @@ class RecipientDto extends DataTransferObject #[CastWith(ContactArrayCaster::class)] public ?array $bcc; - /** @var \Illuminate\Http\UploadedFile[] */ + /** @var \MailCarrier\Dto\AttachmentDto[] */ public array $attachments = []; /** @var \MailCarrier\Dto\RemoteAttachmentDto[] */ diff --git a/src/Dto/SendMailDto.php b/src/Dto/SendMailDto.php index d4bc6f3..495d337 100644 --- a/src/Dto/SendMailDto.php +++ b/src/Dto/SendMailDto.php @@ -38,7 +38,7 @@ class SendMailDto extends DataTransferObject public ?string $trigger; - /** @var \Illuminate\Http\UploadedFile[] */ + /** @var \MailCarrier\Dto\AttachmentDto[] */ public array $attachments = []; /** @var \MailCarrier\Dto\RemoteAttachmentDto[] */ diff --git a/src/Http/Controllers/MailCarrierController.php b/src/Http/Controllers/MailCarrierController.php index 38ddcef..3f055cc 100644 --- a/src/Http/Controllers/MailCarrierController.php +++ b/src/Http/Controllers/MailCarrierController.php @@ -5,9 +5,12 @@ use Exception; use Illuminate\Http\JsonResponse; use Illuminate\Http\Response; +use Illuminate\Http\UploadedFile; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\Config; use MailCarrier\Actions\Attachments\Download; use MailCarrier\Actions\SendMail; +use MailCarrier\Dto\AttachmentDto; use MailCarrier\Dto\SendMailDto; use MailCarrier\Enums\ApiErrorKey; use MailCarrier\Exceptions\AttachmentNotDownloadableException; @@ -39,7 +42,22 @@ public function send(SendMailRequest $request, SendMail $sendMail): JsonResponse { try { $sendMail->run( - new SendMailDto($request->validated()) + new SendMailDto([ + ...$request->safe()->except(['recipients', 'attachments']), + 'recipients' => !$request->has('recipients') + ? null + : $request->collect('recipients') + ->map(fn (array $recipient, int $i) => [ + ...$recipient, + 'attachments' => Collection::make($request->file("recipients.{$i}.attachments")) + ->map(fn (UploadedFile $file) => AttachmentDto::fromUploadedFile($file)) + ->all() + ]) + ->all(), + 'attachments' => Collection::make($request->file('attachments')) + ->map(fn (UploadedFile $file) => AttachmentDto::fromUploadedFile($file)) + ->all(), + ]) ); } catch (MissingVariableException $e) { report($e); diff --git a/src/Models/Log.php b/src/Models/Log.php index a65a76a..5d342f0 100644 --- a/src/Models/Log.php +++ b/src/Models/Log.php @@ -127,4 +127,19 @@ public function prunable(): EloquentBuilder fn (EloquentBuilder $query, string $period) => $query->where('created_at', '<=', Carbon::now()->sub(...explode(' ', $period))) ); } + + public function isFailed(): bool + { + return $this->status === LogStatus::Failed; + } + + public function isPending(): bool + { + return $this->status === LogStatus::Pending; + } + + public function isSent(): bool + { + return $this->status === LogStatus::Sent; + } } diff --git a/src/Resources/LogResource.php b/src/Resources/LogResource.php index 36ebec1..c45a10d 100644 --- a/src/Resources/LogResource.php +++ b/src/Resources/LogResource.php @@ -11,15 +11,19 @@ use Filament\Tables; use Filament\Tables\Actions\Action as TablesAction; use Filament\Tables\Enums\FiltersLayout; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\Config; use Illuminate\Support\Facades\View; use Illuminate\Support\HtmlString; +use Livewire\Features\SupportFileUploads\TemporaryUploadedFile; use MailCarrier\Actions\Logs\GetTriggers; use MailCarrier\Actions\SendMail; +use MailCarrier\Dto\AttachmentDto; use MailCarrier\Dto\LogTemplateDto; use MailCarrier\Dto\SendMailDto; use MailCarrier\Enums\LogStatus; use MailCarrier\Facades\MailCarrier; +use MailCarrier\Models\Attachment; use MailCarrier\Models\Log; use MailCarrier\Models\Template; use MailCarrier\Resources\LogResource\Pages; @@ -28,7 +32,7 @@ class LogResource extends Resource { protected static ?string $model = Log::class; - protected static ?string $navigationIcon = 'heroicon-o-paper-airplane'; + protected static ?string $navigationIcon = 'heroicon-o-envelope'; /** * List all the records. @@ -151,22 +155,29 @@ protected static function getTableActions(): array ->modalCancelActionLabel('Close') ->modalFooterActionsAlignment(Alignment::Right), - Tables\Actions\Action::make('manual_retry') + Tables\Actions\Action::make('resend_email') + ->label(fn (Log $record) => $record->isFailed() ? 'Retry now' : 'Send again') ->icon('heroicon-o-arrow-path') - ->color(Color::Orange) + ->color(fn (Log $record) => $record->isFailed() ? Color::Orange : 'primary') ->form([ FileUpload::make('attachments') + ->label('Additional attachments') + ->helperText('Any extra attachment will be sent along the original ones') ->multiple() ->preserveFilenames() ->storeFiles(false), ]) ->modalWidth('2xl') ->modalIcon('heroicon-o-arrow-path') - ->modalDescription('Are you sure you want to manually retry to send this email?') - ->modalSubmitActionLabel('Retry') + ->modalDescription(fn (Log $record) => + $record->isFailed() + ? 'Are you sure you want to manually retry to send this email?' + : 'Are you sure you want to to send again this email?' + ) + ->modalSubmitActionLabel(fn (Log $record) => $record->isFailed() ? 'Retry' : 'Resend') ->modalFooterActionsAlignment(Alignment::Right) - ->action(fn (?Log $record, array $data) => $record ? static::retryEmail($record, $data) : null) - ->visible(fn (?Log $record) => $record?->status === LogStatus::Failed), + ->action(fn (?Log $record, array $data) => $record ? static::resendEmail($record, $data) : null) + ->visible(fn (?Log $record) => $record?->status !== LogStatus::Pending), ]; } @@ -197,8 +208,15 @@ protected static function getTemplateValue(LogTemplateDto $templateDto, ?Templat ); } - protected static function retryEmail(Log $log, array $data): void + protected static function resendEmail(Log $log, array $data): void { + // Update the status to Failed just to send email again + if ($log->isSent()) { + $log->update([ + 'status' => LogStatus::Failed, + ]); + } + try { SendMail::resolve()->run( new SendMailDto([ @@ -212,7 +230,26 @@ protected static function retryEmail(Log $log, array $data): void 'trigger' => $log->trigger, 'tags' => $log->tags ?: [], 'metadata' => $log->metadata ?: [], - 'attachments' => $data['attachments'] ?? [], + 'attachments' => $log->attachments + ->map(fn (Attachment $attachment) => !$attachment->canBeDownloaded() + ? null + : new AttachmentDto( + name: $attachment->name, + content: $attachment->content, + size: $attachment->size + ) + ) + ->filter() + ->merge( + Collection::make($data['attachments'])->map( + fn (TemporaryUploadedFile $file) => new AttachmentDto( + name: $file->getClientOriginalName(), + content: base64_encode(file_get_contents($file->getRealPath())), + size: $file->getSize() + ) + ) + ) + ->all(), ]), $log ); diff --git a/src/Resources/TemplateResource/Pages/EditTemplate.php b/src/Resources/TemplateResource/Pages/EditTemplate.php index ea7d117..228f63a 100644 --- a/src/Resources/TemplateResource/Pages/EditTemplate.php +++ b/src/Resources/TemplateResource/Pages/EditTemplate.php @@ -87,7 +87,7 @@ protected function sendTestMail(array $data): void Notification::make() ->title('Test email sent correctly') ->icon('heroicon-o-mail') - ->iconColor('success') + ->success() ->send(); } } diff --git a/tests/Feature/Logs/CreateFromGenericMailTest.php b/tests/Feature/Logs/CreateFromGenericMailTest.php index e1f32cc..fd3d0be 100644 --- a/tests/Feature/Logs/CreateFromGenericMailTest.php +++ b/tests/Feature/Logs/CreateFromGenericMailTest.php @@ -188,7 +188,7 @@ template: Template::factory()->create(), variables: ['name' => 'foo'], attachments: [ - new AttachmentDto(UploadedFile::fake()->create('image.jpg', 100, 'image/jpeg')), + AttachmentDto::fromUploadedFile(UploadedFile::fake()->create('image.jpg', 100, 'image/jpeg')), ], remoteAttachments: [ new RemoteAttachmentDto( @@ -290,7 +290,7 @@ template: Template::factory()->create(), variables: ['name' => 'foo'], attachments: [ - new AttachmentDto(UploadedFile::fake()->create('image.jpg', 100, 'image/jpeg')), + AttachmentDto::fromUploadedFile(UploadedFile::fake()->create('image.jpg', 100, 'image/jpeg')), ], remoteAttachments: [ new RemoteAttachmentDto( @@ -402,7 +402,7 @@ template: Template::factory()->create(), variables: ['name' => 'foo'], attachments: [ - new AttachmentDto(UploadedFile::fake()->create('image.jpg', 100, 'image/jpeg')), + AttachmentDto::fromUploadedFile(UploadedFile::fake()->create('image.jpg', 100, 'image/jpeg')), ], remoteAttachments: [ new RemoteAttachmentDto( diff --git a/tests/Feature/SendMailTest.php b/tests/Feature/SendMailTest.php index 337746b..2e30737 100644 --- a/tests/Feature/SendMailTest.php +++ b/tests/Feature/SendMailTest.php @@ -6,6 +6,7 @@ use Illuminate\Support\Facades\Config; use Illuminate\Support\Facades\Mail; use MailCarrier\Actions\SendMail; +use MailCarrier\Dto\AttachmentDto; use MailCarrier\Dto\GenericMailDto; use MailCarrier\Dto\RemoteAttachmentDto; use MailCarrier\Dto\SendMailDto; @@ -722,8 +723,8 @@ 'subject' => 'Welcome!', 'recipient' => 'recipient@example.org', 'attachments' => [ - $attachment1 = UploadedFile::fake()->image('foo.jpg'), - $attachment2 = UploadedFile::fake()->image('bar.jpg'), + AttachmentDto::fromUploadedFile($attachment1 = UploadedFile::fake()->image('foo.jpg')), + AttachmentDto::fromUploadedFile($attachment2 = UploadedFile::fake()->image('bar.jpg')), ], ])); @@ -820,7 +821,7 @@ 'subject' => 'Welcome!', 'recipient' => 'recipient@example.org', 'attachments' => [ - $attachment = UploadedFile::fake()->image('foo.jpg'), + AttachmentDto::fromUploadedFile($attachment = UploadedFile::fake()->image('foo.jpg')), ], 'remoteAttachments' => [ new RemoteAttachmentDto([ @@ -868,13 +869,13 @@ [ 'email' => 'recipient1@example.org', 'attachments' => [ - $attachment1 = UploadedFile::fake()->image('foo.jpg'), + AttachmentDto::fromUploadedFile($attachment1 = UploadedFile::fake()->image('foo.jpg')), ], ], [ 'email' => 'recipient2@example.org', 'attachments' => [ - $attachment2 = UploadedFile::fake()->image('bar.jpg'), + AttachmentDto::fromUploadedFile($attachment2 = UploadedFile::fake()->image('bar.jpg')), ], ], ], @@ -1012,7 +1013,7 @@ [ 'email' => 'recipient1@example.org', 'attachments' => [ - $attachment1 = UploadedFile::fake()->image('foo1.jpg'), + AttachmentDto::fromUploadedFile($attachment1 = UploadedFile::fake()->image('foo1.jpg')), ], 'remoteAttachments' => [ new RemoteAttachmentDto([ @@ -1024,7 +1025,7 @@ [ 'email' => 'recipient2@example.org', 'attachments' => [ - $attachment2 = UploadedFile::fake()->image('bar1.jpg'), + AttachmentDto::fromUploadedFile($attachment2 = UploadedFile::fake()->image('bar1.jpg')), ], 'remoteAttachments' => [ new RemoteAttachmentDto([ @@ -1100,7 +1101,7 @@ 'template' => 'welcome', 'subject' => 'Welcome!', 'attachments' => [ - $globalAttachment = UploadedFile::fake()->image('globalRaw.jpg'), + AttachmentDto::fromUploadedFile($globalAttachment = UploadedFile::fake()->image('globalRaw.jpg')), ], 'remoteAttachments' => [ new RemoteAttachmentDto([ @@ -1121,7 +1122,7 @@ [ 'email' => 'recipient2@example.org', 'attachments' => [ - $recipientAttachment = UploadedFile::fake()->image('bar.jpg'), + AttachmentDto::fromUploadedFile($recipientAttachment = UploadedFile::fake()->image('bar.jpg')), ], ], ], From 4b8e18dfadaba4b8bb53bbe35fd6d757a5bef911 Mon Sep 17 00:00:00 2001 From: danilopolani Date: Tue, 20 Feb 2024 11:23:39 +0100 Subject: [PATCH 2/2] fix linting --- src/Dto/AttachmentDto.php | 6 +++--- .../Controllers/MailCarrierController.php | 2 +- src/Resources/LogResource.php | 19 ++++++++++--------- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/Dto/AttachmentDto.php b/src/Dto/AttachmentDto.php index 3880cbf..38ed84d 100644 --- a/src/Dto/AttachmentDto.php +++ b/src/Dto/AttachmentDto.php @@ -6,9 +6,9 @@ class AttachmentDto { - public static function fromUploadedFile(UploadedFile $file): static + public static function fromUploadedFile(UploadedFile $file): self { - return new static( + return new self( name: $file->getClientOriginalName(), content: base64_encode($file->getContent()), size: $file->getSize(), @@ -16,7 +16,7 @@ public static function fromUploadedFile(UploadedFile $file): static } /** - * @param string $content Base64 encoded file content + * @param string $content Base64 encoded file content */ public function __construct( public readonly string $name, diff --git a/src/Http/Controllers/MailCarrierController.php b/src/Http/Controllers/MailCarrierController.php index 3f055cc..c62f52a 100644 --- a/src/Http/Controllers/MailCarrierController.php +++ b/src/Http/Controllers/MailCarrierController.php @@ -51,7 +51,7 @@ public function send(SendMailRequest $request, SendMail $sendMail): JsonResponse ...$recipient, 'attachments' => Collection::make($request->file("recipients.{$i}.attachments")) ->map(fn (UploadedFile $file) => AttachmentDto::fromUploadedFile($file)) - ->all() + ->all(), ]) ->all(), 'attachments' => Collection::make($request->file('attachments')) diff --git a/src/Resources/LogResource.php b/src/Resources/LogResource.php index c45a10d..e19c324 100644 --- a/src/Resources/LogResource.php +++ b/src/Resources/LogResource.php @@ -169,8 +169,8 @@ protected static function getTableActions(): array ]) ->modalWidth('2xl') ->modalIcon('heroicon-o-arrow-path') - ->modalDescription(fn (Log $record) => - $record->isFailed() + ->modalDescription( + fn (Log $record) => $record->isFailed() ? 'Are you sure you want to manually retry to send this email?' : 'Are you sure you want to to send again this email?' ) @@ -231,13 +231,14 @@ protected static function resendEmail(Log $log, array $data): void 'tags' => $log->tags ?: [], 'metadata' => $log->metadata ?: [], 'attachments' => $log->attachments - ->map(fn (Attachment $attachment) => !$attachment->canBeDownloaded() - ? null - : new AttachmentDto( - name: $attachment->name, - content: $attachment->content, - size: $attachment->size - ) + ->map( + fn (Attachment $attachment) => !$attachment->canBeDownloaded() + ? null + : new AttachmentDto( + name: $attachment->name, + content: $attachment->content, + size: $attachment->size + ) ) ->filter() ->merge(