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

fix(web_services): create_api_user() and create_user_token() work again #10106

Merged
merged 1 commit into from Aug 23, 2016

Conversation

mrclay
Copy link
Member

@mrclay mrclay commented Aug 22, 2016

These uses of the ElggCrypto constructor had not been properly updated when the SiteSecret dependency was added.

(replaces #10100)

@hypeJunction
Copy link
Contributor

You switched these to crypto only recently. Have they made it into a release already?

@hypeJunction
Copy link
Contributor

Maybe we should add public API for these. I tend to use it now and then

@mrclay
Copy link
Member Author

mrclay commented Aug 22, 2016

2.2.0 went out with broken web services.

@hypeJunction
Copy link
Contributor

Well, given it's broken, IMO we should just add public API and fix it. I don't necessarily like null SiteSecret in constructor

@mrclay
Copy link
Member Author

mrclay commented Aug 23, 2016

We can add API and deprecated the public use of this in 2.x

public function __construct(SiteSecret $siteSecret) {
$this->siteSecret = $siteSecret;
public function __construct(SiteSecret $site_secret = null) {
if (!$site_secret) {
Copy link
Member Author

@mrclay mrclay Aug 23, 2016

Choose a reason for hiding this comment

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

Eh, I'll just remove this change. This class has always been private API. It's tough love for plugins who go ahead and use it.

Copy link
Contributor

Choose a reason for hiding this comment

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

SGTM.

These uses of the `ElggCrypto` constructor had not been properly updated
when the `SiteSecret` dependency was added.
@mrclay mrclay merged commit 7ccf5fe into Elgg:2.2 Aug 23, 2016
@mrclay mrclay deleted the Wouter0100-patch-7 branch August 23, 2016 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants