diff --git a/lib/Config/Widget.php b/lib/Config/Widget.php index b1f4fa3ae2..f4b9fd2943 100644 --- a/lib/Config/Widget.php +++ b/lib/Config/Widget.php @@ -12,7 +12,6 @@ class Widget { $this->registerWidget(); if(!is_admin()) { - //$this->setupActions(); $this->setupDependencies(); } else { $this->setupAdminDependencies(); @@ -69,6 +68,9 @@ class Widget { } } + // TODO: extract this method into an Initializer + // - the "ajax" part might probably be useless + // - the "post" (non-ajax) part needs to be redone properly function setupActions() { // ajax requests add_action( diff --git a/lib/Models/Model.php b/lib/Models/Model.php index 94936a06b2..e3d3237377 100644 --- a/lib/Models/Model.php +++ b/lib/Models/Model.php @@ -39,7 +39,6 @@ class Model extends \Sudzy\ValidModel { $this->setTimestamp(); try { parent::save(); - return true; } catch(\Sudzy\ValidationException $e) { $this->setError($e->getValidationErrors()); } catch(\PDOException $e) { @@ -47,7 +46,7 @@ class Model extends \Sudzy\ValidModel { } catch(\Exception $e) { $this->setError($e->getMessage()); } - return false; + return $this; } function trash() { diff --git a/lib/Models/NewsletterTemplate.php b/lib/Models/NewsletterTemplate.php index 4649104845..7e4d3ca3c7 100644 --- a/lib/Models/NewsletterTemplate.php +++ b/lib/Models/NewsletterTemplate.php @@ -32,16 +32,7 @@ class NewsletterTemplate extends Model { $template->set($data); } - $saved = $template->save(); - - if($saved === true) { - return true; - } else { - $errors = $template->getValidationErrors(); - if(!empty($errors)) { - return $errors; - } - } - return false; + $template->save(); + return $template; } } diff --git a/lib/Models/Segment.php b/lib/Models/Segment.php index f9dce06c84..615983bdac 100644 --- a/lib/Models/Segment.php +++ b/lib/Models/Segment.php @@ -164,8 +164,7 @@ class Segment extends Model { $segment->set($data); } - $segment->save(); - return $segment; + return $segment->save(); } static function getPublic() { diff --git a/lib/Models/Setting.php b/lib/Models/Setting.php index 8b5e6e7e88..a9191ace03 100644 --- a/lib/Models/Setting.php +++ b/lib/Models/Setting.php @@ -10,8 +10,7 @@ class Setting extends Model { parent::__construct(); $this->addValidations('name', array( - 'required' => 'name_is_blank', - 'isString' => 'name_is_not_string' + 'required' => __('You need to specify a name.') )); } @@ -55,10 +54,11 @@ class Setting extends Model { $value = serialize($value); } - return Setting::createOrUpdate(array( + $setting = Setting::createOrUpdate(array( 'name' => $key, 'value' => $value )); + return ($setting->id() > 0 && $setting->getErrors() === false); } else { $main_key = array_shift($keys); @@ -101,8 +101,7 @@ class Setting extends Model { } public static function createOrUpdate($model) { - $exists = self::where('name', $model['name']) - ->find_one(); + $exists = self::where('name', $model['name'])->findOne(); if($exists === false) { $new_model = self::create(); diff --git a/lib/Router/Forms.php b/lib/Router/Forms.php index 31b53b465f..c266de15ba 100644 --- a/lib/Router/Forms.php +++ b/lib/Router/Forms.php @@ -202,37 +202,28 @@ class Forms { } function restore($id) { - $result = false; - $form = Form::findOne($id); if($form !== false) { - $result = $form->restore(); + $form->restore(); } - - return $result; + return ($form->getErrors() === false); } function trash($id) { - $result = false; - $form = Form::findOne($id); if($form !== false) { - $result = $form->trash(); + $form->trash(); } - - return $result; + return ($form->getErrors() === false); } function delete($id) { - $result = false; - $form = Form::findOne($id); if($form !== false) { $form->delete(); - $result = 1; + return 1; } - - return $result; + return false; } function duplicate($id) { diff --git a/lib/Router/Newsletters.php b/lib/Router/Newsletters.php index 1d1fb8c05f..003886f064 100644 --- a/lib/Router/Newsletters.php +++ b/lib/Router/Newsletters.php @@ -85,17 +85,17 @@ class Newsletters { NewsletterOption::where('newsletter_id', $newsletter->id) ->deleteMany(); - $optionFields = NewsletterOptionField::where( + $option_fields = NewsletterOptionField::where( 'newsletter_type', $data['type'] )->findArray(); - foreach($optionFields as $optionField) { - if(isset($options[$optionField['name']])) { + foreach($option_fields as $option_field) { + if(isset($options[$option_field['name']])) { $relation = NewsletterOption::create(); $relation->newsletter_id = $newsletter->id; - $relation->option_field_id = $optionField['id']; - $relation->value = $options[$optionField['name']]; + $relation->option_field_id = $option_field['id']; + $relation->value = $options[$option_field['name']]; $relation->save(); } } @@ -263,15 +263,14 @@ class Newsletters { $sender = false, $reply_to = false ); + $result = $mailer->send($newsletter, $data['subscriber']) - wp_send_json(array( - 'result' => $mailer->send($newsletter, $data['subscriber']) - )); + return array('result' => $result); } catch(\Exception $e) { - wp_send_json(array( + return array( 'result' => false, 'errors' => array($e->getMessage()), - )); + ); } } @@ -299,7 +298,7 @@ class Newsletters { $item['queue'] = ($queue !== false) ? $queue->asArray() : null; } - wp_send_json($listing_data); + return $listing_data; } function bulkAction($data = array()) { @@ -307,7 +306,7 @@ class Newsletters { '\MailPoet\Models\Newsletter', $data ); - wp_send_json($bulk_action->apply()); + return $bulk_action->apply(); } function create($data = array()) { @@ -328,25 +327,28 @@ class Newsletters { unset($data['options']); } - $result = $newsletter->save(); - if($result !== true) { - wp_send_json($newsletter->getValidationErrors()); + $newsletter->save(); + $errors = $newsletter->getErrors(); + if(!empty($errors)) { + return $errors; } else { if(!empty($options)) { - $optionFields = NewsletterOptionField::where('newsletter_type', $newsletter->type)->findArray(); + $option_fields = NewsletterOptionField::where( + 'newsletter_type', $newsletter->type + )->findArray(); - foreach($optionFields as $optionField) { - if(isset($options[$optionField['name']])) { + foreach($option_fields as $option_field) { + if(isset($options[$option_field['name']])) { $relation = NewsletterOption::create(); $relation->newsletter_id = $newsletter->id; - $relation->option_field_id = $optionField['id']; - $relation->value = $options[$optionField['name']]; + $relation->option_field_id = $option_field['id']; + $relation->value = $options[$option_field['name']]; $relation->save(); } } } $newsletter->body = json_decode($newsletter->body); - wp_send_json($newsletter->asArray()); + return $newsletter->asArray(); } } } \ No newline at end of file diff --git a/lib/Router/Segments.php b/lib/Router/Segments.php index ff59cca24e..d3930171a5 100644 --- a/lib/Router/Segments.php +++ b/lib/Router/Segments.php @@ -87,37 +87,28 @@ class Segments { } function restore($id) { - $result = false; - $segment = Segment::findOne($id); if($segment !== false) { - $result = $segment->restore(); + $segment->restore(); } - - return $result; + return ($segment->getErrors() === false); } function trash($id) { - $result = false; - $segment = Segment::findOne($id); if($segment !== false) { - $result = $segment->trash(); + $segment->trash(); } - - return $result; + return ($segment->getErrors() === false); } function delete($id) { - $result = false; - $segment = Segment::findOne($id); if($segment !== false) { $segment->delete(); - $result = 1; + return 1; } - - return $result; + return false; } function duplicate($id) { diff --git a/lib/Router/Subscribers.php b/lib/Router/Subscribers.php index eb2b88bb62..35177c4ad3 100644 --- a/lib/Router/Subscribers.php +++ b/lib/Router/Subscribers.php @@ -191,37 +191,28 @@ class Subscribers { } function restore($id) { - $result = false; - $subscriber = Subscriber::findOne($id); if($subscriber !== false) { - $result = $subscriber->restore(); + $subscriber->restore(); } - - return $result; + return ($subscriber->getErrors() === false); } function trash($id) { - $result = false; - $subscriber = Subscriber::findOne($id); if($subscriber !== false) { - $result = $subscriber->trash(); + $subscriber->trash(); } - - return $result; + return ($subscriber->getErrors() === false); } function delete($id) { - $result = false; - $subscriber = Subscriber::findOne($id); if($subscriber !== false) { $subscriber->delete(); - $result = 1; + return 1; } - - return $result; + return false; } function bulkAction($data = array()) { diff --git a/tests/unit/Models/CustomFieldCest.php b/tests/unit/Models/CustomFieldCest.php index 90b5f3b723..ff9a02a336 100644 --- a/tests/unit/Models/CustomFieldCest.php +++ b/tests/unit/Models/CustomFieldCest.php @@ -30,7 +30,8 @@ class CustomFieldCest { } function itCanBeCreated() { - expect($this->saved)->equals(true); + expect($this->saved->id() > 0)->true(); + expect($this->saved->getErrors())->false(); } function itCanHaveName() { @@ -46,10 +47,14 @@ class CustomFieldCest { } function itHasToBeValid() { - $empty_model = CustomField::create(); - expect($empty_model->save())->notEquals(true); - $validations = $empty_model->getValidationErrors(); - expect(count($validations))->equals(2); + $invalid_custom_field = CustomField::create(); + + $result = $invalid_custom_field->save(); + $errors = $result->getErrors(); + + expect(is_array($errors))->true(); + expect($errors[0])->equals('You need to specify a name.'); + expect($errors[1])->equals('You need to specify a type.'); } function itHasACreatedAtOnCreation() { diff --git a/tests/unit/Models/FormCest.php b/tests/unit/Models/FormCest.php index ddc42d03b8..67e29c860c 100644 --- a/tests/unit/Models/FormCest.php +++ b/tests/unit/Models/FormCest.php @@ -14,15 +14,17 @@ class FormCest { } function itCanBeCreated() { - expect($this->saved)->equals(true); + expect($this->saved->id() > 0)->true(); + expect($this->saved->getErrors())->false(); } function itHasToBeValid() { - expect($this->saved)->equals(true); - $empty_model = Form::create(); - expect($empty_model->save())->notEquals(true); - $validations = $empty_model->getValidationErrors(); - expect(count($validations))->equals(1); + $invalid_form = Form::create(); + $result = $invalid_form->save(); + $errors = $result->getErrors(); + + expect(is_array($errors))->true(); + expect($errors[0])->equals('You need to specify a name.'); } function itHasACreatedAtOnCreation() { diff --git a/tests/unit/Models/NewsletterCest.php b/tests/unit/Models/NewsletterCest.php index 92dad6f0e8..835f99bc2d 100644 --- a/tests/unit/Models/NewsletterCest.php +++ b/tests/unit/Models/NewsletterCest.php @@ -19,11 +19,12 @@ class NewsletterCest { $newsletter = Newsletter::create(); $newsletter->hydrate($this->data); $this->newsletter = $newsletter; - $this->result = $newsletter->save(); + $this->saved = $newsletter->save(); } function itCanBeCreated() { - expect($this->result)->equals(true); + expect($this->saved->id() > 0)->true(); + expect($this->saved->getErrors())->false(); } function itHasSubject() { diff --git a/tests/unit/Models/NewsletterOptionFieldCest.php b/tests/unit/Models/NewsletterOptionFieldCest.php index 40462446b3..ceb0508d3d 100644 --- a/tests/unit/Models/NewsletterOptionFieldCest.php +++ b/tests/unit/Models/NewsletterOptionFieldCest.php @@ -29,7 +29,8 @@ class NewsletterOptionFieldCest { } function itCanBeCreated() { - expect($this->saved)->equals(true); + expect($this->saved->id() > 0)->true(); + expect($this->saved->getErrors())->false(); } function itHasName() { @@ -45,11 +46,13 @@ class NewsletterOptionFieldCest { } function itHasToBeValid() { - expect($this->saved)->equals(true); - $empty_model = NewsletterOptionField::create(); - expect($empty_model->save())->notEquals(true); - $validations = $empty_model->getValidationErrors(); - expect(count($validations))->equals(2); + $invalid_newsletter_option = NewsletterOptionField::create(); + $result = $invalid_newsletter_option->save(); + $errors = $result->getErrors(); + + expect(is_array($errors))->true(); + expect($errors[0])->equals('You need to specify a name.'); + expect($errors[1])->equals('You need to specify a newsletter type.'); } function itHasACreatedAtOnCreation() { diff --git a/tests/unit/Models/NewsletterTemplateCest.php b/tests/unit/Models/NewsletterTemplateCest.php index b470afa1e7..704fa49757 100644 --- a/tests/unit/Models/NewsletterTemplateCest.php +++ b/tests/unit/Models/NewsletterTemplateCest.php @@ -13,18 +13,22 @@ class NewsletterTemplateCest { $template = NewsletterTemplate::create(); $template->hydrate($this->data); - $this->result = $template->save(); + $this->saved = $template->save(); } function itCanBeCreated() { - expect($this->result)->equals(true); + expect($this->saved->id() > 0)->true(); + expect($this->saved->getErrors())->false(); } function itHasToBeValid() { - $empty_model = NewsletterTemplate::create(); - expect($empty_model->save())->notEquals(true); - $validations = $empty_model->getValidationErrors(); - expect(count($validations))->equals(2); + $invalid_newsletter_template = NewsletterTemplate::create(); + $result = $invalid_newsletter_template->save(); + $errors = $result->getErrors(); + + expect(is_array($errors))->true(); + expect($errors[0])->equals('You need to specify a name.'); + expect($errors[1])->equals('Template body cannot be empty.'); } function itHasName() { @@ -46,25 +50,28 @@ class NewsletterTemplateCest { } function itCanCreateOrUpdate() { - $is_created = NewsletterTemplate::createOrUpdate( + $created_template = NewsletterTemplate::createOrUpdate( array( 'name' => 'Another template', 'description' => 'Another template description', 'body' => '{content: {}, globalStyles: {}}', )); - expect($is_created)->equals(true); + expect($created_template->id() > 0)->true(); + expect($created_template->getErrors())->false(); $template = NewsletterTemplate::where('name', 'Another template') ->findOne(); expect($template->name)->equals('Another template'); - $is_updated = NewsletterTemplate::createOrUpdate( + $updated_template = NewsletterTemplate::createOrUpdate( array( 'id' => $template->id, 'name' => 'Another template updated', 'body' => '{}' )); - expect($is_updated)->equals(true); + expect($updated_template->id() > 0)->true(); + expect($updated_template->getErrors())->false(); + $template = NewsletterTemplate::findOne($template->id); expect($template->name)->equals('Another template updated'); } diff --git a/tests/unit/Models/SegmentCest.php b/tests/unit/Models/SegmentCest.php index fc1e1fe8bc..ee150db381 100644 --- a/tests/unit/Models/SegmentCest.php +++ b/tests/unit/Models/SegmentCest.php @@ -41,7 +41,8 @@ class SegmentCest { } function itCanBeCreated() { - expect($this->saved)->equals(true); + expect($this->saved->id() > 0)->true(); + expect($this->saved->getErrors())->false(); } function itCanHaveName() { @@ -51,7 +52,9 @@ class SegmentCest { function nameMustBeUnique() { $segment = Segment::create(); $segment->hydrate($this->data); - $errors = $segment->save(); + $result = $segment->save(); + $errors = $result->getErrors(); + expect(is_array($errors))->true(); expect($errors[0])->contains('Duplicate'); } @@ -61,11 +64,13 @@ class SegmentCest { } function itHasToBeValid() { - expect($this->saved)->equals(true); - $empty_model = Segment::create(); - expect($empty_model->save())->notEquals(true); - $validations = $empty_model->getValidationErrors(); - expect(count($validations))->equals(1); + $invalid_segment = Segment::create(); + + $result = $invalid_segment->save(); + $errors = $result->getErrors(); + + expect(is_array($errors))->true(); + expect($errors[0])->equals('You need to specify a name.'); } function itHasACreatedAtOnCreation() { diff --git a/tests/unit/Models/SettingCest.php b/tests/unit/Models/SettingCest.php index 1ece7a95b2..69c02e0ac6 100644 --- a/tests/unit/Models/SettingCest.php +++ b/tests/unit/Models/SettingCest.php @@ -11,19 +11,21 @@ class SettingCest { $setting = Setting::create(); $setting->hydrate($this->data); - $this->result = $setting->save(); + $this->saved = $setting->save(); } function itCanBeCreated() { - expect($this->result)->equals(true); + expect($this->saved->id() > 0)->true(); + expect($this->saved->getErrors())->false(); } function itHasToBeValid() { - expect($this->result)->equals(true); - $empty_model = Setting::create(); - expect($empty_model->save())->notEquals(true); - $validations = $empty_model->getValidationErrors(); - expect(count($validations))->equals(2); + $invalid_setting = Setting::create(); + $result = $invalid_setting->save(); + $errors = $result->getErrors(); + + expect(is_array($errors))->true(); + expect($errors[0])->equals('You need to specify a name.'); } function itHasACreatedAtOnCreation() { @@ -65,18 +67,20 @@ class SettingCest { 'value' => 'data' ); - $result = Setting::createOrUpdate($data); - expect($result)->equals(true); - $record = Setting::where('name', $data['name']) - ->find_one(); - expect($record->value)->equals($data['value']); + $created_setting = Setting::createOrUpdate($data); + expect($created_setting->id() > 0)->true(); + expect($created_setting->getErrors())->false(); + + $setting = Setting::where('name', $data['name'])->findOne(); + expect($setting->value)->equals($data['value']); $data['value'] = 'new data'; - $result = Setting::createOrUpdate($data); - expect($result)->equals(true); - $record = Setting::where('name', $data['name']) - ->find_one(); - expect($record->value)->equals('new data'); + $updated_setting = Setting::createOrUpdate($data); + expect($updated_setting->id() > 0)->true(); + expect($updated_setting->getErrors())->false(); + + $setting = Setting::where('name', $data['name'])->findOne(); + expect($setting->value)->equals('new data'); } function itCanGetAndSetValue() { diff --git a/tests/unit/Models/SubscriberCest.php b/tests/unit/Models/SubscriberCest.php index 7a4be4af97..c372d9630e 100644 --- a/tests/unit/Models/SubscriberCest.php +++ b/tests/unit/Models/SubscriberCest.php @@ -20,7 +20,8 @@ class SubscriberCest { } function itCanBeCreated() { - expect($this->saved)->true(); + expect($this->saved->id() > 0)->true(); + expect($this->saved->getErrors())->false(); } function itHasFirstName() { @@ -62,14 +63,13 @@ class SubscriberCest { } function itCanChangeStatus() { - $subscriber = Subscriber::where('email', $this->data['email']) - ->findOne(); + $subscriber = Subscriber::where('email', $this->data['email'])->findOne(); $subscriber->status = 'subscribed'; - expect($subscriber->save())->equals(true); - $subscriber_updated = Subscriber::where( - 'email', - $this->data['email'] - ) + $subscriber->save(); + + expect($subscriber->id() > 0)->true(); + expect($subscriber->getErrors())->false(); + $subscriber_updated = Subscriber::where('email', $this->data['email']) ->findOne(); expect($subscriber_updated->status)->equals('subscribed'); } @@ -89,17 +89,17 @@ class SubscriberCest { function itHasGroupFilter() { $subscribers = Subscriber::filter('groupBy', 'unconfirmed') ->findMany(); - foreach ($subscribers as $subscriber) { + foreach($subscribers as $subscriber) { expect($subscriber->status)->equals('unconfirmed'); } $subscribers = Subscriber::filter('groupBy', 'subscribed') ->findMany(); - foreach ($subscribers as $subscriber) { + foreach($subscribers as $subscriber) { expect($subscriber->status)->equals('subscribed'); } $subscribers = Subscriber::filter('groupBy', 'unsubscribed') ->findMany(); - foreach ($subscribers as $subscriber) { + foreach($subscribers as $subscriber) { expect($subscriber->status)->equals('unsubscribed'); } } @@ -150,7 +150,7 @@ class SubscriberCest { 'type' => 'text', ) ); - foreach ($customFieldData as $data) { + foreach($customFieldData as $data) { $customField = CustomField::create(); $customField->hydrate($data); $customField->save(); @@ -168,7 +168,7 @@ class SubscriberCest { 'value' => 'France' ) ); - foreach ($subscriberCustomFieldData as $data) { + foreach($subscriberCustomFieldData as $data) { $association = SubscriberCustomField::create(); $association->hydrate($data); $association->save(); diff --git a/tests/unit/Models/SubscriberCustomFieldCest.php b/tests/unit/Models/SubscriberCustomFieldCest.php index 32e934812d..eb871ffd69 100644 --- a/tests/unit/Models/SubscriberCustomFieldCest.php +++ b/tests/unit/Models/SubscriberCustomFieldCest.php @@ -20,7 +20,9 @@ class SubscriberCustomFieldCest { function itCanBeCreated() { $subscriberCustomField = SubscriberCustomField::create(); $subscriberCustomField->hydrate($this->data[0]); - expect($subscriberCustomField->save())->true(); + $subscriberCustomField->save(); + expect($subscriberCustomField->id() > 0)->true(); + expect($subscriberCustomField->getErrors())->false(); } function itCanCreateOrUpdateMultipleRecords() {