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

feat: add validity date in curriculum create dto #135

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

piresbruna
Copy link

Resolve a issue #128

@piresbruna piresbruna self-assigned this Jun 15, 2022
@piresbruna piresbruna marked this pull request as ready for review June 19, 2022 01:48
import java.time.LocalDate;

@Getter
public class DateChronologyException extends RuntimeException {
Copy link
Member

@domingoslatorre domingoslatorre Jun 19, 2022

Choose a reason for hiding this comment

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

Talvez essa exception poderia ter um nome mais específico como StartDateAfterEndDateException

List<Curriculum> existedCurriculums = curriculumRepository.findAllByCourseId(courseId);
if(endDate != null){
if(!existedCurriculums.isEmpty()){
verifyDatesChronology(startDate,endDate);
Copy link
Member

@domingoslatorre domingoslatorre Jun 19, 2022

Choose a reason for hiding this comment

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

Esse método verifyDatesChronology(startDate,endDate); pode ser chamado logo no início do método, dessa forma caso a data inicial for maior que a final a exception será lançada já parando a execução do método. Para isso dar certo, você precisa considerar se o endDate é ou não null no método verifyDatesChronology. Vamos refatorar isso juntos na review semana que vem.

import java.time.LocalDate;

@Getter
public class TimeOverlayException extends RuntimeException {
Copy link
Member

Choose a reason for hiding this comment

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

Essa classe está bem flexível, podendo ser usada em qualquer situação de sobreposição de tempo. Isso é muito bom. Outra solução seria ter uma exceção mais específica e que indicasse com qual currículo acontece a sobreposição (id ou code), isso simplificaria a criação dela lá no service. Algo assim:

public class CurriculumTimeOverlayException extends RuntimeException {
    private final UUID curriculumOverlaidId;

    public TimeOverlayException(UUID curriculumOverlaidId) {
        super();
        this.curriculumOverlaidId = curriculumOverlaidId;
    }
}

Vamos discutir mais sobre isso no review semana que vem.

Copy link
Member

@domingoslatorre domingoslatorre left a comment

Choose a reason for hiding this comment

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

As alterações estão atendendo os critérios de aceitação da issue. Antes de aprovar, vamos refatorar o código do service juntos. É mais fácil explicar em uma call do que escrever tudo por aqui.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants