secit-pl / imap-bundle

Simple php-imap integration for Symfony
MIT License
71 stars 20 forks source link

Add enabled flag #41

Closed antedebaas closed 7 months ago

antedebaas commented 8 months ago

I need a flag to check if a config is enabled.

secit-pl commented 8 months ago

Please update the documentation before merge.

Also, I think there should be some logic behind this "flag". Please add checking if connection is enabled in methods getMailbox, testConnection and tryTestConnection and add the column "enabled" in the ValidateConnectionsCommand.

antedebaas commented 8 months ago

there you go. renamed it to enabled so the column matches the config

antedebaas commented 7 months ago

@secit-pl friendly ping

secit-pl commented 7 months ago

Changes looks ok. One thought before merge, shouldn't there go some logic behind the isEnabled()? At this moment the flag doesn't do anything. Maybe we should block possibility to get the mailbox (getMailbox() -> if (!$connection->isEnabled()) throw \ErrorException('connection disabled')) and do not initiate connection on the test command for disabled connections? WDYT?

antedebaas commented 7 months ago

I agree; Originally I had the idea of using the flag on my project side. But for this it makes more sense to add logic to it here so I added code for it. i also modified the validate-connection command not to try and open the mailbox when its disabled

secit-pl commented 7 months ago

Merged. Thank you for your contribution!