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

Some restrictions are added to the model Book #11

Merged
merged 3 commits into from
Jul 26, 2016
Merged

Some restrictions are added to the model Book #11

merged 3 commits into from
Jul 26, 2016

Conversation

NestorEduardo
Copy link
Contributor

Some restrictions added so that the fields are mandatory. The ISBN can not be more than thirteen numbers.

@amhed
Copy link
Contributor

amhed commented Jul 26, 2016

Perfecto. Necesitamos tests que validen estas reglas

@amhed
Copy link
Contributor

amhed commented Jul 26, 2016

…Book. Tambien valida cuando un ISBN tiene mas de trece digitos
@NestorEduardo
Copy link
Contributor Author

Se que los tests sobre un campo nulo se pueden reducir usando lo que en NUnit se llama [TestCase], soy nuevo en Xunit y hasta ahora no se como se haría aquí.

@amhed
Copy link
Contributor

amhed commented Jul 26, 2016

Carajo, se me va salir una lagrima (de felicidad) 🙃

@amhed
Copy link
Contributor

amhed commented Jul 26, 2016

Mañana le doy Merge q toy echao!

@@ -25,7 +26,86 @@ public void BookHasNecessaryProperties()
ISBN = "020161622X",
CoverUrl = "https://images-na.ssl-images-amazon.com/images/I/41BKx1AxQWL._SX258_BO1,204,203,200_.jpg"
};
var ValidationResult = new List<ValidationResult>();

Choose a reason for hiding this comment

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

What about create a method to encapsulate this validation, so you don't have to repeat these lines only the method.

@esanmiguelc
Copy link
Collaborator

This is great. I have an idea though, what if we had validations happen in the outermost layer of the application. In this case, I'm thinking in a "viewmodel" or something like that. The benefits that we would achieve from that is we could add internationalization easily from the UI/Web component, and keep invalid input from reaching the internals of the application. what do you think? @amhed @willjobs @NestorEduardo

public string CoverUrl { get; set; }

public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this being used?

@amhed
Copy link
Contributor

amhed commented Jul 26, 2016

One thing at a time @esanmiguelc. This is great =D

@esanmiguelc
Copy link
Collaborator

Could we keep the commits to a single language? either spanish or english for consistency

@Willjobs94
Copy link

@esanmiguelc totally agree and I prefer English, Is the most used in this area (Software development).

… I also added a message indicating what the test result must return
@amhed
Copy link
Contributor

amhed commented Jul 26, 2016

@NestorEduardo update the names on the tests and I'll merge this asap

@NestorEduardo
Copy link
Contributor Author

I'm sorry for the delay

@amhed amhed merged commit f1addb6 into developersdo:master Jul 26, 2016
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.

None yet

4 participants