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

APP-6820: Adding the Enable Billing Service Action #4630

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

RoxyFarhad
Copy link
Member

@RoxyFarhad RoxyFarhad commented Dec 13, 2024

This turns on the Billing Service from the CLI

Output:

➜  rdk git:(APP-6820) go run ./cli/viam/main.go organization billing-service enable --org-id 8780edbe-b671-4604-bf5b-28d
77b3048c2 --address="100 ainslie 
 street, Brooklyn, NY, 11211"
Info: Using "https://app.viam.dev" as base URL value
Successfully enabled billing service for organization "8780edbe-b671-4604-bf5b-28d77b3048c2"

With an empty orgID:

➜  rdk git:(APP-6820) go run ./cli/viam/main.go organization billing-service enable --org-id=""  --address="100 ainslie
 street, Brooklyn, NY, 11211"
Info: Using "https://app.viam.dev" as base URL value
Error: Cannot enable billing service without an organization ID
exit status 1

With a bad address:

➜  rdk git:(APP-6820) go run ./cli/viam/main.go organization billing-service enable --org-id=""  --address="100 ainslie
 street, Brooklyn, NY, 11211"
Info: Using "https://app.viam.dev" as base URL value
Error: Cannot enable billing service without an organization ID
exit status 1

with an empty address:

➜  rdk git:(APP-6820) go run ./cli/viam/main.go organization billing-service enable --org-id 8780edbe-b671-4604-bf5b-28d77b3048c2 --address=""
Info: Using "https://app.viam.dev" as base URL value
Error: Cannot enable billing service to an empty address
exit status 1

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Dec 13, 2024
@RoxyFarhad RoxyFarhad requested a review from a team as a code owner December 20, 2024 18:26
@RoxyFarhad RoxyFarhad requested review from njooma and stuqdog December 20, 2024 18:26
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Dec 20, 2024
@@ -989,6 +989,19 @@ func TestAppClient(t *testing.T) {
test.That(t, err, test.ShouldBeNil)
})

t.Run("EnableBillingSevice", func(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

do we need code in app_client? whats this testing then?

Copy link
Member Author

Choose a reason for hiding this comment

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

// EnableBillingService enables a billing service to an address in an organization.
func (c *AppClient) EnableBillingService(ctx context.Context, orgID string, billingAddress *BillingAddress) error {
	_, err := c.client.EnableBillingService(ctx, &pb.EnableBillingServiceRequest{
		OrgId:          orgID,
		BillingAddress: billingAddressToProto(billingAddress),
	})
	return err
}

this is already in the code ^

Comment on lines +398 to +403
cCtx, ac, out, errOut := setup(asc, nil, nil, nil, nil, "token")
test.That(t, ac.organizationEnableBillingServiceAction(cCtx, "test-org",
"123 Main St, Suite 100, San Francisco, CA, 94105"), test.ShouldBeNil)
test.That(t, len(errOut.messages), test.ShouldEqual, 0)
test.That(t, len(out.messages), test.ShouldEqual, 1)
test.That(t, out.messages[0], test.ShouldContainSubstring, "Successfully enabled billing service for organization")
Copy link
Member

Choose a reason for hiding this comment

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

tests for empty org + empty address?

Copy link
Member Author

Choose a reason for hiding this comment

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

tested manually ^

Copy link
Member

@jr22 jr22 left a comment

Choose a reason for hiding this comment

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

can you post example of output and it working? otherwise few nits but lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test This pull request is marked safe to test from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants