Implemented processor for prepayments #117

QuanMPhm · 2024-12-04T00:37:15Z

Closes #115 . This PR consists of the two last commits, "Implemented processor for prepayments" and "Moved institution...". More details about the implementation is in the commit message.

@joachimweyl With this implementation, backprocessing (processing past invoices) will no longer be supported.

naved001

The unit tests file only gets larger with this. Could we put the unit tests for prepayment in their own file? I know we have #116 but that's for tests that are already in the codebase, for the new stuff we should just put it in the right place to begin with.

naved001 · 2024-12-09T17:26:41Z

process_report/process_report.py

+    if args.prepay_debits:
+        prepay_debits_filepath = args.prepay_debits
+    else:
+        prepay_debits_filepath = fetch_s3_prepay_debits()


I should have spoken earlier about this, but we have too many of these fetch_something_from_s3() type methods that take no argument. We should factor this out and then re-use the methods.

def fetch_from_s3(local_name, s3_path): # do the thing

this doesn't have to be part of this PR.

naved001 · 2024-12-09T17:28:12Z

process_report/util.py

+    return institute_map
+
+
+def get_institution_from_pi(institute_map, pi_uname):


I have seen this method somewhere before

I've added a commit to move the institution functions to util.py

QuanMPhm · 2024-12-12T18:49:11Z

process_report/processors/prepayment_processor.py

+
+    def _export_s3_prepay_debits(self):
+        invoice_bucket = util.get_invoice_bucket()
+        invoice_bucket.upload_file(self.prepay_debits_filepath, "TODO.csv")  # TODO


@naved001 @knikolla @joachimweyl I'll amend this after we agree on the S3 filename for the debits file

@QuanMPhm Prepay/prepay_debits.csv seems fine.

Back this up in the same way that we backup the old pi file (or upload it to a different location with a timestamp)

I picked "Prepay/prepay_debits.csv" as the s3 location, and f"Prepay/Archive/prepay_debits {util.get_iso8601_time()}.csv" as the backup location

I won't be around for a while to reivew so don't want to block it.

knikolla

Looks good to me. 2 TODOs are left in the code which should be removed.

knikolla · 2025-01-10T15:03:28Z

process_report/processors/prepayment_processor.py

+
+    def _export_s3_prepay_debits(self):
+        invoice_bucket = util.get_invoice_bucket()
+        invoice_bucket.upload_file(self.prepay_debits_filepath, "TODO.csv")  # TODO


Back this up in the same way that we backup the old pi file (or upload it to a different location with a timestamp)

knikolla · 2025-01-10T15:22:43Z

process_report/invoices/bu_internal_invoice.py

@@ -15,7 +15,11 @@ class BUInternalInvoice(invoice.Invoice):
        invoice.INVOICE_DATE_FIELD,
        invoice.PI_FIELD,
        "Project",


Not related to this PR, but you should switch this field (and all other occurences of the magic string "Project") to instead using invoice.PROJECT_NAME_FIELD in a future patch.

knikolla · 2025-01-10T15:35:13Z

process_report/tests/unit_tests.py

+            invoice_month,
+        )
+
+        # Invoice month is after some credits and debits are given, TODO: Waiting for Kim's response


Did you get the response you needed? Please make sure there are no TODOs left over in the PR.

If not please link to what the question was.

I believe my question was whether we would allow "backprocessing". You responded no. This was relevant to how I design the test case, and if we're not doing backprocessing, that will make things a lot simpler :)

QuanMPhm · 2025-01-14T19:27:28Z

I have implemented all feedback so far and removed the TODOs

Prepayments have been implemented by a new processor, `PrepaymentProcessor`. The full implementation for prepayments involved the following changes: - Several new columns names have been added to `invoices/invoice.py`, including column names for the prepay data files and for the exported invoices - Many invoices will now export with 4 new columns: `Prepaid Group Name`, `Prepaid Group Institution`, `Prepaid Group Balance`, `Prepaid Group Used` - 4 command line arguments have been added to `process_report.py`. 3 of them allows the user to pass in a local version of the prepay credits, contacts, and projects file. The last one (`—prepay-debits`) allows passing a local version of the prepay debits files, and defaults to fetching from s3 if not provided - A set of test cases have been added for `PrepaymentProcessor` Since the implementation of this feature required a lot of logic decisions (i.e What happens if a prepaid project is active, but has no funds?), below is (hopefully) an exhaustive list of code logic decisions that were made. These can also be inferred through the test cases. - Prepay projects listed in `prepaid_projects.csv` are identified by their project name, not project - allocation name - Attempting to process past invoices (“backprocessing”) may result in incorrect output due to the nature of the prepay debit ledger - While backprocessing is not supported for past months, processing the same invoice month twice will still return correct output. In this case, the month’s debit entry may be will be overwritten - Prepay balances can be used in the same month they are added. - The time range in which prepay projects are considered “active” includes their start and end date - After processing of any given invoice month, debit entries for that month will be added. I emphasize this for clarification. A debit entry such as: `2024-11,G1,1062.48` Should be interpreted as: In the period from 2024-11-01 to 2024-11-30, prepay group G1 spent $1062.48 As opposed to: In the period from 2024-10-01 to 2024-10-31, … - If prepay projects are “active” but their prepay group has $0 balance, their prepay info (group name, contact email) is still included, but the prepay balance will be displayed as $0 and the prepay used as an empty field

QuanMPhm · 2025-01-14T21:06:21Z

The PR has been rebased

…r` to `util` Since these functions are now used in more than one location (`add_insti_proc` and `prepay_proc`), it makes sense for them to be relocated to `util` The test case checking institution-processing has changed slightly

knikolla · 2025-01-17T14:34:57Z

process_report/processors/prepayment_processor.py

+    PREPAY_DEBITS_S3_FILEPATH = "Prepay/prepay_debits.csv"
+
+    @property
+    def PREPAY_DEBITS_S3_BACKUP_FILEPATH(self):


nitpick: Generally, all capital letters is reserved for constants. This isn't a constant.

QuanMPhm requested review from larsks, knikolla, naved001 and hakasapl December 4, 2024 00:37

naved001 previously requested changes Dec 6, 2024

View reviewed changes

naved001 reviewed Dec 9, 2024

View reviewed changes

QuanMPhm force-pushed the 115/prepayment_proc branch from 8a8d9c7 to e8ad7d8 Compare December 11, 2024 21:46

QuanMPhm mentioned this pull request Dec 12, 2024

Refactor all the fetch_s3_... functions into one #123

Open

QuanMPhm commented Dec 12, 2024

View reviewed changes

knikolla reviewed Jan 10, 2025

View reviewed changes

QuanMPhm force-pushed the 115/prepayment_proc branch from e8ad7d8 to 5ba7121 Compare January 14, 2025 19:26

QuanMPhm force-pushed the 115/prepayment_proc branch from 5ba7121 to 5ab6185 Compare January 14, 2025 21:06

QuanMPhm force-pushed the 115/prepayment_proc branch from 5ab6185 to e95349a Compare January 14, 2025 21:14

QuanMPhm requested a review from knikolla January 14, 2025 21:15

knikolla approved these changes Jan 17, 2025

View reviewed changes

knikolla reviewed Jan 17, 2025

View reviewed changes

knikolla merged commit 28318f7 into CCI-MOC:main Jan 17, 2025
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented processor for prepayments #117

Implemented processor for prepayments #117

QuanMPhm commented Dec 4, 2024 •

edited

Loading

naved001 left a comment

naved001 Dec 9, 2024

naved001 Dec 9, 2024

QuanMPhm Dec 12, 2024

QuanMPhm Dec 12, 2024

knikolla Jan 10, 2025

knikolla Jan 10, 2025

QuanMPhm Jan 14, 2025

knikolla left a comment

knikolla Jan 10, 2025

knikolla Jan 10, 2025

knikolla Jan 10, 2025

joachimweyl Jan 10, 2025

QuanMPhm Jan 14, 2025

QuanMPhm commented Jan 14, 2025

QuanMPhm commented Jan 14, 2025

knikolla Jan 17, 2025

		return institute_map


		def get_institution_from_pi(institute_map, pi_uname):

Implemented processor for prepayments #117

Implemented processor for prepayments #117

Conversation

QuanMPhm commented Dec 4, 2024 • edited Loading

naved001 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knikolla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QuanMPhm commented Jan 14, 2025

QuanMPhm commented Jan 14, 2025

Choose a reason for hiding this comment

QuanMPhm commented Dec 4, 2024 •

edited

Loading