[usage] Introduce GetBalance API #13214
Conversation
|
started the job as gitpod-build-se-get-balance.1 because the annotations in the pull request description changed |
svenefftinge
force-pushed
the
se/get-balance
branch
from
9d02ad8 to
f4030f2
Compare
| @@ -322,3 +322,37 @@ func TestListBalance(t *testing.T) { | |||
| CreditCents: -50, | |||
| }) | |||
| } | |||
|
|
|||
| func TestGetBalance(t *testing.T) { | |||
There was a problem hiding this comment.
Please add a test for when there are no usage records for an attributionID and it should return a 0
| rows, err := conn.WithContext(ctx). | ||
| Model(&Usage{}). | ||
| Select("sum(creditCents) as balance"). | ||
| Where("attributionId = ?", string(attributionId)). | ||
| Group("attributionId"). | ||
| Rows() | ||
| if err != nil { | ||
| return 0, fmt.Errorf("failed to get rows for list balance query: %w", err) | ||
| } | ||
| defer rows.Close() | ||
|
|
||
| if !rows.Next() { | ||
| return 0, nil | ||
| } | ||
|
|
||
| var balance CreditCents | ||
| err = conn.ScanRows(rows, &balance) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("failed to scan row: %w", err) | ||
| } | ||
| return balance, nil |
There was a problem hiding this comment.
It should be possible to do it this way, if you wanted
var balance Balance
tx := conn.WithContext(ctx).
Table((&Usage{}).TableName()).
Select("attributionId, sum(creditCents) as creditCents").
Where("attributionId = ?", string(attributionId)).
Group("attributionId").
First(&balance)
if tx.Error != nil {
if tx.Error == gorm.ErrRecordNotFound {
return 0, nil
}
return 0, fmt.Errorf("failed to get rows for list balance query: %w", tx.Error)
}
return balance.CreditCents, nil
There was a problem hiding this comment.
Why would you prefer that? btw. I realized I unnecessarily added a group by clause. Will remove that and add the suggested test in a follow-up PR.
There was a problem hiding this comment.
Just because it removes the need for the manual iteration over rows, which can hurt readability for those not familiar with gorm. It also handles the not found case a bit more explicitly which helps readability. But I don't mind, just wanted to offer an alternative in case you wanted to go with it.
|
Ugh, why did this get merged? A race between the approve and hold? |
roboquat
added
deployed: webapp
Description
Introduces a dedicated getBalance API that we can use to check against the spending limit in the billing-service. We currently use balance from ListUsage, which i a follow up change will be changed to only sum up the usage for the given range.
Related Issue(s)
Related #13155
How to test
Release Notes
Documentation
Werft options:
If enabled this will build
install/previewValid options are
all,workspace,webapp,ide