Page MenuHomePhabricator

Improve maintainability and scalability of Mail component
Open, LowPublic

Description

I noticed that the current Mail component has low scalability issues.

Other components directly hardcode access to a concrete class in Mail component, not an abstract type. This will hinder us to expanding our Mail capabilities in the future.

Based on this, we should write one or several interfaces to let other components depend on these interfaces instead of the concrete classes.

Event Timeline

Since I am working on a design-related task for the first time, it would be nice if someone came to verify the intention or provide help of this task.

Change 471313 had a related patch set uploaded (by 星耀晨曦; owner: 星耀晨曦):
[mediawiki/core@master] Improve maintainability and scalability of Mail component

https://gerrit.wikimedia.org/r/471313

Krinkle added a project: Platform Engineering.
Krinkle subscribed.

When we talk about design, we usually refer to graphical design of user interfaces. In the context of code, we usually call it "architecture".

I've cc-ed the TC and CPT groups who may be interested in giving you feedback on this approach.

I reviewed the patch. The approach looks good, but the refactoring should be improved to achieve better decoupling of components.

Status update: the code is looking good, but needs manual testing. There seems to be no good way to test sendmail and Pear Mail related code in a unit test.

@daniel What do we need to test? Mail content?

@RazeSoldier the fact that it actually sends mail with the correct content and headers :)

Extra points for checking for encoding/escaping issues, both in headers and content.

Ideally, testing is not done by the author of the code.

Change 471313 had a related patch set uploaded (by 星耀晨曦; owner: 星耀晨曦):
[mediawiki/core@master] Improve maintainability and scalability of Mail component

https://gerrit.wikimedia.org/r/471313

WDoranWMF subscribed.

We're waiting on progress on T215918 in order to be able to move this ticket forward.

Aklapper added a subscriber: daniel.

Removing task assignee due to inactivity, as this open task has been assigned for more than two years (see emails sent to assignee on May26 and Jun17, and T270544). Please assign this task to yourself again if you still realistically [plan to] work on this task - it would be very welcome!

(See https://www.mediawiki.org/wiki/Bug_management/Assignee_cleanup for tips how to best manage your individual work in Phabricator.)

Change 736814 had a related patch set uploaded (by 星耀晨曦; author: 星耀晨曦):

[mediawiki/core@master] refactor(Mail): Use Emailer service instead of UserMailer::send()

https://gerrit.wikimedia.org/r/736814

Change 471313 abandoned by 星耀晨曦:

[mediawiki/core@master] Improve maintainability and scalability of Mail component

Reason:

This patch is too old. I will continue to work on future patches.

https://gerrit.wikimedia.org/r/471313

Change 736816 had a related patch set uploaded (by 星耀晨曦; author: 星耀晨曦):

[mediawiki/core@master] Hard-deprecate UserMailer::rfc822Phrase() because it has not been used by anything.

https://gerrit.wikimedia.org/r/736816

Change 736817 had a related patch set uploaded (by 星耀晨曦; author: 星耀晨曦):

[mediawiki/core@master] Hard-deprecate UserMailer::rfc822Phrase() because it has not been used by anything.

https://gerrit.wikimedia.org/r/736817

Change 736816 abandoned by 星耀晨曦:

[mediawiki/core@master] Hard-deprecate UserMailer::rfc822Phrase() because it has not been used by anything.

Reason:

See https://gerrit.wikimedia.org/r/c/mediawiki/core/+/736817/

https://gerrit.wikimedia.org/r/736816

Change 736814 abandoned by Reedy:

[mediawiki/core@master] refactor(Mail): Use Emailer service instead of UserMailer::send()

Reason:

Changes done by I5d682d434b4bc1dc0fa040b91854ef1a43779473

https://gerrit.wikimedia.org/r/736814

Change 736817 merged by jenkins-bot:

[mediawiki/core@master] Hard-deprecate UserMailer::rfc822Phrase() because it has not been used by anything.

https://gerrit.wikimedia.org/r/736817