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

\Apigee\ManagementAPI\DeveloperApp::loadByAppId() should not return a CompanyApp #80

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mxr576
Copy link
Contributor

@mxr576 mxr576 commented Aug 7, 2017

Besides this is a bad practice it also brakes \Apigee\ManagementAPI\AbstractApp::loadFromResponse() call if an app is a company app. In that case, this method is unable to fill in its first argument's properties from the API response. This is the reason why you had to grab the returned value of the loadByAppId() in the \ApigeeCompanyAppController::delete() in the Apigee Developer Portal's code base, because $dev_app0's properties remain empty.

After applying this fix you'll probably fix some calls on the portal where you have relied on this bad implementation. For example in this delete() call I'd check whether the app is a company app if it is, I'd delete it as a company app, otherwise as a simple developer app.

anything different than a DeveloperApp entity.
@mxr576 mxr576 requested a review from cnovak May 9, 2018 09:52
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.

1 participant