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

Add support for cancelling subscriptions #46

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

Conversation

sjmiller609
Copy link
Contributor

@sjmiller609 sjmiller609 commented Aug 30, 2024

Motivation

Add support for cancelling subscriptions

https://docs.withorb.com/reference/cancel-subscription

Checklist

  • If altering the functionality or API surface of this crate, this PR has a corresponding CHANGELOG.md entry.
    • The format is based on Keep a Changelog. Add your entries below the [Unreleased] section header.
  • There are no deviations from Orb's API.

@sjmiller609
Copy link
Contributor Author

@arusahni Thank you for your review

@sjmiller609
Copy link
Contributor Author

I'll open another branch to main after this PR is merged.

sjmiller609#1

@arusahni arusahni self-requested a review September 26, 2024 12:47
Copy link
Contributor

@arusahni arusahni left a comment

Choose a reason for hiding this comment

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

Thank you for submitting this PR! Since you're an external contributor the secrets required for the API test suite are unavailable to Github Actions. I ran the tests locally and can confirm they work.

In addition to the requested change, could you squash your commits? I'll merge this once everything is resolved.


/// Parameters for cancelling a subscription.
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize)]
pub struct CancelSubscriptionParams {
Copy link
Contributor

Choose a reason for hiding this comment

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

For things that map to a POST body, we prefer to use "request"

Suggested change
pub struct CancelSubscriptionParams {
pub struct CancelSubscriptionRequest {

Comment on lines +685 to +687
// The Orb API does not immediately update the status of a cancelled subscription.
// But the end date is set to now when cancelled immediately.
// assert_eq!(immediately_cancelled_subscription.status, Some(SubscriptionStatus::Ended));
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, there are some frustrating latencies contained in the API. Depending on what you've seen, we'd be open to a sleep (as we do in test_events()). If it's longer than several seconds, I think it's fine to leave this as-is.

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.

2 participants