Skip to content

Conversation

cpapazoglou
Copy link
Contributor

@cpapazoglou cpapazoglou commented Sep 29, 2021

Changes proposed in this Pull Request:

  • Removes preferred_view conditional since the experience doesn't depend on user's choice
  • Adds a filter in Atomic plugins menu so that we can override the behavior from wpcomsh until Marketplace is launched. When marketplace is launched we will remove that filter condition.

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Simple Sites

  • apply D67583
  • Visit your sandbox /wp-admin
  • Make sure that Plugins menu doesn't have any submenu and that it links to Calypso https://wordpress.com/plugins/[DOMAIN]

Atomic Sites
EITHER

  • add
function my_wpcomsh_update_plugin_add_filter() {
	// Changes plugin-install.php destination to Calypso.
	add_filter( 'wpcom_marketplace', '__return_true' );
}
add_action( 'admin_menu', 'my_wpcomsh_update_plugin_add_filter' );

in a mu-plugin

OR

  • go to MC of your atomic dev blog and add wpcom-marketplace sticker
  • in your atomic dev blog, copy the diff from current jetpack pr and wpcomsh part 758-gh-Automattic/wpcomsh/files
  • visit your site
  • make sure that Plugins -> Add New links to Calypso https://wordpress.com/plugins/[DOMAIN]

@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello cpapazoglou! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D67583-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

@github-actions github-actions bot added [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Feature] Masterbar WordPress.com Toolbar and Dashboard customizations Unit Tests labels Sep 29, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Sep 29, 2021

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ⚠️ All commits were linted before commit.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • 🔴 Add changelog entries to affected projects

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.


🔴 Action required: Please add missing changelog entries for the following projects: projects/packages/post-list

Use the Jetpack CLI tool to generate changelog entries by running the following command: jetpack changelog add.
Guidelines: /docs/writing-a-good-changelog-entry.md


Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.


Jetpack plugin:

  • Next scheduled release: November 2, 2021.
  • Scheduled code freeze: October 25, 2021.

@github-actions github-actions bot added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Sep 29, 2021
@cpapazoglou cpapazoglou force-pushed the update/change_plugin_install_destination_depending_on_hook branch from 5f0cb37 to f04ffdb Compare October 5, 2021 08:32
@cpapazoglou cpapazoglou changed the title [WIP] Update/change plugin install destination depending on hook Update/change plugin install destination depending on hook Oct 5, 2021
@cpapazoglou cpapazoglou added [Status] Needs Team Review Obsolete. Use Needs Review instead. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Oct 5, 2021
@gavande1
Copy link
Contributor

gavande1 commented Oct 5, 2021

It works as described. 👍

@gavande1 gavande1 added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Team Review Obsolete. Use Needs Review instead. labels Oct 5, 2021
Copy link
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.

I just have a quick note. Additionally, what would you think about adding a new test for that potential new menu item scenario?

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. nav-unification and removed [Status] Needs Review This PR is ready for review. labels Oct 5, 2021
@cpapazoglou cpapazoglou added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Oct 5, 2021
@cpapazoglou cpapazoglou force-pushed the update/change_plugin_install_destination_depending_on_hook branch from d98a807 to bca979d Compare October 5, 2021 15:39
@cpapazoglou
Copy link
Contributor Author

cpapazoglou commented Oct 5, 2021

I just have a quick note. Additionally, what would you think about adding a new test for that potential new menu item scenario?

Good idea @jeherve, added a test bca979d (and rebased)

Copy link
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.

Thanks for adding a test. It looks like it's failing on multisite at the moment:

  1) Test_Atomic_Admin_Menu::test_add_plugins_menu
  Undefined offset: 1
  
  /tmp/wordpress-master/src/wp-content/plugins/jetpack/tests/php/modules/masterbar/test-class-atomic-admin-menu.php:335
  
  ERRORS!
  Tests: 1723, Assertions: 4739, Errors: 1, Skipped: 29, Incomplete: 7.

-- https://github.com/Automattic/jetpack/runs/3805159973

Do you think you could give it a look?

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Oct 5, 2021
@cpapazoglou cpapazoglou force-pushed the update/change_plugin_install_destination_depending_on_hook branch from c2b7be2 to 34bc0c9 Compare October 7, 2021 06:34
@github-actions github-actions bot added [Package] Post List [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Oct 7, 2021
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Oct 7, 2021
@jeherve jeherve added this to the jetpack/10.3 milestone Oct 7, 2021
@cpapazoglou cpapazoglou merged commit 2c98c55 into master Oct 7, 2021
@cpapazoglou cpapazoglou deleted the update/change_plugin_install_destination_depending_on_hook branch October 7, 2021 11:40
@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2021

Great news! One last step: head over to your WordPress.com diff, D67583-code, and commit it.
Once you've done so, come back to this PR and add a comment with your changeset ID.

Thank you!

@github-actions github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 7, 2021
davidlonjon added a commit that referenced this pull request Oct 7, 2021
* master:
  Update/change plugin install destination depending on hook (#21236)
  Calendly block: Fix preview in the block inserter (#21323)
  Post List: Release 0.2.1 (#21316)
davidlonjon added a commit that referenced this pull request Oct 7, 2021
* master:
  Update/change plugin install destination depending on hook (#21236)
  Calendly block: Fix preview in the block inserter (#21323)
  Post List: Release 0.2.1 (#21316)
davidlonjon added a commit that referenced this pull request Oct 7, 2021
* master:
  Update/change plugin install destination depending on hook (#21236)
  Calendly block: Fix preview in the block inserter (#21323)
  Post List: Release 0.2.1 (#21316)
davidlonjon added a commit that referenced this pull request Oct 7, 2021
* master:
  Update/change plugin install destination depending on hook (#21236)
  Calendly block: Fix preview in the block inserter (#21323)
  Post List: Release 0.2.1 (#21316)
davidlonjon added a commit that referenced this pull request Oct 7, 2021
…boost

* master:
  Update/change plugin install destination depending on hook (#21236)
  Calendly block: Fix preview in the block inserter (#21323)
  Post List: Release 0.2.1 (#21316)
davidlonjon added a commit that referenced this pull request Oct 8, 2021
…workflow

# By Brad Jorsch (17) and others
# Via GitHub
* master: (151 commits)
  Boost: Fix overall speed scores info tooltip UI (#21319)
  Fix regular expressions in regression-checklist config (#21301)
  CLI: Add example.php inside src folder to plugin/package skeleton (#21333)
  mirrors: Add auto-release action (#21335)
  Pin dependencies (#21312)
  Explicitly ignore locale if same as site setting (#21305)
  Block Icons: change foreground color on WPCOM (#21328)
  Jetpack Plugin: Move <LoadingCard> to be generally accessible (#21197)
  VaultPress: Add Changelog for 2.2.0 (#21318)
  Boost: fix no-boost tooltip overlay issue (#21321)
  Update/change plugin install destination depending on hook (#21236)
  Calendly block: Fix preview in the block inserter (#21323)
  Post List: Release 0.2.1 (#21316)
  [not verified] Post List: Release 0.2.0 (#21309)
  Update dependency yoast/phpunit-polyfills to v1.0.2 (#21314)
  Init version 3.0.4-alpha (#21310)
  Allow an empty message string in the publicize share endpoint. (#21306)
  Beta Plugin: release v3.0.3 (#21308)
  Unit Test updates (#21307)
  Publicize: use post metadata as the primary data source. (#21231)
  ...

# Conflicts:
#	tools/e2e-commons/config/ecosystem.config.js
@cpapazoglou
Copy link
Contributor Author

Deployed r233707-wpcom

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Masterbar WordPress.com Toolbar and Dashboard customizations Marketplace nav-unification [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ Touches WP.com Files Unit Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants