Currently you configure a fetcher and have no idea if it works.

All mail MTA have a "test connection" button that confirms credentials are fine.

Followup from #2379889: Fetch email by IMAP

Proposed resolution

Add a test connection button.

Remaining tasks

User interface changes

API changes

Members fund testing for the Drupal project. Drupal Association Learn more


miro_dietiker’s picture

Component: User interface » Deliverer
miro_dietiker’s picture

Hint: There's an imap_ping() method to test a connection.

mbovan’s picture

Status: Active » Needs review
3.39 KB
22.49 KB

Added a "Connect" button.

imap_ping() requires opening an IMAP connection, so it seems doImap() call is enough. It fails in case connection cannot be open.

Also, about the tests, I'm only asserting there is a "Connect" button. PHP IMAP extension is needed to test it properly which doesn't seem to be available in tests...

miro_dietiker’s picture

Status: Needs review » Needs work

We should move all connection credential specific elements into a fieldset and put the Connect button there as last element.

For instance, the Batch size is NOT related to the connection credentials. :-)

Yeah, the IMAP php module isn't that common on server environments... Likely it's missing on testbot.
Testing this would need abstracting it / with mocking capability. Seems to be way too much overhead.

mbovan’s picture

Makes sense. :)

Added a fieldset:

miro_dietiker’s picture

Status: Needs review » Needs work

If i click on "Connect" with wrong credentials, i see no error message. We need one.

The fieldset "IMAP" is repetitive. Let's name it "Account".

I think the button should be labeled "Test connection", because it does not save...

mbovan’s picture

Implemented suggestions above.

Berdir’s picture

+++ b/src/Plugin/inmail/Deliverer/FetcherBase.php
@@ -111,4 +111,34 @@ abstract class FetcherBase extends DelivererBase implements FetcherInterface {
+   */
+  public function submitTestConnection(array $form, FormStateInterface $form_state) {
+    // Implement it in the subclass.
+  }

maybe throw an exception here, so implementations see what they need to do if they forget?

mbovan’s picture

miro_dietiker’s picture

Status: Needs review » Fixed

Committed! Thx for adding. :-)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.