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

137 validar que un alumno puede inscribirse a una materia #148

Merged

Conversation

fede9612
Copy link
Contributor

@fede9612 fede9612 commented Apr 6, 2019

Closes #137

Validación cuando un alumno intenta cursar una materia que tiene correlativas y todavía no las hizo(rindió o regularizó).
Tiene hecho un test para comprobar que el método de validación funciona.

image

@fede9612 fede9612 requested a review from JuanCS95 April 6, 2019 21:38
@fede9612 fede9612 requested a review from faloi as a code owner April 6, 2019 21:38
@ghost ghost assigned fede9612 Apr 6, 2019
@ghost ghost added the review label Apr 6, 2019
this.cursadas.add(cursada);
}

public void addCursadaComprobandoCorrelativas(Cursada cursada, Materia materia) {
Copy link
Contributor

Choose a reason for hiding this comment

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

¿Por qué un método aparte? La idea es que nunca se pueda agregar una cursada inválida, y el código del modelo no debe permitirlo.

El parámetro materia no deberías necesitarlo, porque ya está dentro de la cursada.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tenes razón Fede, ya lo corregí. Buena observación.

if(!this.puedeMatricularseA(materia)){
throw new ModelException("No puede matricularse en la materia "
+ materia.getNombre() +
" porque debe sus correlativas.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Estaría bueno mostrar cuáles debe. Lo dejamos para otra issue: #151.

public class CorrelativaTest {

@Test
public void agregarCorrelativaandaTest(){
Copy link
Contributor

Choose a reason for hiding this comment

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

Este test solo prueba que anda el método addCorrelativa de las materias, lo cual no nos dice nada sobre lo que hiciste.

Yo haría estos tests:

  • un alumno que tiene las correlativas, se anota a la materia y no pasa nada.
  • un alumno que no tiene las correlativas, intenta anotarse a la materia y tira 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.

Listo Fede

@faloi faloi merged commit a51c45a into master Apr 15, 2019
@faloi faloi deleted the 137-validar-que-un-alumno-puede-inscribirse-a-una-materia branch April 15, 2019 13:55
@ghost ghost added production and removed review labels Apr 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validar que un alumno puede inscribirse a una materia
3 participants