Irio / mymoip

MoIP transactions in a gem to call your own.
MIT License
31 stars 21 forks source link

Credit card validations #39

Closed hugomaiavieira closed 11 years ago

hugomaiavieira commented 11 years ago

@Irio some credit card attributes (:owner_name, :owner_cpf and :card_number) doesn't have any validation. There are some reason for it? Can I add a validation for the presence of these attributes?

hugomaiavieira commented 11 years ago

On Moip javascript they validate the presence of owner_name and do this validation for owner_cpf:

cpfEhValido = function(cpf) {

  cpf = String(cpf).replace(/\D/g, "");

  if (cpf.length != 11)
    return false;

  return true;
}

And this to the card_number:

  validarNumeroDoCartao = function(cartao, instituicao) {

    var numero = cartao.Numero;

    if (naoInformou(numero)) {
      adicionarErro(905, "Informe o número do cartão");

    } else {
      var qtdCaracteres = String(numero).length;

      if (instituicao == "Visa" && qtdCaracteres != 16) {
        adicionarErro(905, "Número de cartão inválido");
      }
      else if (instituicao == "Mastercard" && qtdCaracteres != 16) {
        adicionarErro(905, "Número de cartão inválido");
      }
      else if (instituicao == "AmericanExpress" && (qtdCaracteres < 15 || qtdCaracteres > 16)) {
        adicionarErro(905, "Número de cartão inválido");
      }
      else if (instituicao == "Diners" && qtdCaracteres != 14) {
        adicionarErro(905, "Número de cartão inválido");
      }
      else if (instituicao == "Hipercard" && (qtdCaracteres < 13 || qtdCaracteres > 19 || qtdCaracteres == 17 ||qtdCaracteres == 18)) {
        adicionarErro(905, "Número de cartão inválido");
      }
    }
  };

Can I add this validations (with a better code)?

Irio commented 11 years ago

That's because the API itself don't requires those attributes to return a successful response.

This bug is already reported and no modification took place. Since MyMoip is just an interface with Moip's API, don't see reason to add an extra validation before the server.

hugomaiavieira commented 11 years ago

@Irio the problem is that this will generate an invalid moip transaction. It return ok at first, but moip will not complete the transaction. We don't want the user to not fill the creditcard number, for instance, because it will generate a invalid transaction. If we don't add the validations on the gem, I will have to open the CreditCard class to inject the validations =/

Irio commented 11 years ago

A good way to make explicit this bug from Moip while allowing to fix it without monkey patching or "fixing" the external API is add a flag in MyMoip::CreditCard.

# Initialization
MyMoip::CreditCard.new(logo: 'visa', perform_extra_validation: true)

# lib/mymoip/credit_card.rb
module MyMoip
  class CreditCard
    attr_accessor :perform_extra_validation

    validates_presence_of :card_number, :expiration_date, :owner_name,
                          :owner_phone, :owner_rg,
                          if: ->(resource) { resource.perform_extra_validation }
  end
end

What do you think about this?

hugomaiavieira commented 11 years ago

@Irio :+1: I liked your solution and already did a pull request.