Skip to content

Jetpack: Add url request was made on/from to actor#10134

Merged
gititon merged 4 commits intomasterfrom
sync-add-from-url
Sep 13, 2018
Merged

Jetpack: Add url request was made on/from to actor#10134
gititon merged 4 commits intomasterfrom
sync-add-from-url

Conversation

@gititon
Copy link
Copy Markdown
Contributor

@gititon gititon commented Sep 12, 2018

This PR adds the url the request was made on/from to the sync actor to assist in troubleshooting issues where the actor and or payload data does not seem correct.

Testing instructions:

Please see the detailed instructions I've added for how to observe Jetpack Sync data that is sent from Jetpack to a WPCOM sandbox: PCYsg-hnH-p2

Please follow the above instructions, make a post on your Jetpack site, and confirm that the sync payload (array) that appears in your terminal has a from_url field.

@gititon gititon added [Status] Needs Review This PR is ready for review. [Package] Sync labels Sep 12, 2018
@gititon gititon requested a review from a team September 12, 2018 15:44
@gititon gititon requested a review from a team as a code owner September 12, 2018 15:44
@jetpackbot
Copy link
Copy Markdown
Collaborator

Warnings
⚠️

"Proposed changelog entry" is missing for this PR. Please include any meaningful changes

This is automated check which relies on PULL_REQUEST_TEMPLATE.We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS


function get_request_url() {
return 'http' . (isset($_SERVER['HTTPS']) ? 's' : '') . '://' . "{$_SERVER['HTTP_HOST']}{$_SERVER['REQUEST_URI']}";
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rather relying on $_SERVER here, which can be unreliable, I recommend using, the approach outlined here:

https://konstantin.blog/2012/current-url-in-wordpress/

Copy link
Copy Markdown
Contributor

@roccotripaldi roccotripaldi left a comment

Choose a reason for hiding this comment

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

This has worked well on the few server configs i've tested on - Jetpack sandbox and docker with ngrok.

@jeherve jeherve requested a review from a team September 13, 2018 10:26
@brbrr brbrr added the Enhancement Changes to an existing feature — removing, adding, or changing parts of it label Sep 13, 2018
@dereksmart
Copy link
Copy Markdown
Contributor

confirm that the sync payload (array) that appears in your terminal has a from_url field.

Confirmed there is a from_url field!
Note that when testing settings in the Jetpack admin, the from_url value is a REST url, and not the URL of the admin page. I also sometimes get the URL as {siteurl}/wp-admin/admin-ajax.php

Talked with @gititon in Slack, who says this is fine for now.

@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 Sep 13, 2018
@gititon gititon merged commit c0bbf93 into master Sep 13, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Sep 13, 2018
@gititon gititon deleted the sync-add-from-url branch September 13, 2018 19:59
jeherve added a commit that referenced this pull request Sep 24, 2018
jeherve added a commit that referenced this pull request Sep 25, 2018
* Readme: add boilerplate for next release, 6.6

* Add 6.5 to the changelog.txt file

* Set boilerplate testing list for 6.6

* Readme: update stable tag to 6.5

* Add bullets to 6.5 changelog items

* Readme: add link to previous changelogs

This will help folks who want to know more about past releases,
while keeping the readme.txt short so as to not overwhelm translators and site owners only looking for information about the last release.

* Changelog: add information at the top of the changelog file.

* Changelog: add #10054

* Changelog: add #10078

* Changelog: add #10079

* Changelog: add #10064

* Changelog: add #10094

* Changelog: add #10096

* Testing list: add more information based on #10087

* Changelog: add #9847

* Changelog: add #10084

* Changelog: add #9918

* Changelog: add #7614

* Changelog: add #10116

* Changelog: add #10108

* Changelog: add #10041

* Changelog: add #10121

* Changelog: add #10134

* Changelog: add #10130

* Changelog: add #10109

* changelog: add #10137

* changelog: add #9952

* changelog: add #10120

* changelog: add #10162

* Changelog: add #10163

* Changelog: add #10092

* changelog: add #10156

* Changelog: add #10154

* changelog: add #10122

* Changelog: add #10101

* changelog: add #10105

* changelog: add #10190

* Changelog: add #10196

* changelog: add #10152

* Changelog: add #10153

* Testing list: add more details to Site Verification testing steps.

@see #10143 (comment)

* changelog: add #10194

* Changelog: add #10193
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 [Package] Sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants