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

Added filters support for stub #270

Merged
merged 3 commits into from
Sep 14, 2016
Merged

Added filters support for stub #270

merged 3 commits into from
Sep 14, 2016

Conversation

tigran-m-dev
Copy link

Please test this improvements. I think this will be helpful.

@drewish
Copy link

drewish commented Sep 6, 2016

could you say more about how it would be used? some unit tests would be helpful towards that end.

@tigran-m-dev
Copy link
Author

For example if you need to get an account information and need to get all subscription of this account which is active. But first of all you want be sure, that account is also active.
Here is code

$account = \Recurly_Account::get($account_id);
if ($account->state == 'active')
{
  $subscriptionsForAccount = $account->subscriptions->get(); // old version
  $subscriptionsForAccount = $account->subscriptions->get((array('state' => 'active')); // new version after update
}

without this update you will get all subscription information without any filter so you should filter manually in code and not via request (which is not good point)

Surely you can get subscription for account using

$subscriptionsForAccount =\Recurly_SubscriptionList::getForAccount($account_id, array('state' => 'active'))

But in this situation you can't check account is active or not...

This is just one example with state filter, but you can use all filters which api allowed for that request.

@tigran-m-dev
Copy link
Author

@drewish Please test and let me know the status.

Thanks.

@tigran-m-dev
Copy link
Author

@drewish do you have any updates?

@drewish
Copy link

drewish commented Sep 9, 2016

Yeah it looks like a really good enhancement. Do you mind adding a unit test for the new behavior? If you want to enable the new Allow edits from maintainers feature I'm happy to help with that.

@drewish drewish self-assigned this Sep 9, 2016
@tigran-m-dev
Copy link
Author

I enabled Allow edits from maintainers

@drewish
Copy link

drewish commented Sep 12, 2016

@developer-devPHP awesome. i threw a couple of commits on there. i'll have another dev here +1 it and merge.

@drewish drewish assigned bhelx and unassigned drewish Sep 12, 2016
Tigran Makaryan and others added 3 commits September 13, 2016 18:43
When `get()`ing a stub you're returning the full object so lets refer to
it that way.
@drewish
Copy link

drewish commented Sep 14, 2016

@bhelx Actually give this a merge before you rebase #274

@bhelx
Copy link
Contributor

bhelx commented Sep 14, 2016

👍

@bhelx bhelx merged commit 0504655 into recurly:master Sep 14, 2016
@drewish drewish mentioned this pull request Sep 15, 2016
@bhelx bhelx added the V2 V2 Client label Mar 3, 2020
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