-
Notifications
You must be signed in to change notification settings - Fork 815
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
MU WPCOM: dashboard: add general tasks widget scaffolding + one card (domain upsell) #41150
Conversation
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Follow this PR Review Process:
Still unsure? Reach out in #jetpack-developers for guidance! Mu Wpcom plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Wpcomsh plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
d486bb7
to
b0646aa
Compare
Code Coverage SummaryCoverage changed in 2 files.
|
Code Coverage SummaryCoverage changed in 2 files.
Full summary · PHP report · JS report Coverage check overridden by
I don't care about code coverage for this PR
|
@@ -13,7 +15,36 @@ function load_wpcom_dashboard_widgets() { | |||
return; | |||
} | |||
|
|||
enqueue_wpcom_dashboard_widgets(); | |||
$layout_response = Client::wpcom_json_api_request_as_blog( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, you need to request_as_user
here, then you won't need to "fix" authentication on the server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh! That works! 😮 Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wpcom_json_api_request_as_user
does not seem to work on dot com, so I've opted for using wpcom_json_api_request_as_blog
in that case for now. Only that method seems to support internal requests. I wonder if there's anything preventing us from adding it to wpcom_json_api_request_as_user
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wpcom_json_api_request_as_user
does not seem to work on dot com
What does it do instead of working? And what does "on dot com" mean? On a Simple site? Or on public-api
? A Simple site should never call request_as_user
or request_as_blog
in the first place: that means sending a REST request basically to itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should make an internal request in that case, see #41864
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wpcom-api-direct
thing is very interesting. Maybe that's what we need to make Gutenberg preloading work 🙂
4314a1f
to
c4e2078
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
b3ac1c8
to
9d08e7a
Compare
Thank you @jsnajdr! Let's follow-up with a fix to make |
See Automattic/wp-calypso#95386.
Proposed changes:
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions:
onboarding_checklist_completed
site option (OR if your blog is older than 2018-02-01, it should always appear). If you need a site to test you can useellaiseulde
.