Skip to content

JSON API: Remove obsolete code to query invites directly from a JP site#8515

Merged
zinigor merged 2 commits intomasterfrom
remove/invites-direct-queries
Jan 30, 2018
Merged

JSON API: Remove obsolete code to query invites directly from a JP site#8515
zinigor merged 2 commits intomasterfrom
remove/invites-direct-queries

Conversation

@nylen
Copy link
Copy Markdown
Contributor

@nylen nylen commented Jan 12, 2018

All invite data has been stored on WP.com for some time now. This PR assumes this is true for multisite Jetpack installations as well, which appears to be the case because I can't find any Jetpack code that calls this endpoint.

If this is indeed the case, then we can simplify this endpoint pretty dramatically (short of a full removal of the endpoint from Jetpack, which would be another option).

All invite data has been stored on WP.com for some time now.
@nylen nylen added [Feature] WPCOM API [Status] Needs Review This PR is ready for review. labels Jan 12, 2018
@nylen nylen requested a review from ebinnion January 12, 2018 21:37
@nylen nylen requested a review from a team as a code owner January 12, 2018 21:37
@jeherve jeherve added the Enhancement Changes to an existing feature — removing, adding, or changing parts of it label Jan 16, 2018
if ( ! is_multisite() ) {
return new WP_Error( 'forbidden', 'To query invites, site must be on a multisite installation.', 403 );
if ( ! defined( 'IS_WPCOM' ) || ! IS_WPCOM ) {
return new WP_Error( 'not_implemented', 'Querying WP.com endpoint data from a Jetpack site is not implemented.', 400 );
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This should say "invite" instead of "endpoint".

@ebinnion
Copy link
Copy Markdown
Contributor

@nylen Apologies for the slow reply. I've added a task to review this tomorrow.

@nylen
Copy link
Copy Markdown
Contributor Author

nylen commented Jan 23, 2018

No worries. After thinking about this some more, I think we should remove this endpoint from Jetpack entirely and moving it to WPCOM-only. If I am understanding the situation correctly, it hasn't been able to do anything for a good while.

@ebinnion
Copy link
Copy Markdown
Contributor

After thinking about this some more, I think we should remove this endpoint from Jetpack entirely and moving it to WPCOM-only

I agree, especially since we're having to add ?force_wpcom=true

@nylen nylen 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 Jan 24, 2018
@nylen
Copy link
Copy Markdown
Contributor Author

nylen commented Jan 24, 2018

We could also remove this one: https://github.com/Automattic/jetpack/blob/master/json-endpoints/class.wpcom-json-api-update-invites-endpoint.php

However, there's not as much of a reason to do it, because as far as I can tell, we don't need to make any changes to that endpoint to support new invite features.

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.

Looks great! Thank you :)

@zinigor zinigor merged commit 02dc070 into master Jan 30, 2018
@zinigor zinigor deleted the remove/invites-direct-queries branch January 30, 2018 11:56
@zinigor zinigor removed the [Status] Ready to Merge Go ahead, you can push that green button! label Jan 30, 2018
jeherve added a commit that referenced this pull request Jan 30, 2018
zinigor pushed a commit that referenced this pull request Jan 30, 2018
* Changelog 5.8: create base for changelog.

* Update 5.8 release post link

* fix 5.8 release date

* Updates to plugin description

* Changelog: add #8499

* Changelog: add #8506

* Changelog: add #8509

* Changelog: add #8516

* Changelog: add #8517

* Changelog: add #8523

* Changelog: add #8547

* Changelog: add #8496

* Changelog: add #8584

* Changelog: add #8595

* Changelog: add #8445

* Changelog: add #8431

* Changelog: add #8284

* Changelog: add #8270

* Changelog: add #8124

* Changelog: add #8581

* Changelog: add #8463

* Changelog: add #8568 (#8646)

* Updates to testing list and changelog

* Changelog: add #8443

* Changelog: add #8459

* Changelog: add #8469

* Changelog: add #8464

* Changelog: add #8478 and #8479

* Changelog: add #8483

* Changelog: add #8488

* Changelog: add #8513

* Changelog: add #8555

* Changelog: add #8565

* Changelog: add #8601

* Changelog: add #8612

* Changelog: add first pass at Search items.

* Changelog: add more info to help test Search.

* Changelog: add #8144

* Changelog: add #8313

* Changelog: add #8419

* Changelog: add #8465

* Changelog: add #8515

* Changelog: add #8587

* Changelog: add #8591

* Changelog: add #8659

* Changelog: add #8661

* Changelog: add #8671

* Changelog: add 5.7.1 to archived changelog too.

* Reverted changes to readme, removed entry about backups.
nylen added a commit that referenced this pull request Feb 23, 2018
This endpoint does not work with the current implementation of invites
management.

All invite data is stored on the WP.com server and manipulated via
WP.com-only API endpoints, so we can remove this code from Jetpack.

Follow-up to #8515.
dereksmart pushed a commit that referenced this pull request Feb 26, 2018
This endpoint does not work with the current implementation of invites
management.

All invite data is stored on the WP.com server and manipulated via
WP.com-only API endpoints, so we can remove this code from Jetpack.

Follow-up to #8515.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] WPCOM API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants