Delegate Fulfillment instead of using FulfillmentInfo (PP-1641) #2035
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2035 +/- ##
==========================================
+ Coverage 90.72% 90.75% +0.03%
==========================================
Files 342 342
Lines 40572 40516 -56
Branches 8796 8771 -25
==========================================
- Hits 36808 36772 -36
+ Misses 2503 2486 -17
+ Partials 1261 1258 -3 ☔ View full report in Codecov by Sentry. |
jonathangreen
force-pushed
the
chore/refactor-fulfillment
branch
from
1fdb345 to
d166625
Compare
| active attempt to fulfill the loan. | ||
| :param locked_to: A DeliveryMechanismInfo object representing the | ||
| delivery mechanism to which this loan is 'locked'. | ||
| """ | ||
| super().__init__(collection, data_source_name, identifier_type, identifier) | ||
| self.start_date = start_date | ||
| self.end_date = end_date | ||
| self.fulfillment_info = fulfillment_info | ||
| self.fulfillment = fulfillment |
There was a problem hiding this comment.
Self fulfillment is sooooo important! 🙃
src/palace/manager/api/axis.py
Outdated
| debug_message=error_details, | ||
| ) | ||
| def __init__( | ||
| self, content_link: str, content_type: str | None, verify: bool |
There was a problem hiding this comment.
Could this content_type default to DeliveryMechanism.ADOBE_DRM? Would we be calling this with any others?
There was a problem hiding this comment.
Makes sense. I'll change that.
@tdilauro I think I got all the ones that need to be changed. Its a bit annoying, the axis API has an API call named circulation/src/palace/manager/api/axis.py Line 185 in d166625 |
jonathangreen
changed the title
Description
Refactor
FulfillmentInfoclass which just contained information about a fufillment, but had multiple methods to override how fulfillment was done, to be a new class calledFulfillmentwhich the loans controller delegates fulfillment to.This lets each API control how fulfillment is done, and reduces the amount of if / then we need to have in the loan controller. We define several types of Fulfillment, corresponding to the logic that used to exist in the loans controller, and update the API classes to use them.
The goal of this PR is the have fulfillment for each API act the same way before and after, just refactoring to use the new structure.
Motivation and Context
This refactor builds on #2034 and is part of the work needed for #2033. We are already effectively delegating control of the fulfillment, but not being explicit about it, so I think this refactor makes it easier to reason about how fulfillment will work.
How Has This Been Tested?
Checklist