Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented processing and exporting for BU Internal invoice #26

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

QuanMPhm
Copy link
Contributor

@QuanMPhm QuanMPhm commented Apr 23, 2024

Closes #25 fully, I have implemented the function to export the BU Internal invoice, following @joachimweyl's specifications in the linked issue.

The BU subsidy will be applied to every BU PI after the New-PI credit is applied to eligible PIs in the combined invoice. With how this is currently implemented, the combined invoice may NOT show BU PIs' final balances, as the BU-only invoice is made and exported after the combined invoice is exported.

The current subsidy is set at $100, but I have made the amount configurable via a cli arguement.

process_report/process_report.py Outdated Show resolved Hide resolved
process_report/process_report.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@naved001 naved001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QuanMPhm could you update the commit message with a brief description of how this is supposed to work? For example, I don't remember if subsidies apply before credits or the other way around etc.

process_report/process_report.py Outdated Show resolved Hide resolved
process_report/process_report.py Outdated Show resolved Hide resolved
@QuanMPhm QuanMPhm force-pushed the 25/BU_internal_subsidy branch 2 times, most recently from e2716be to 74bc0f8 Compare April 25, 2024 14:42
@QuanMPhm
Copy link
Contributor Author

@knikolla I'll rebase once I know which PR is approved?

process_report/process_report.py Show resolved Hide resolved
process_report/process_report.py Outdated Show resolved Hide resolved
Copy link
Contributor

@knikolla knikolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me. @naved001 can you do a final pass?

Copy link
Collaborator

@naved001 naved001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one suggestion for improving the tests, other than that it looks good to me.

process_report/tests/unit_tests.py Show resolved Hide resolved
@QuanMPhm QuanMPhm merged commit 8ac8cca into CCI-MOC:main Apr 30, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a new HU & BU only Invoice & Create a BU Internal Invoice CSV
4 participants