Move validation of Mage_Customer_Model_Address to another class #17

Closed
stalniy opened this Issue May 7, 2012 · 8 comments

Comments

Projects
None yet
3 participants
@stalniy

stalniy commented May 7, 2012

There is no ability to change validation rules for Mage_Customer_Model_Address (Mage_Customer_Model_Address_Abstract::validate). For example, I changed "is_required" field for attribute "lastname" to false and when i was trying to place order without lastname field i got an error: Please enter the last name.

I want to customize validate logic and there is only 2 bad ways (rewrite model, place the same file to app/code/local). I think the best solution is validator. Something like this should be (pseudo code):

 class AddressForm {
      public function __construct($model) {
           $this->_configure();
           $this->_fillByModel();
      }

      protected function _configure() {
           $this->addField('firstname')->addValidator(new ValidatorEmpty());
           ....................................(the same for each field that i want to see in form and validate)
      }

      public function isValid() {
           for each validator do {
                $validator->validate();
           }
           return $result;
      }
 }

Usage:
$form = new AddressForm($quote->getBillingAddress());
if ($form->isValid()) {
$quote->getBillingAddress()->save();
}

I think that it's better if validation process is in controller then it will easy to change validation logic. More easy solution for you make checking address attributes for required flag and another rules

@ghost ghost assigned magento-team May 9, 2012

@magento-team

This comment has been minimized.

Show comment Hide comment
@magento-team

magento-team May 9, 2012

Contributor

Address validation is implemented in Mage_Customer_Model_Address_Abstract::validate() without the ability to customize validation rules in order to guarantee presence of the required data for any payment method. For instance, last name field cannot be optional because some payment methods require value for it (for example, take a look at "Payer Information Fields" section of the PayPal doc https://cms.paypal.com/us/cgi-bin/?cmd=_render-content&content_ID=developer/e_howto_api_nvp_r_DoDirectPayment).

Contributor

magento-team commented May 9, 2012

Address validation is implemented in Mage_Customer_Model_Address_Abstract::validate() without the ability to customize validation rules in order to guarantee presence of the required data for any payment method. For instance, last name field cannot be optional because some payment methods require value for it (for example, take a look at "Payer Information Fields" section of the PayPal doc https://cms.paypal.com/us/cgi-bin/?cmd=_render-content&content_ID=developer/e_howto_api_nvp_r_DoDirectPayment).

@stalniy

This comment has been minimized.

Show comment Hide comment
@stalniy

stalniy May 10, 2012

What must i do if I dont want to use any online payment method or i want to use payment method that accept only one field - "name"?

stalniy commented May 10, 2012

What must i do if I dont want to use any online payment method or i want to use payment method that accept only one field - "name"?

@magento-team

This comment has been minimized.

Show comment Hide comment
@magento-team

magento-team May 11, 2012

Contributor

If you don't want to be compatible with online payment methods, as you mentioned previously, class Mage_Customer_Model_Address_Abstract can be rewritten to customize method validate().

Contributor

magento-team commented May 11, 2012

If you don't want to be compatible with online payment methods, as you mentioned previously, class Mage_Customer_Model_Address_Abstract can be rewritten to customize method validate().

@stalniy

This comment has been minimized.

Show comment Hide comment
@stalniy

stalniy May 11, 2012

If I want to customize standard checkout then i need to rewrite a lot of code. Because this method "validation" are called everywhere. I need to rewrite quote and quote addresses because class (alias) for quote addresses also hard coded in quote class and maybe some another classes.
Do you think it's ok for getting possibility to change only one method?

stalniy commented May 11, 2012

If I want to customize standard checkout then i need to rewrite a lot of code. Because this method "validation" are called everywhere. I need to rewrite quote and quote addresses because class (alias) for quote addresses also hard coded in quote class and maybe some another classes.
Do you think it's ok for getting possibility to change only one method?

@stalniy

This comment has been minimized.

Show comment Hide comment
@stalniy

stalniy May 11, 2012

You said that customer class depends from payment methods. Do you think it's ok?

stalniy commented May 11, 2012

You said that customer class depends from payment methods. Do you think it's ok?

@stalniy

This comment has been minimized.

Show comment Hide comment
@stalniy

stalniy May 11, 2012

Ok, the easiest solution:

class Mage_Customer_Model_Address_Abstract  {

          public function validate() {
               return this->getValidator()->validate($this);
          }

          public function setValidator(Mage_Core_Model_Validator_Interface $validator) {
               $this->_validator = $validator;
          }

          public function getValidator() {
               return $this->_validator ? $this->_validator : new Mage_Customer_Model_Address_Validator()
          }
}

And put logic from validate method to Mage_Customer_Model_Address_Validator class into method validate

stalniy commented May 11, 2012

Ok, the easiest solution:

class Mage_Customer_Model_Address_Abstract  {

          public function validate() {
               return this->getValidator()->validate($this);
          }

          public function setValidator(Mage_Core_Model_Validator_Interface $validator) {
               $this->_validator = $validator;
          }

          public function getValidator() {
               return $this->_validator ? $this->_validator : new Mage_Customer_Model_Address_Validator()
          }
}

And put logic from validate method to Mage_Customer_Model_Address_Validator class into method validate

@magento-team magento-team reopened this May 17, 2012

@LeeSaferite

This comment has been minimized.

Show comment Hide comment
@LeeSaferite

LeeSaferite May 17, 2012

I 100% agree with this. Address validation should be a pluggable functionality. You could implement via an interface or an event.

I 100% agree with this. Address validation should be a pluggable functionality. You could implement via an interface or an event.

@magento-team

This comment has been minimized.

Show comment Hide comment
@magento-team

magento-team May 23, 2012

Contributor

The feature was approved by the product management and added to the roadmap. However it's not scheduled yet for the execution.
Thank you for the suggestion.

Contributor

magento-team commented May 23, 2012

The feature was approved by the product management and added to the roadmap. However it's not scheduled yet for the execution.
Thank you for the suggestion.

magento-team pushed a commit that referenced this issue Jan 16, 2015

Merge pull request #17 from magento-api/MAGETWO-31999-github-829-oAut…
…h-issue

[API] MAGETWO-31999: oAuth issue [from github]

This was referenced Mar 4, 2015

@stevieyu stevieyu referenced this issue Apr 3, 2015

Closed

locale bug #1164

This was referenced Jan 10, 2018

Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment