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

AB#4215 Add the repository for confirmation code with a delete function #1849

Closed
wants to merge 1 commit into from

Conversation

ESDC-Olivier-Roy
Copy link
Contributor

@ESDC-Olivier-Roy ESDC-Olivier-Roy commented Jun 26, 2024

Description

Add the repository for confirmation code with a delete function. This will delete all confirmation codes that have their expiry date less than the inserted time.

Related Azure Boards Work Items

AB#4215

Checklist

  • I have tested the changes locally
  • I have updated the documentation if necessary
  • I have added/updated tests that prove my fix is effective or that my feature works
  • I have checked that my code follows the project's coding style by running npm run format:check
  • I have checked that my code contains no linting errors by running npm run lint
  • I have checked that my code contains no type errors by running npm run typecheck
  • I have checked that all unit tests pass by running npm run test:unit -- run
  • I have checked that all e2e tests pass by running npm run test:e2e

Comment on lines 35 to 37
assertThat(confirmationCodeRepository.findById("a6ea4925-f813-493e-80ec-a5b90ca28aaa")).isNotEmpty();
confirmationCodeRepository.deleteByExpiryDateLessThan(Instant.now());
assertThat(confirmationCodeRepository.findById("a6ea4925-f813-493e-80ec-a5b90ca28aaa")).isEmpty();
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be a good idea to verify that confirmation codes with an expiry date in the future and an expiry date with the current instant are not deleted.

@ActiveProfiles("test")
@Import({ DataSourceConfig.class })
@AutoConfigureTestDatabase(replace = Replace.NONE)
public class ConfirmationCodeRepositoryIT {
Copy link
Contributor

Choose a reason for hiding this comment

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

Something that I don't like about this test is that it relies on data that is in the data-init.sql file.

I know that other tests do the same thing.. I feel that they're also not good tests.

What you should do, instead.. is do a findById(..) and verify that the data doesn't exist, then insert using save(..) and do another findById(..)

Copy link
Contributor

Choose a reason for hiding this comment

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

...same idea with the delete method

@ESDC-Olivier-Roy ESDC-Olivier-Roy force-pushed the add-confirmation-code-repository branch from 0af8233 to e5fa237 Compare June 28, 2024 14:04
@ESDC-Olivier-Roy ESDC-Olivier-Roy force-pushed the add-confirmation-code-repository branch from e5fa237 to 96aaa05 Compare June 28, 2024 15:23
@sebastien-comeau
Copy link
Contributor

Close it for reference. @nicholas-ly will implement it later.

auto-merge was automatically disabled July 10, 2024 13:59

Pull request was closed

@sebastien-comeau sebastien-comeau deleted the add-confirmation-code-repository branch July 10, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants