Refactored invoices to seperate invoice processing from exporting #97
Conversation
This file contains 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
A `Processor` class has been added, subclassing from the `Invoice` class. This is the first step to refactor invoicing in order to seperate the processing and filtering/exporting functionalities of our current Invoice subclasses. Subclasses of `Processor` should only process invoices and manipulate its internal data, while subclasses of `Invoice` should only perform fitering and exporting, never changing any data itself. In addition to implementing `Processor`, two of its subclasses, `ValidatePIAliasProcessor` and `AddInstitutionProcessor` has been added to perform some preliminary processing steps.
Note that, for now, only the Lenovo invoice will take the processed data from the `LenovoProcessor`. All other invoices will take the data from `AddInstituteProcessor`. This is due to the processors adding new columns. This odd code design will be removed once invoices gain the feature to filter out their exported columns.
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 this discount. During discussions about billing, it was made noted that some discounts are not provided by the MGHPCC, but instead rom other sources, such as the BU subsidy. This provided motivation for the `PI Balance` field, which reflects how much money the PI owes, as opposed to the `Balance` field, which reflects how much money the MGHPCC should receive. `apply_discount_on_project` has been slightly modified, where the PI balance and MGHPCC balance is now calculated seperately. For now, the `PI Balance` field will appear in all invoices after the Billable invoice.
Since all current discounts have been implemented as processors, the `DiscountInvoice` class is now removed. No invoice class will now class `_prepare()` or `_process()`. `BUSubsidyProcessor`, which handles processing for the BU subsidy, requires introduction of a new field, `BU Balance`, because the subsidy is not provided by NERC. Because of this, `BU Balance` indicates the money which BU (not the PI they are subsidizing) owes to the MGHPCC.
… and column names Two class attributes, `export_columns_list` and `exported_columns_map`, has been added to `Invoice`, along with a class function `_filter_columns()`. Subclasses of `Invoice` must now define `export_columns_list`, containing the ordered list of columns that must be exported in their respective invoices. Subclasses can optional define `exported_columns_map`, containing mappings between "internal" column names and what their name should be when exported. The field name `RATE_FIELD` has been added to `invoice.py`. It was previously forgotten.
QuanMPhm
force-pushed
the
95/refactor_balances
branch
from
c56d313 to
8ba638c
Compare
naved001
reviewed
Sep 24, 2024
There was a problem hiding this comment.
@QuanMPhm I find it hard to leave a quality review. This PR refactors all the invoices and also adds some new functionality (ignoring discount by mghpcc), new columns etc. It would be ideal if this could be split into smaller chunks.
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.
Closes #95. The
Processorclass has been implemented, and all processing steps that I could think of has been implemented as a subclass of theProcessorclass. I have also tried my best to implement every feature listed in #95.I will do cleanup of some comments I've left in after everything is settled.
I hope the commit messages provide enough explanation.