Skip to content
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

[3.6] Add setting to control display of Marketplace Suggestions #23218

Merged
merged 4 commits into from
Apr 9, 2019

Conversation

timmyc
Copy link
Contributor

@timmyc timmyc commented Apr 5, 2019

All Submissions:

Changes proposed in this Pull Request:

See #23198

This branch adds a new store setting which allows store operators to disable the display of Marketplace Suggestions. By default the setting to display Marketplace Suggestions is turned on/truthy. And as of now the woocommerce_allow_marketplace_suggestions filter can still over-ride this setting to always force suggestions off/on via code if that is desired.

This branch also includes a modification to the default/empty suggestions state that adds a link to the new setting to allow users to quickly manage this preference. A follow-up PR will be needed to add similar links to other suggestion states ( /cc @haszari )

How to test the changes in this Pull Request:

  1. Observe the default "suggestions enabled" functionality by editing a product, and noting the "More Options" tab is still shown
  2. Next Visit the "Accounts & Privacy" portion of the admin /wp-admin/admin.php?page=wc-settings&tab=account, toggle the Marketplace Suggestions setting off ( uncheck ), and save the settings.
  3. Repeat step one and verify the suggestions are no longer shown.

image

no-suggestions-manage-link

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Enhancement: Expose a new setting to control the display of Marketplace Suggestions.

@timmyc timmyc added this to the 3.6.0 milestone Apr 5, 2019
@codecov
Copy link

codecov bot commented Apr 5, 2019

Codecov Report

Merging #23218 into master will decrease coverage by 0.01%.
The diff coverage is 91.85%.

Impacted Files Coverage Δ Complexity Δ
...estions/templates/html-product-data-extensions.php 0% <ø> (ø) 0 <0> (ø) ⬇️
...e-suggestions/class-wc-marketplace-suggestions.php 0% <0%> (ø) 22 <0> (+1) ⬆️
...udes/admin/settings/class-wc-settings-accounts.php 86.58% <93.23%> (-7.5%) 4 <0> (+1)

Copy link
Member

@haszari haszari left a comment

Choose a reason for hiding this comment

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

Code looks good and it's working well in my testing :)

Copy link
Member

@kloon kloon left a comment

Choose a reason for hiding this comment

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

Not sure if the settings tabs are the appropriate location of these kinds of settings, specifically the accounts tab. All these tabs retains to the functionality of the store and not admin related.

Another thing, settings are manageable by the store manager role whereas suggestions are only presented to users with the capability to install plugins. Same goes for the tracking options that forms part of that settings section.

@kloon kloon added needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. and removed status: needs review labels Apr 8, 2019
@timmyc
Copy link
Contributor Author

timmyc commented Apr 8, 2019

@kloon - given the short-time frame here, can you think of another place where this should live? I like how both tracks and suggestions are toggle-able in the same spot. I feel that is a good ux for store operators.

That is a very valid point regarding suggestions only being shown to users whom can install plugins. Should I add some conditional logic to only show this setting to those who would actually see the suggestions?

@timmyc timmyc force-pushed the add/suggestions-display-setting branch from 4242541 to e4552c5 Compare April 8, 2019 23:32
@timmyc
Copy link
Contributor Author

timmyc commented Apr 8, 2019

@kloon in e4552c5 I updated the logic to only show the setting to users who can install plugins. This has made the diff look a little funky, so maybe just viewing the file is the easiest way to see the changes.

I don't think we have time or a place for this and the tracks setting to live outside if where they are shown for now, so I think we need to punt on that aspect of your review until 3.6.x

Copy link
Member

@haszari haszari left a comment

Choose a reason for hiding this comment

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

Looking good. Retested and the setting is working correctly. Also tested with a Shop manager user - setting UI wasn't present - nice :)

@timmyc timmyc merged commit 78aa676 into master Apr 9, 2019
@timmyc
Copy link
Contributor Author

timmyc commented Apr 9, 2019

@kloon & @mikejolley I went ahead and merged this in to unblock @haszari - if either of you want further changes here let me know and I can do another branch, and we can evaluate a better long-term location for these settings post 3.6.

@mikejolley
Copy link
Member

@kloon @timmyc The only places I could think of would be under advanced settings, or better, your profile under users menu since this option affects a single user, not the store..

@timmyc
Copy link
Contributor Author

timmyc commented Apr 9, 2019

The only places I could think of would be under advanced settings, or better, your profile under users menu since this option affects a single user, not the store.

I think usage tracking is definitely a store-level setting - but concur that marketplace suggestions could very well be a user-level setting. Something worth looking into, but I also fear that if we made it user-level it would be more work for all users of a store to go in and set their preference vs managing store-wide.

@mikejolley
Copy link
Member

@timmyc Then advanced might be the place for both to go but probably fine to punt.

@jessuppi
Copy link

So, currently is woocommerce_allow_marketplace_suggestions the only filter related to the various advertising and "suggestions" drama over the past few weeks, or are there others? There are so many discussions on these issues it's a bit hard to decipher... anyway, thanks!

Note: I'm not talking about Jetpack, WP Core, or other non-WooCommerce features... ;)

@haszari
Copy link
Member

haszari commented Apr 14, 2019

So, currently is woocommerce_allow_marketplace_suggestions the only filter related to the various advertising and "suggestions" drama over the past few weeks, or are there others?

Hi @jessuppi! Yes, this is the one filter related to suggestions :)

There's some more info about other recent changes in this post: https://woocommerce.wordpress.com/2019/04/10/woocommerce-3-6-rc2/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: author feedback The issue/PR needs a response from any of the parties involved in the issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants