Refactor LoanInfo and HoldInfo to be dataclasses (PP-1728) #2113
Conversation
jonathangreen
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2113 +/- ##
==========================================
+ Coverage 90.69% 90.71% +0.02%
==========================================
Files 351 351
Lines 40878 40878
Branches 8858 8853 -5
==========================================
+ Hits 37073 37084 +11
+ Misses 2493 2487 -6
+ Partials 1312 1307 -5 ☔ View full report in Codecov by Sentry. |
| @@ -1423,14 +1410,6 @@ def _raise_exception_on_error( | |||
|
|
|||
|
|
|||
| class XMLResponseParser(ResponseParser, Axis360Parser[T], ABC): | |||
There was a problem hiding this comment.
The changes here make the XMLResponseParser subclasses return the parsed data directly, rather then a LoanInfo or HoldInfo object. Previously they returned a partial object, and the caller had to fill in the missing details, now the caller creates the whole LoanInfo or HoldInfo object.
| """Either turn the given document into a LoanInfo | ||
| object, or raise an appropriate exception. | ||
| """ | ||
| self.raise_exception_on_error(e, namespaces) | ||
|
|
||
| # If we get to this point it's because the checkout succeeded. | ||
| expiration_date = self._xpath1(e, "//axis:expirationDate", namespaces) | ||
| fulfillment_url = self._xpath1(e, "//axis:url", namespaces) | ||
| if fulfillment_url is not None: | ||
| fulfillment_url = fulfillment_url.text |
There was a problem hiding this comment.
This was parsed, but never used.
|
This one has some missing coverage, but all the lines that are not covered, we not previously covered, so I think it should be fine to merge as is. |
jonathangreen
force-pushed
the
chore/refactor-loan-hold-info
branch
from
d490e4b
to
4e69c0c
Compare
Description
Refactor
LoanInfoandHoldInfoto be simple dataclasses, and move the logic for creating and updating loans and holds from aLoanInfoorHoldInfointo a method calledcreate_or_update.In the process, I removed the
Hold.external_identifiercolumn, which appears to be unused.Motivation and Context
Needed to modify how
LoanInfoandHoldInfoare used as part of PP-1728, this refactor makes that work easier, but I thought it would be easier to understand as a separate PR.How Has This Been Tested?
Checklist