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 free plan creation #134

Open
thisiskseniab opened this issue Nov 1, 2019 · 2 comments
Open

Support free plan creation #134

thisiskseniab opened this issue Nov 1, 2019 · 2 comments

Comments

@thisiskseniab
Copy link
Contributor

thisiskseniab commented Nov 1, 2019

Currently, this API does not support free plan creation due to UnitAmount having omitempty set on every currency and UnitAmount.MarshalXML method explicitly checking values to be greater than 0.

In order to create a free plan one (or all) currency values have to be set to 0. Unfortunately, solving this problem isn't as simple as just unsetting omitempty on every currency in UnitAmount struct and not checking for values being greater than 0 because this will mean that by default all plans will cost 0 in currencies where the value isn't set which might have unintended consequences. Besides, removing omitempty on every currency will also cause issues with sites where multi-currency support is not enabled. UnitAmount is also used to define adjustments, coupons, and subscriptions.

We're not entirely sure what is the best approach to solving this is, but hopefully, maintainers of this project have some suggestions! 🙏

Reference code.

// UnitAmount can read or write amounts in various currencies.
type UnitAmount struct {
	USD int `xml:"USD,omitempty"`
	EUR int `xml:"EUR,omitempty"`
	GBP int `xml:"GBP,omitempty"`
	CAD int `xml:"CAD,omitempty"`
	AUD int `xml:"AUD,omitempty"`
}

// MarshalXML ensures UnitAmount is not marshaled unless one or more currencies
// has a value greater than zero.
func (u UnitAmount) MarshalXML(e *xml.Encoder, start xml.StartElement) error {
	if u.USD > 0 || u.EUR > 0 || u.CAD > 0 || u.GBP > 0 || u.AUD > 0 {
		type uaAlias UnitAmount
		e.EncodeElement(uaAlias(u), start)
	}
	return nil
}
@cristiangraz
Copy link
Member

Hi @ksenish, I can't think of a way to accomplish this with the library as-is while still addressing the ability to omit a unit amount all together.

I did peak at the v3 api briefly, and one possibility is to make a breaking change that is also v3 friendly with how library users use this version and would use the v3 upgrade with minor changes. This isn't quite it, but throwing a possible starter out there:

// Currency holds a known currency type (e.g. USD/EUR).
type Currency string

// Currency constants.
const (
	USD Currency = "USD"
	EUR          = "EUR"
	GBP          = "GBP"
	CAD          = "CAD"
	AUD          = "AUD"
)

type UnitAmount struct {
	Currency   Currency  `xml:"currency"`
	UnitAmount int `xml:"unit_amount"`
}

Then the plan struct would have something like this as a field:

type Plan struct {
   // ...
   UnitAmountInCents []UnitAmount `xml:"unit_amount_in_cents,omitempty"`
   // ...
}

And a marshaler/unmarshaler that can write/read XML if the array is not nil/empty (the presence of a currency would mean the library user is requesting marshaling, which is why I went with int instead of NullInt).

I put this together quickly, but curious to hear your thoughts. Recurly's V3 API will require breaking changes, but would be nice to set this up in a way where we get the structs setup right so the user code won't need a lot of updating when they upgrade to v3.

I won't be able to make this change⁠—it's likely that I wouldn't address until I (or someone) updates the library to the v3 API. But I'm happy to discuss and review any PRs!

For reference, UnitAmount is currently used in

  • AccountBalance (read only)
  • AddOn
  • Coupon (as *UnitAmount)
  • Plan

@thisiskseniab
Copy link
Contributor Author

@cristiangraz Thank you very much for your response!

It is unfortunate there is no easy way to solve this issue. Let's hope that upgrade to v3 API will help with this 🤞

Currently, I don't have any capacity to do the upgrade to v3, but I might be able to help slowly chip away at it incrementally in a branch!

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

No branches or pull requests

2 participants