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

Confusing default configuration values #66

Closed
emilv opened this issue Mar 20, 2018 · 4 comments
Closed

Confusing default configuration values #66

emilv opened this issue Mar 20, 2018 · 4 comments

Comments

@emilv
Copy link

emilv commented Mar 20, 2018

The library assumes that there will exist a default account, and that account must have all properties configured.

In particular I tried to use this configuration:


    'accounts' => [
        'default' => [
            'port' => 993,
            'encryption' => 'tls',
            'validate_cert' => true,
        ],
        'grappling' => [
            'host' => 'mail.example.com',
            'username' => 'blaha@example.com',
            'password' => env('IMAP_GRAPPLING_PASSWORD', null),
        ],
    ],

But in Client::setConfig only configuration properties from the default account will ever be set.

Expected behavior is that all the values from both grappling are used, with default values from default.

@Webklex
Copy link
Owner

Webklex commented Mar 20, 2018

Hi @emilv ,
please make sure you published the package config file so it is available in your laravel config folder (config/imap.php) and take a look at the original config file: src/config/imap.php

You can define several accounts inside the accounts array. The default account which is currently placed inside, describes the account which will be used if none is specified.

If you are only using one account you don't have to add any account information inside the imap.php config file, because you can use the env-variables to archive the same.

You usually don't have to use Client::setConfig() at all - only if you want to manipulate configs during the runtime..

Hope I could help :)

Update:
..maybe calling the default account identifier default wan't the best name but how else would you name it..?

@emilv
Copy link
Author

emilv commented Mar 27, 2018

I don't care about the default account. The confusion here is that the only config keys you are allowed to set are those that are defined on the default account. Look again on my example. I removed host, username and password from the default account. That makes it impossible to set those same values on the grappling account.

This is a leaky abstraction. Instead of letting the code itself know valid config keys you rely on the `default account to have those keys.

@Webklex
Copy link
Owner

Webklex commented Mar 27, 2018

Ah I see. Yes you're right. I will move to a fixed config set - something like this:

public function setConfig(array $config) {
        $defaultAccount = config('imap.default');
        $defaultConfig  = config("imap.accounts.$defaultAccount");

        $availableConfig = ['host', 'port', 'encryption', 'validate_cert', 'username', 'password'];

        foreach($availableConfig as $key){
            $this->$key = isset($config[$key]) ? $config[$key] : $defaultConfig[$key];
        }

        return $this;
}

@Webklex
Copy link
Owner

Webklex commented Mar 28, 2018

Hi @emilv ,
I just released a new version which handles the "config setting process" a bit different. Its now based on fixed keys :)

Ref.: v. 1.0.5.5

@Webklex Webklex closed this as completed Mar 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants