Skip to content
This repository has been archived by the owner on Jun 7, 2022. It is now read-only.

Disabling wc-admin: Remove filter #6568

Closed
wants to merge 2 commits into from
Closed

Conversation

psealock
Copy link
Collaborator

@psealock psealock commented Mar 11, 2021

Removes the ability to turn off wc-admin.

Now that wc-admin is in Core, more and more functionality is baked into the plugin, and extensions build off its functionality, it makes less and less sense to allow users or plugins to disable it.

This PR removes the ability to disable the plugin and will be a good space to discuss the ramifications.

@psealock
Copy link
Collaborator Author

cc @timmyc @elizaan36 @pmcpinto

Copy link
Contributor

@joshuatf joshuatf left a comment

Choose a reason for hiding this comment

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

Code-wise this looks good to me.

I would like it if all of our features were better encapsulated so that they could be toggled on/off, but that would probably be a rather large refactor to some areas of WooCommerce Admin.

@leewillis77
Copy link
Contributor

#6563 is related (and could be closed if this gets merged)

@timmyc timmyc added the status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. label Mar 11, 2021
@timmyc
Copy link
Contributor

timmyc commented Mar 11, 2021

Added the blocked label here since this is likely something we will need to do a bit of coordination on.

In 5.2 of core we will start collecting Tracker data ( for woocommerce.com connected sites ) to get a sense of how many sites are currently toggling wc-admin off via filter.

I also feel we might want to do some community outreach here and get some further details from store owners / builders who have deactivated wc-admin and see if there are certain features they would like to toggle off, and maybe we need to expand the feature flag system to allow a finer control over active features.

@timmyc
Copy link
Contributor

timmyc commented Mar 11, 2021

Just saw this comment from @lkraav that really speaks to the desire for finer grained controls of wc-admin features. The primary feature, as mentioned in that comment, that I personally feel users will want to disable is Analytics since many sites do already have an analytics toolkit established that they prefer to use.

Adding in full feature flag support for analytics might be a bit of work considering how integrated it is, but is definitely feasible.

@joshuatf
Copy link
Contributor

Adding in full feature flag support for analytics might be a bit of work considering how integrated it is, but is definitely feasible.

Strongly agree.

@roykho
Copy link
Member

roykho commented Mar 12, 2021

We should definitely allow ways for plugins to disable specific or all features of WC Admin. A good use case would be Product Vendors extension where the logged in user is a vendor admin. They have their own customized WP menu items such as custom reports and such however they do share the Products menu with the site admin. WC Admin attempts to hijack this and causes issues for vendor admins where the Products menu item disappears.

Since vendor admins don't need to use any of the features, would be good to allow them to disable all of this. Currently the way to do this for Product Vendors is something like this:

if ( is_vendor() ) {
     add_filter( 'woocommerce_admin_features', 'remove_features' );
}

function remove_features() {
     return array();
}

Of course we could continue to use this method.

@timmyc
Copy link
Contributor

timmyc commented Mar 13, 2021

Thanks @roykho - another great use case we need to consider.

@adrianduffell
Copy link
Contributor

Some more feedback from @erikmolenaarnl in #6563 (comment).

I always set woocommerce_admin_disabled to true, as I believe a CMS is supposed to only deliver content, and not include extensive reporting tools as this should be done externally to spare resources.

@adrianduffell
Copy link
Contributor

Just noting our own documentation causes a fatal error when WC Admin is disabled with this filter. Issue #6673

@adrianduffell
Copy link
Contributor

adrianduffell commented Mar 31, 2021

Another instance where an extension had a fatal error when WC Admin is disabled #6703

Since this PR is blocked, I think we need to raise more awareness about this filter among extension developers.

@timmyc
Copy link
Contributor

timmyc commented Apr 7, 2021

Kicked off a thread to discuss this further among the dev teams: pcShBQ-1i-p2

@github-actions
Copy link

github-actions bot commented Jun 6, 2021

This PR has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the status: stale The issue/PR is stale label Jun 6, 2021
@github-actions
Copy link

This PR was automatically closed due to being stale.

@psealock
Copy link
Collaborator Author

Thanks for the discussion everyone. I'm closing this one in favour of #6568 which hopefully accomplishes the goal of keeping wc-admin the platform while allowing parts to be turned off to keep WooCommerce lean.

@psealock psealock closed this Jun 24, 2021
@lkraav
Copy link

lkraav commented Jun 24, 2021

Thanks for the discussion everyone. I'm closing this one in favour of #6568 which hopefully accomplishes the goal of keeping wc-admin the platform while allowing parts to be turned off to keep WooCommerce lean.

But this is #6568?

@adrianduffell
Copy link
Contributor

@lkraav Apologies, it should be #7232.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. status: stale The issue/PR is stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants