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

Refactor swiftmailer to symfony mailer #10

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

AlexzPurewoko
Copy link
Member

@AlexzPurewoko AlexzPurewoko commented Jul 16, 2024

Problem Statement

Swiftmailer is no longer maintained and they recommend us to use [Symfony Mailer](https://symfony.com/doc/current/mailer.html).

[The end of Swiftmailer (Symfony Blog)](https://symfony.com/blog/the-end-of-swiftmailer)

Plan & Todos

  • Mencari inspirasi implementasi update pada laravel terbaru, di mana pada versi laravel 9.x, laravel sudah melakukan migrasi dari swiftmailer ke symfony mailer. Adapun versi yang digunakan pada laravel 9.x adalah symfony/mailer 6.0. Versi 9.x laravel dipakai karena lebih dekat implementasinya ke 4.2, menghindari adanya beberapa major change pada versi selanjutnya.
  • Update dengan menyesuaikan beberapa perubahan yang ada di laravel 9.x beserta testingnya
  • Setelah update berhasil, menyesuaikan implementasi pada dicoding codebase

@AlexzPurewoko AlexzPurewoko self-assigned this Jul 16, 2024
@AlexzPurewoko AlexzPurewoko marked this pull request as ready for review July 18, 2024 05:17
@AlexzPurewoko AlexzPurewoko requested a review from rizqyhi July 26, 2024 01:31
@AlexzPurewoko
Copy link
Member Author

Kira kira ini mau di evaluate kapan yak?

1 similar comment
@AlexzPurewoko
Copy link
Member Author

AlexzPurewoko commented Sep 5, 2024

Kira kira ini mau di evaluate kapan yak?

@AlexzPurewoko
Copy link
Member Author

Guys sepertinya ini akan obsolete ketika tidak di implementasikan

Comment on lines -254 to -259
* @return \Illuminate\Auth\Reminders\RemindableInterface
*
* @throws \UnexpectedValueException
*/
public function getUser(array $credentials)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexzPurewoko ada pertimbangan tertentu kenapa return value-nya jadi nullable dan throw-nya dihapus?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexzPurewoko kenapa perlu dibuat V2?

@@ -13,7 +13,8 @@
"illuminate/log": "4.2.*",
"illuminate/support": "4.2.*",
"illuminate/view": "4.2.*",
"swiftmailer/swiftmailer": "~5.1"
"swiftmailer/swiftmailer": "~5.1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexzPurewoko switftmailer-nya ngga sekalian dihapus aja?

Comment on lines 282 to +287
// If the given view is an array with numeric keys, we will just assume that
// both a "pretty" and "plain" view were provided, so we will return this
// array as is, since must should contain both views with numeric keys.
if (is_array($view) && isset($view[0]))
{
return $view;
return [$view[0], $view[1] ?? null];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexzPurewoko kenapa perlu mengecek masing-masing element? Kalau melihat deskripsi di atas, lebih masuk akal mengembalikan $view langsung.

Comment on lines -366 to -371
elseif (is_string($callback))
else
{
return $this->container[$callback]->mail($message);
}

throw new \InvalidArgumentException("Callback is not valid.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexzPurewoko kenapa exception ini dihapus?

@@ -31,6 +31,7 @@
"symfony/finder": "~6.4",
"symfony/http-foundation": "~6.4",
"symfony/http-kernel": "~6.4",
"symfony/mailer": "^6.4",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexzPurewoko swiftmailer di list ini juga perlu dihapus.

Comment on lines +51 to +53
public function sender(string $address, ?string $name = null): self
{
$this->swift->setSender($address, $name);
$this->message->sender(new Address($address, $name));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexzPurewoko parameter $name di constructor Address ngga nullable, jadi ini perlu diberi pengaman seperti di method from di atas.

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.

2 participants