added a Settings-Test - #1
Conversation
With checking a box and press save in the modules config page, a testconnection to MailChimp gets invoked, that pulls the list name, list id and adress settings. If settings are wrong, it reports errors. For example a 401 or 404.
danielstieber
left a comment
danielstieber
left a comment
There was a problem hiding this comment.
I commented some questions, mostly because of my limited knowledge in module dev. Maybe you can have a look before I merge it.
| try { | ||
| $http = new WireHttp(); | ||
| $http->setHeaders([ | ||
| //'Content-Type' => 'application/json', |
There was a problem hiding this comment.
Is this line commented because it is not necessary? Should I remove it from my get requests too?
There was a problem hiding this comment.
No, your requests have post data that is of type json, so you want to keep it. But I only sent a GET request without other data. Please remove may comment here.
| $mc = wire('modules')->get('SubscribeToMailchimp'); | ||
| $list = is_null($list) ? $mc->default_list : $list; | ||
| $dc = explode('-',$mc->api_key)[1]; // get the datacenter code from the api key, to use it in the api url | ||
| $request = "https://{$dc}.api.mailchimp.com/3.0/lists/{$list}/"; |
There was a problem hiding this comment.
I made the getApiBase() method to centralize the Mailchimp API path to faster manage API version upgrades. Can/should we use this here too?
There was a problem hiding this comment.
Yep, at first I used it, but it returns to much. I only need the base, but your method also returns /{$list}/members/. Maybe you can modify the getApiBase() method with an optional param, so that it returns a base URL, with optionally added string(s) to it:
private function getApiBase($list = null, $param = null) {
$dc = explode('-',$this->api_key)[1];
$return = "https://".$dc.".api.mailchimp.com/3.0/";
if(!is_null($list)) $return .= "lists/".$list."/";
if(!is_null($param)) $return .= $param; // should we automatically add a trailing slash here ?
return $return;
}
There was a problem hiding this comment.
aah, got you. thats great yeah.
| // static, because mainly it gets called from static function getModuleConfigInputfields! | ||
| // But also can get called from within your template files: $response = SubscribeToMailchimp::testSettings($listID); | ||
| public static function testSettings($list = null) { | ||
| $mc = wire('modules')->get('SubscribeToMailchimp'); |
There was a problem hiding this comment.
Do I need this, since the function is already inside the module? Or is it with intent to simulate a normal use case?
There was a problem hiding this comment.
The getModuleConfig() is a static method, therefore we need to declare and call the testSettings() method also static. Static methods do not have access to the class (default) params via $this->. But we need $this->api_key, $this->getApiBase($list), or others. Therefore I use $mc as a dynamic(?) class instance. If you have another idea, please tell me or rework my code.
2 participants
With checking a box and press save in the modules config page, a testconnection to MailChimp gets invoked, that pulls the list name, list id and adress settings. If settings are wrong, it reports errors. For example a 401 or 404.