🇺🇦Ukraine @bohart

bohart → created an issue.

bohart → created an issue.

bohart → created an issue.

Added a few comments for better future maintainability.

bohart → created an issue.

bohart → created an issue.

The majority of the issues reported by phpstan have been fixed and committed.
The remaining issues will be fixed at:
- 🐛 Private Message formState storage isn't populated with the members widget data Needs work
- 📌 Replace \Drupal::currentUser() with dependency injection at src/Service/PrivateMessageService.php Needs review
- 📌 Move rule support into a new submodule (or remove it) Active
- 📌 Call to deprecated function user_role_names() Active
- 📌 Constructor of class PrivateMessageAccessControlHandler has an unused parameters Active
- 📌 DI problems Needs review

As this MR is not mergeable anymore, let's simplify this issue.

"\Drupal::currentUser(" should be fixed at 📌 Replace \Drupal::currentUser() with dependency injection at src/Service/PrivateMessageService.php Needs review

The goal of this issue is to fix `PrivateMessageThread::load` only.

Thanks!

@Mohd Sahzad , please be respectful to `Assigned` field of the issues.
If it is chosen, that means someone already works on it. Thanks!

bohart → created an issue.

bohart → created an issue.

bohart → created an issue.

bohart → created an issue.

Hi @artem_sylchuk,
Something went wrong on this issue.

The commit added this line:
if (empty($storage['field_storage']['#parents']['#fields']['members'])) {

But variable $storage is always empty (as it is never defined).
Should we move `$storage = $form_state_copy->getStorage();` above the if or completely remove those lines?

Thanks!

PHPCS job is now green.

Still, there are some issues with eslint, stylelint and phpstan. They will be processed at:
- 📌 Fix the issues reported by phpstan Active

Thanks all!

bohart → created an issue.

bohart → changed the visibility of the branch private_message_phpcs_fix33179853317985 to hidden.

bohart → changed the visibility of the branch 3317985-3.0.x-fix-cs to hidden.

bohart → changed the visibility of the branch private_message_fix_phpcs-33179853317985 to hidden.

bohart → changed the visibility of the branch private_message_phpcs_fix-33179853317985 to hidden.

bohart → changed the visibility of the branch 3317985-fix-what-reported to hidden.

bohart → changed the visibility of the branch 3.0.x to hidden.

bohart → made their first commit to this issue’s fork.

There were three blocked issues to merge this one:
- 🐛 Error: Call to a member function get() on null in Drupal\private_message\Form\PrivateMessageForm->save() (line 481 of modules/contrib/private_message/src/Form/PrivateMessageForm.php). Fixed
- 📌 SQL Error at checkPrivateMessageMemberExists nethod Fixed
- 📌 Add settings.count_method missing schema Fixed
As all of them are fixed/merged already, this one is also merged.

Still, we have eslint, phpcs, phpstan, styleling warnings to fix. All of them to be fixed at:
- 📌 DI problems Needs review
- 📌 Fix the issues reported by phpcs Needs review

Summary of this issue:
- gitlab ci pipelines has been added;
- phpstan configuration has been added;
- DrupalCI (deprecated) tests run for 3.0.x branch have been removed;

Here is a followup issue for the future: 📌 Thread entity stored 4 times in PrivateMessageForm Active

bohart → created an issue.

Hi @artem_sylchuk,

There is one more opened MR related to this issue:
https://git.drupalcode.org/project/private_message/-/merge_requests/38/d...

Should it be closed?
Thanks!

Hi @artem_sylchuk,

Is it okay that the subject is not displayed either in the "Private Message Inbox" block or on the "/private-message/X" pages?
Should we create an additional issue to implement so?

Thanks!

bohart → created an issue.

bohart → created an issue.

Tested locally, tests passed locally.

OK (8 tests, 169 assertions)

Summary:
- added .gitlab-ci.yml file to run Gitlab CI;
- added phpstan.neon file to properly configure phpstan;

All jobs (composer, eslint, phpcs, phpstan) work and are green now.
Merge request: !20
Pipeline: #83372

Marking as "needs review". Thanks!

bohart → created an issue.

Summary:
- added .gitlab‎-ci.yml‎ for pipeline configuration;
- added phpstan configurations;
- fixes stylelint errors;
- basic fixes for PHPUnit;

Remaining tasks:
📌 Add settings.plugins.mentions missing schema Needs work

bohart → created an issue.

bohart → created an issue.

bohart → created an issue.

Everything is done.

The project page includes the latest release, dev version for it, and GitLab CI status.

bohart → created an issue.

bohart → created an issue.

Summary:
- added .gitlab-ci.yml file to run Gitlab CI;
- added phpstan.neon file to properly configure phpstan;
- fixed all phpcs and phpstan warnings and errors;

All jobs (composer, eslint, phpcs, phpstan) work and are green now.
Merge request: !8
Pipeline: #83204

Marking as "needs review". Thanks!

bohart → created an issue.

Summary:
- added .gitlab-ci.yml file to run Gitlab CI;
- added phpstan.neon file to properly configure phpstan;
- fixed all phpcs and phpstan warnings and errors;

All jobs (composer, eslint, phpcs, phpstan) work and are green now.
Merge request: !7
Pipeline: #83125

Marking as "needs review". Thanks!

bohart → created an issue.

Summary:
- added .gitlab-ci.yml file to run Gitlab CI;
- added phpstan.neon file to properly configure phpstan;
- fixed all phpcs and eslint warnings and errors;

All jobs (composer, eslint, phpcs, phpstan) work and are green now.
Marking as "needs review". Thanks!

bohart → created an issue.

@Andrii Momotov , thanks for your contribution!

Tests are failed, so mark this as "needs work".

Hi @Abyss and @Quadrex,
thanks for your contributions!

A few further notes:
1. Let's move RerouteEmailPolicyTestTrait into Policy folder, as it is used only there.

2. policyConfigurationEditFormPath is used only in the trait.
Why do we need to declare it in 6 tests?

3. overridePolicyLink is used only once.
Please move it from the class declaration to the setup method.

4. Do we really need to specify strictConfigSchema now?

5. It looks like 2 usages of getRerouteConfiguration() should be updated.
(commented in MR)

6. .gitlab-ci.yml updated in the wrong way. Tests are not running now.

Thanks!

The only remaining task is 🐛 Add tests for Drupal Symfony Mailer (if non global reroute configuration is used) Active
But it's a new test request, so it's not a blocker to release the new version.

So, 2.3.0-rc1 release was created a few moments ago:
https://www.drupal.org/project/reroute_email/releases/2.3.0-rc1 →

Thanks @all for the contribution!

Committed to 2.3.x dev branch. It will be a part of the next 2.3 series releases.

@Eduardo Morales Alberti, thanks for your contribution!

It looks like the issue was fixed as a part of 📌 Adapt ContactFormTest, CaseSensitivityTest, MailKeysTest, TestEmailFormTest tests for 'Reroute Email (Symfony Mailer support)' module. Fixed
Committed to 2.3.x dev branch and will be a part of the next 2.3 series releases.

Commit: 6026d713658947a8868c98ecab70fd9d50bb1cc9 (Plugin/RerouteEmailHandler/SymfonyMailer.php, lines 180-195)

Please re-open the issue or raise a new one if any new problems occur.
Thanks!

@quadrexdev, thanks for your contribution!
Committed to 2.3.x dev branch. It will be a part of the next 2.3 series releases.

Thanks!

Fixed as a part of 📌 Adapt ContactFormTest, CaseSensitivityTest, MailKeysTest, TestEmailFormTest tests for 'Reroute Email (Symfony Mailer support)' module. Fixed

Commit: 6026d713658947a8868c98ecab70fd9d50bb1cc9

bohart → created an issue.

Hi @nord102,

I'm assume this is fixed on the dev version of the module (it will be released as a new stable version in the nearest weeks).
As "use_global" should always be present now. Still, it might be an excellent catch to double-check the default value everywhere.

The branch of MR !72 has been rebased and MR merged.
So, the changes are committed to 2.3.x dev branch.
They will be a part of the next 2.3 series releases.

Thanks!

bohart → made their first commit to this issue’s fork.

@quadrexdev, thanks for your contribution!
Committed to 2.3.x dev branch. It will be a part of the next 2.3 series releases.

We will also need to handle a new test InvalidAddressesTest for Symfony Mailer submodule,
but it will be a part of 📌 SymfonyMailerTestEmailFormTest skipped 6 tests to run Active .

Thanks!

Fixed as a part of 🐛 Incorrect converting the comma-separated emails into Address class Needs review

No backport to 2.2.x is needed.

bohart → created an issue.

Hi @quadrexdev,
thanks for your contribution!

1) reroute_email_extract_addresses function is located in the module files as a legacy artifact.
Let's move it as a method to RerouteEmailHandlerPluginBase (this is the only place where it is used).

2) We should cover this bug with a test. There is MultipleRecipientsTest for hook_mail_alter implementation.
This test should be added to reroute_email_symfony_mailer tests.

Thanks!

Committed to 2.3.x dev branch. It will be a part of the next 2.3 series releases.
Thanks!

Hi @Abyss,

1) <body> can have classes, styles, or some additional attributes.
In this case, the code will not work correctly.
Let's use preg_match / preg_replace for the proper check and insertion.

2) If we insert the text inside <body> tag, we should wrap our plain text with <br/> tags to have the correct displaying in the email.
Once we do this, let's update RerouteEmailHandlerPluginInterface::prependBody method description by adding the proper @params for consistency (it should say that the prepend text value should be a plain text).

3) Please rename the input argument for the method prependBody from string $body to string $prepend. Because it's a bit confusing now (as it is not actually an email body value).
Also, let's update the method description in the interface (it says "Set email body." right now, and this is not correct; it should be something like this "Prepends the specified text to the email body.").

  /**
   * Prepends the specified text to the email body.
   *
   * @param string $prepend
   *   A plain text to be prepended to the email body. Line break tags will be added automatically in the case of HTML emails.
   */
  public function prependBody(string $prepend): void;

Committed to 2.3.x dev branch. It will be a part of the next 2.3 series releases.
Thanks!