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

Refactor processing for the New-PI credit #101

Open
QuanMPhm opened this issue Sep 25, 2024 · 1 comment · May be fixed by #112
Open

Refactor processing for the New-PI credit #101

QuanMPhm opened this issue Sep 25, 2024 · 1 comment · May be fixed by #112
Assignees

Comments

@QuanMPhm
Copy link
Contributor

QuanMPhm commented Sep 25, 2024

Dependant on #108, the implementation of the New-PI credit should be implemented as a Processor subclass.

@QuanMPhm
Copy link
Contributor Author

QuanMPhm commented Nov 7, 2024

@knikolla @naved001 While writing the PR for this issue, it occurred to me that the New-PI Processor will be dependant on the ValidateBillablePIsProcessor proposed in #108. This is because we would want to know which projects or PIs are non-billable before applying the New-PI discount.

As such, the New-PI Processor has a dependancy (ValidateBillablePIsProcessor), it also follows that BillableInvoice has a dependancy as well (ValidateBillablePIsProcessor) because it reads a field (Is Billable) provided by this processor.

For the purpose of this issue, this only means that I believe this issue is dependant on #108. For the purpose of code structure, I just wanted to highlight the fact that our Invoice and Processor classes may have dependencies of one (or more) Processor, in case this is something you guys might have concerns with.

For now, I will note in the docstring of Invoice and Processor subclasses if they have any specific dependencies.

@QuanMPhm QuanMPhm linked a pull request Nov 7, 2024 that will close this issue
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 a pull request may close this issue.

1 participant