Implemented processor for New-PI credit #112
Conversation
QuanMPhm
force-pushed
the
101/proc_new_pi_credit
branch
2 times, most recently
from
0299851 to
59c0012
Compare
|
I fixed a bug with @knikolla @naved001 I could write the test cases that would catch this mistake in this PR, but since the PR for #102 would catch this, I believe I will just write these tests in that PR? |
Could you say more about the impact of this bug? Would we need to rerun the invoice with the bug fix?
Which commit has the bug fix? the commit message in the 3rd commit covers a bunch of things and I didn't find a note about the bug fix. Also, if the fix for a bug is in this PR I think the test should also be a part of this. |
QuanMPhm
force-pushed
the
101/proc_new_pi_credit
branch
from
59c0012 to
2073310
Compare
Code from `BillableInvoice` and `DiscountInvoice` has been copied over without change
The `DiscountProcessor` class has been created, to be subclassed by processors which applies discounts, such as the New-PI Credit. This processor class introduces an important class constant, `IS_DISCOUNT_BY_NERC`, which detemines whether the final balance, as opposed to the PI balance (more info below), reflects the discount being applied. The New-PI credit is now implemented by `NewPICreditProcessor` During discussions about billing, it was made noted that some discounts are not provided by the MGHPCC, but instead from other sources, such as the BU subsidy, which is provided to BU PIs by BU. This provided motivation for a `PI Balance` field, which would reflect how much money the PI should be billed, as opposed to the `Balance` field, which currently reflects how much money the MGHPCC should receive. These two fields would not equal each other if the PI received discounts not provided by the MGHPCC. Implementation of `NewPICreditProcessor` and the new billing feature required a range of changes: - `apply_discount_on_project()` in `DiscountProcessor` has been slightly modified, where the PI balance and MGHPCC balance is now calculated seperately. - As `BillableInvoice` no longer performs any processing itself, the dataframe from `NewPICreditProcessor` is now passed to all invoice objects. - The test cases for the New-PI credit have been refactored. Test cases for the new billing feature is not written yet. I plan to write them when the processor for the BU Subsidy is implemented - With the new processor, certain Processor and Invoice classes depend on fields created by other Processors, such as the case with `NewPICreditProcessor` and `ValidateBillablePIsProcessor`. As such, docstrings have been added to indicate dependancies
QuanMPhm
force-pushed
the
101/proc_new_pi_credit
branch
from
2073310 to
1686888
Compare
|
@knikolla do you think you'd have time to review this? |
|
With 3 approvals I feel fine with this merging without my review. |
Closes #101. Depends on #108. I've implemented processor for applying the New-PI Credit, and included a new field to distinguish between the PI balance and the NERC balance. More details are in my commit message.
There are two commits to this PR, starting with
Initialized processor for applying the New-PI Credit and discounts. The first one only copies over code, while the second commit actually makes functional changes.