Skip to content

Conversation

@gpeng
Copy link
Contributor

@gpeng gpeng commented Jan 7, 2026

Description

  • remove manage_breast_screening/auth/tests/factories.py
  • update imports to use the clinics version
  • move UserFactory to manage_breast_screening/users where the model is defined
  • remove auth and clinic versions of UserFactory
  • update all the imports
  • fix failing tests that expected email = "" by setting that explicitly

We had two slightly different definitions of UserFactory. The removed one didn't generate an email by default so switching to the manage_breast_screening/clinics/tests/factories.py required some tests to be fixed.

Review notes

No behavioural change. Just a tidy up.

@gpeng gpeng requested a review from a team as a code owner January 7, 2026 17:43
Copy link
Contributor

@MatMoore MatMoore left a comment

Choose a reason for hiding this comment

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

The model is defined in auth so can you move the factory there or keep that one instead?

* remove manage_breast_screening/auth/tests/factories.py
* move manage_breast_screening/clinic/tests/factories.py UserFactory to
  manage_breast_screening/user as it the model is defined there
* fix failing tests that expected email = "" by setting that explicitly

We had two slightly different definitions of UserFactory both arguably
in the wrong place. Move to the correct place.
@gpeng gpeng force-pushed the remove-duplicate-user-factory-definition branch from a22f072 to 83e0d7b Compare January 8, 2026 13:38
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 8, 2026

@gpeng
Copy link
Contributor Author

gpeng commented Jan 8, 2026

Our User is defined in manage_breast_screening/users so I've moved the factory there.

@gpeng gpeng merged commit 8459e2b into main Jan 8, 2026
14 checks passed
@gpeng gpeng deleted the remove-duplicate-user-factory-definition branch January 8, 2026 14:45
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