Skip to content

XMLRPC: remove activateManage method#6990

Merged
dereksmart merged 1 commit intomasterfrom
remove/xmlrpc-action-activate-manage-module
Apr 25, 2017
Merged

XMLRPC: remove activateManage method#6990
dereksmart merged 1 commit intomasterfrom
remove/xmlrpc-action-activate-manage-module

Conversation

@roccotripaldi
Copy link
Copy Markdown
Contributor

Since Jetpack 4.4, the Manage Module is now activated by default. We no longer need an XMLRPC method for it.

To test:

Add this PR to one of you Jetpack sites.
Test the various Jetpack Connect flows, and ensure there are no regressions.

Proposed changelog entry for your changes:

Removing an no longer necessary XMLRPC method.

Since Jetpack 4.4, the Manage Module is now activated by default. We no longer need an XMLRPC method for it.
Copy link
Copy Markdown
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

[redacted duplicate]

Copy link
Copy Markdown
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

I checked out this branch on a sandbox site (reports 4.9-alpha).

  • From wordpress.com/jetpack/connect
  • From site's Jetpack dashboard ✅

Are there other flows which should be tested?

@roccotripaldi roccotripaldi requested a review from zinigor April 19, 2017 15:19
Copy link
Copy Markdown
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Although the Manage module is enabled by default on new sites, it can still be disabled on existing sites. Shouldn't we leave the option in there to support existing installations where Manage is not currently enabled?

@roccotripaldi
Copy link
Copy Markdown
Contributor Author

Shouldn't we leave the option in there to support existing installations where Manage is not currently enabled?

I'm not sure. Can you elaborate on situations where you think it may be needed.

@jeherve
Copy link
Copy Markdown
Member

jeherve commented Apr 19, 2017

Can you elaborate on situations where you think it may be needed.

What happens if you deactivate Manage on your site (via wp jetpack module deactivate manage for example), and then go to WordPress.com/plugins? You should get a prompt asking you to enable Manage there.

@roccotripaldi
Copy link
Copy Markdown
Contributor Author

roccotripaldi commented Apr 19, 2017

Ah, thanks for clarifying. In that case, we send folks back to wp-admin to activate, and I think we should continue to do that rather than use an XMLRPC method. Longer term we are looking to rely less on, if not fully deprecate XMLRPC methods. If need be, we can use WP API to turn on manage from calypso.

@jeherve
Copy link
Copy Markdown
Member

jeherve commented Apr 19, 2017

That makes sense. It seems the method is still mentioned in one of our wpcom API endpoints on WordPress.com (jetpack-blog-endpoint), but I can't seem to find any log of this being used in our logs. I guess that means we're good to go! 👍

@roccotripaldi
Copy link
Copy Markdown
Contributor Author

It seems the method is still mentioned in one of our wpcom API endpoints on WordPress.com (jetpack-blog-endpoint )

True, and we are going to nuke that too!

Automattic/wp-calypso#13205 (comment)

Copy link
Copy Markdown
Contributor

@zinigor zinigor left a comment

Choose a reason for hiding this comment

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

LGTM, but let's merge the Calypso counterpart first.

@dereksmart
Copy link
Copy Markdown
Contributor

Calypso PR has been merged. Merging to Jetpack 🐑

@dereksmart dereksmart added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Apr 25, 2017
@dereksmart dereksmart merged commit 6d41fbe into master Apr 25, 2017
@dereksmart dereksmart removed the [Status] Ready to Merge Go ahead, you can push that green button! label Apr 25, 2017
@dereksmart dereksmart deleted the remove/xmlrpc-action-activate-manage-module branch April 25, 2017 00:10
jeherve added a commit that referenced this pull request Apr 25, 2017
eliorivero pushed a commit that referenced this pull request Apr 25, 2017
* Changelog: initial commit for 4.9 release.

* Changelog: add #6929

* Changelog: move old changelogs to changelog.txt

* Readme: restore deleted release post link.

The post is now live.

* Changelog: add #6853

* Changelog: add #6856

* Changelog: add #6857

* Changelog: add #6884

* Changelog: add #6885

* Changelog: add #6892

* Changelog: add #6894

* Changelog: add #6898

* Changelog: add #6899

* Changelog: add #6900

* Changelog: add #6909

* Changelog: add #6927

* Changelog: add #6947

* Chagelog: add #6958

* Changelog: add #6961

* Changelog: add #6963

* Changelog: add #6965

* Changelog: add #6986

* Changelog: add #7000

* Changelog: add #7013

* Changelog: add #7015

* Changelog: add #7019

* Changelog: add #7028

* Changelog: add #6998

* Changelog: add #6999

* Changelog: add #7044

* Changelog: add #6881

* Changelog: add #6922

* Changelog: add #6940

* Changelog: add #6962

* Changelog: add #6942

* Changelog: add #6959

* Changelog: add #7018

* Changelog: add #6948

* Changelog: add #6657

* Changelog: add #7030

* Changelog: add #7048

* Changelog: add #7031

* Changelog: add #6990

* Changelog: add #6957

* Changelog: add #7027
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants