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

Support zero amounts and priority with finance update #25

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

llewelld
Copy link

Summary

The finance update command checks whether a parameter has been passed and only updates the value if it has. With the previous implementation an existence check is made. This masks out None values, but also masks out zero values. In order to support zero values, an explicit check for None is needed.

This changes the check in this way to allow for a zero amount or zero priority to be passed as a parameter.

Issues

Fixes #24

Reviewer

This change allows zero priority and amount. But maybe other values (ticket?) should have an explicit None check as well? I wasn't certain about this.

How to run

Create a finance row (not on a production system):

rctab sub finance create --subscription-id ABC --date-from 2024-12 --date-to 2024-12 --amount 100 --finance-code DEF --ticket GHI --priority 100

Get the finance row ID:

rctab sub finance list ABC

Find the finance row just created and replace the <FIN_ID> values below with the value shown in the finance-id field.

Update the amount and priority to attempt to set them to zero:

rctab sub finance update --finance-id <FIN_ID> --subscription-id ABD --amount 0 --priority 0

Check that the amount and priority are now set to zero:

rctab sub finance get --finance_id <FIN_ID>

The finance update command checks whether a parameter has been passed
and only updates the value if it has. With the previous implementation
an existence check is made. This masks out None values, but also masks
out zero values. In order to support zero values, an explicit check for
None is needed.

This changes the check in this way to allow for a zero amount or zero
priority to be passed as a parameter.
@llewelld llewelld requested review from Iain-S and dlpbc December 16, 2024 18:08
@llewelld
Copy link
Author

For the failed pipelines, is this something I need to fix? It looks like the missing API keys are on the Turing's side.

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.

Support finance update with amount set to zero
1 participant