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

Se agrega la validación de NIF para el validador de Portugal #1

Merged
merged 6 commits into from
Feb 27, 2021

Conversation

ivanhoe
Copy link
Contributor

@ivanhoe ivanhoe commented Feb 26, 2021

¿Qué hice?

  • Genere el módulo principal que va a recibir los identificadores y de acuerdo al país se va a invocar al validador correspondiente
  • Genere la validación de NIF(Número de identificación personal) para Portugal
  • Issue relacionado 750

lib/bali.ex Outdated
Comment on lines 8 to 15
def validate(_) do
{:error, "No es posible realizar la validación del identificador"}
end

## Examples
@spec validate(any, any) :: {:error, String.t()}
def validate(_, _) do
{:error, "No es posible realizar la validación del identificador"}
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Esto no es necesario si se llama a la validate con una aridad erronea esto debería ocasionar un error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okas Erick eliminadas

lib/bali.ex Show resolved Hide resolved
"""
@spec valid(any, any) :: {:error, String.t()}
def valid(_, _) do
{:error, "No es posible validar el documento de Portugal"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aquí el mensaje no queda muy claro, este caso aplicaría cuando el tipo de documento no sea válido por lo que se podría poner algo como "Tipo de documento inválido"

Copy link
Contributor Author

@ivanhoe ivanhoe Feb 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vale, hacemos el cambio del mensaje para hacerlo más claro

lib/bali.ex Outdated
Comment on lines 20 to 22
def validate(_, document_type, _value) do
{:error, "País no soportado para validar documento: #{document_type}"}
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creo que lo único que se está validando en este nivel es que el país tenga un validador creado (un módulo), pero por lo que veo en la función de Portugal (La unica que agregaste) es que si se puede validar tal o cual documento se haría el check al nivel del validador, ex. ValidadorPt.

Creo que con que dejemos el

{:error, "País #{country} no soportado"}

podemos manejar los posibles documentos en el Validador tal cual.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vale de acuerdo realizo el cambio

@@ -0,0 +1,27 @@
defmodule ValidatorPt do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deberíamos mantener los módulos en inglés no?

ValidatorPt

O en una carpeta separada los validadores

Validators.Pt

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Si, en inglés sería más consistente

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok los agrupamos en carpetas mejor

lib/validator_pt.ex Outdated Show resolved Hide resolved
- Se agregan los validadores en un carpeta
- Se ajustan las pruebas
@erickgnavar erickgnavar merged commit d8ebab6 into master Feb 27, 2021
@erickgnavar erickgnavar deleted the ivan/create_portugal_validator branch February 27, 2021 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants