set up purchase subscription schema

[everdance]

we can not simply use NewSubscription object in Subscriptions array as Purchase object already includes some new subscription fields, and Recurly server responds XML invalid if we include these fields in subscriptions node. So we need to set up PurchaseSubscription specifically for purchase interface.

e.g, if we pass currency at subscription level, recurly will respond with:

<?xml version="1.0" encoding="UTF-8"?>
<error>
    <symbol>invalid_xml</symbol>
    <description>The provided XML was invalid.</description>
    <details>32:0: ERROR: Element 'currency': This element is not expected.</details>
</error>

[everdance]

@cristiangraz please take a look, thank you.

[everdance]

@cristiangraz just ping again in case you missed previous notification. thank you.

[cristiangraz]

Will take a look at this today.

[cristiangraz]

Hi @everdance,

I could use some more information on this.

It sounds like you are calling Purchases.Create() and Purchase.Subscriptions has extra fields that are not accepted by Recurly? Is that correct?

Recurly's API documentation is really poor -- they say a list of subscription objects but their example just shows the subscription code being sent. Not sure if they require an entire subscription object or if you only need to send:

<subscriptions>
    <subscription>
      <plan_code>plan1</plan_code>
    </subscription>
  </subscriptions>

with nothing but the plan_code allowed. Or if they accept additional fields, but only some fields and not all (if so, how do we determine which fields are allowed and which ones are not?)

Seeing a sample of the code made with this library and some more information would be really helpful.

[everdance]

@cristiangraz I've tested against the API endpoint specifically with the following fields. so the fields can be sent must be a subset of it. Do you want me to add the example fields to test?

type PurchaseSubscription struct {
    XMLName              xml.Name             `xml:"subscription"`
    PlanCode             string               `xml:"plan_code"`
    SubscriptionAddOns   *[]SubscriptionAddOn `xml:"subscription_add_ons>subscription_add_on,omitempty"`
    UnitAmountInCents    int                  `xml:"unit_amount_in_cents,omitempty"`
    Quantity             int                  `xml:"quantity,omitempty"`
    TrialEndsAt          NullTime             `xml:"trial_ends_at,omitempty"`
    StartsAt             NullTime             `xml:"starts_at,omitempty"`
    TotalBillingCycles   int                  `xml:"total_billing_cycles,omitempty"`
    RenewalBillingCycles NullInt              `xml:"renewal_billing_cycles,omitempty"`
    NextBillDate         NullTime             `xml:"next_bill_date,omitempty"`
    AutoRenew            bool                 `xml:"auto_renew,omitempty"`
    CustomFields         *CustomFields        `xml:"custom_fields,omitempty"`
}

[everdance]

@cristiangraz what do you think?

[cristiangraz]

@everdance I emailed Recurly support asking them to clarify this. Clearly the full subscription object isn't accepted, but I'd like to make sure we get the full list of accepted fields. I usually get responses from them within 1-2 business days so it shouldn't be long.

In the meantime I'll look over the PR and add any feedback (if any) so once we hear back we can just modify the field list and any corresponding tests and get this merged!

[everdance]

@cristiangraz lmk if you got any updates from recurly. thanks

[cristiangraz]

Hi @everdance, I had some back-and-forth with Recurly support but did finally hear back. Here is the main part of the response:

What I'm wondering is: Rather than creating a new struct, if you create a purchase as-is but just ensure that fields like Currency aren't sent with the subscription but instead with the purchase, will everything go through?

I'm hesitant to introduce more custom subscription types, and in fact this is an area where I want to further look at consolidating into a single Subscription struct in the library and just adding validation. So for example, the CreatePurchase function might inspect each of the attached Subscription struct fields and return an error (before calling Recurly) if currency was set on the subscription object.

Coupled with comments on the specific functions, this could help reduce the number of structs but give users specific error messages.

Let me know if altering your request with the current master branch, knowing the info above, works. If so I'd propose we just carefully document it and then do a larger simplification/validation PR with subscriptions.

[everdance]

@cristiangraz thanks for the feedback. I actually have tested and verified every field in current new subscription payload whether they can be sent in purchase->subscription. So the following fields are directly included in Purchase and can not be passed in its inner Subscription struct:

account, collection_method, currency, po_number, net_terms, 
coupon_codes, gift_card, customer_notes, terms_and_conditions, 
gateway_code, vat_reverse_charge_notes, shipping_address_id

If we do not introduce a new type struct, it would be very awkward to copy and marshaling these duplicative fields internally in the library and it's also confusing for lib interface.

I don't think we can easily migrate to an unified model here due to non-restful recurly API. And if we need to find a way out in Go, then we probably should consider using generic map struct for request struct.

[cristiangraz]

Thanks for the clarification, @everdance. It's unfortunate we can't easily unify the types. Going to merge this in, appreciate the contribution!