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 ramp pricing feature to plans #700

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

Smolations
Copy link

@Smolations Smolations commented Aug 2, 2022

The Ramp Pricing feature is supported as a result of this change. Also, some refactoring was done to start curbing bad patterns and separate code out into logical chunks:

  • The CONTRIBUTING.md file was updated to provide a bit more direction for developers looking to add to this library with no prior experience.
  • An XmlTools class was created with the aim of being a purely static class to handle individual XML operations. Much of the Recurly_Resource code was moved here.
  • An XmlDoc trait was created with the aim of being adding the ability to represent any class as an XML node/document. It includes mechanisms to track the possible child nodes of a class as well as handle the conversion from the object into an XML document string.
  • Recurly_Base and Recurly_Resource were overhauled to accommodate these new tools, moving in a more common sense direction around model relationships in this library (e.g. a "resource" can be directly fetched from the server and a "node" (or doc) simply needs to be represented as XML).

To see how the new ramp pricing feature is utilized, see the spec tests added in this change.

@Smolations Smolations force-pushed the add-ramp-pricing-feature-to-plans branch 9 times, most recently from cd50190 to f62a0aa Compare August 2, 2022 22:35
- implements ramp interval features for plans
- adds specs for new ramp intervals feature
- does some other refactoring to harden tests
- backfills some missing specs

Co-authored-by: danilocandido <dcandido@recurly.com>
@Smolations Smolations force-pushed the add-ramp-pricing-feature-to-plans branch from 018bc62 to 50bbe55 Compare August 3, 2022 19:51
@Smolations Smolations marked this pull request as ready for review August 3, 2022 19:51
@@ -83,8 +117,36 @@ public function testUpdateXml() {
$plan->dunning_campaign_id = '1234abcd';

$this->assertEquals(
"<?xml version=\"1.0\"?>\n<plan><plan_code>platinum</plan_code><name>Platinum Plan</name><unit_amount_in_cents><USD>1500</USD><EUR>1200</EUR></unit_amount_in_cents><setup_fee_in_cents><USD>500</USD><EUR>500</EUR></setup_fee_in_cents><total_billing_cycles nil=\"nil\"></total_billing_cycles><tax_exempt>false</tax_exempt><tax_code>fake-tax-code</tax_code><trial_requires_billing_info>false</trial_requires_billing_info><dunning_campaign_id>1234abcd</dunning_campaign_id></plan>\n",
"<?xml version=\"1.0\"?>\n<plan><dunning_campaign_id>1234abcd</dunning_campaign_id><name>Platinum Plan</name><plan_code>platinum</plan_code><setup_fee_in_cents><USD>500</USD><EUR>500</EUR></setup_fee_in_cents><tax_code>fake-tax-code</tax_code><tax_exempt>false</tax_exempt><total_billing_cycles nil=\"nil\"></total_billing_cycles><trial_requires_billing_info>false</trial_requires_billing_info><unit_amount_in_cents><USD>1500</USD><EUR>1200</EUR></unit_amount_in_cents></plan>\n",

Choose a reason for hiding this comment

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

i think if you use assertXmlStringEqualsXmlString you can compare if that xml is equal to plan xml and indent it to be more readable

Copy link
Author

Choose a reason for hiding this comment

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

Yea that definitely seems like it would be better. However, it has been "decreed" that no more refactors take place (though I'm not sure how that's gonna be possible 😅) so since I was just fixing a previously broken test I didn't spend any additional time. However, my upcoming work might touch/break some tests that are similar here and I think I could make it better. We shall see!

protected function updateErrorAttributes()
{
if (sizeof($this->_errors) > 0) {
for ($i = sizeof($this->_errors) - 1; $i >= 0; $i--) {

Choose a reason for hiding this comment

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

maybe $i could be $errorIndex?

Copy link
Author

@Smolations Smolations Aug 5, 2022

Choose a reason for hiding this comment

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

If I'm reading this right, $i represents the size of that array, yea? This was just a method I had shuffled around so I didn't actually write this code.. 😅

EDIT: Didn't read it closely enough. You're right, it represents an index for the loop.. 🙈

@btruncali1 btruncali1 merged commit f10588a into v2 Aug 8, 2022
@btruncali1 btruncali1 deleted the add-ramp-pricing-feature-to-plans branch August 8, 2022 17:21
recurly-integrations pushed a commit that referenced this pull request Aug 24, 2022
[Full Changelog](2.12.25...2.12.26)

**Merged Pull Requests**

- Add Ramp Pricing Feature to Subscriptions [#703](#703) ([Smolations](https://github.com/Smolations))
- Add ramp pricing feature to plans [#700](#700) ([Smolations](https://github.com/Smolations))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants