refactor(cl): refactored factories #5953
Open
+303
−374
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Move DocketFactory towards the top of the file so it can be referenced directly.
Moved CitationWithParentsFactory to after the factory it references.
Instead of redoing OpinionClusterFactoryWithChildren, it just inherits from it.
Removes code from DocketParentMixin that is better accomplished through SelfAttributes. Also has the benefit of making implicit behavior explicit, increasing readability, and reducing code.
DocketEntryWithParentsFactory is the exact same as DocketEntryFactory, so it makes no sense to have both.
It is used in two places, one of which can use a different parent class, and does nothing that an explicit one-liner doesn't do.
It was used once.
Renamed OpinionClusterWithChildrenAndParentsFactory, OpinionClusterFactoryWithChildren, and OpinionClusterFactoryMultipleOpinions so they end in Factory like all other factories.
Fixed all the places where RECAPDocument.document_number was being treated as an int instead of a string.
The test test_percolate_document_on_ingestion was relying on a typing bug. When creating the RECAPDocument rd_2, it was correctly setting the document_number to a string, "1". However, when it changed the value to trigger a webhook, it set the value to the integer 1. This worked because 1 != "1", even though Django converts it later to a string and the result in the database is the same.
for more information, see https://pre-commit.ci
|
Thanks. This all looks good to me, but I'll Alberto take a more careful look. Looks really nice though. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After working a lot with the factories, I found a number of things that could be streamlined. I focused on favoring explicit over implicit when it resulted in less code, removing redundancies, and consistency.
This is best reviewed by commit, as they each represent a discrete change. Most of the important changes are in
cl/search/factories.py, with the bulk of the other changes just renaming. It looks like a lot, but fortunately all the changes are in tests, so it has 100% coverage.