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

Backport fix for issue #27170 in WooCommerce related to missing product type dropdown #331

Open
viktorix opened this issue Aug 13, 2021 · 15 comments
Labels
enhancement New feature or request Has PR Issue has a PR.

Comments

@viktorix
Copy link
Member

Is your feature request related to a problem? Please describe.
When Classic Commerce is used on WordPress 5.5+ the product type dropdown is missing. This is a known issue and was fixed in WooCommerce 4.3.2.

Describe the solution you'd like
Backport fix from WooCommerce 4.3.2 issue #27170.

Additional context
I'm submitting it as a feature request because this issue affects Classic Commerce only if used with WordPress 5.5+. It does not affect ClassicPress. A user brought up a good point in the forums that some folks might switch to Classic Commerce from WooCommerce before they switch from WordPress to ClassicPress to test e-commerce functionality.

@ClassyBot
Copy link

This issue has been mentioned on ClassicPress Forums. There might be relevant details there:

https://forums.classicpress.net/t/classiccommerce-missing-product-data-dropdown/3426/13

@adegans
Copy link

adegans commented Aug 13, 2021

Yes! Looking forward to having this fix.
If you need help testing, let me know.

@nylen
Copy link
Contributor

nylen commented Aug 13, 2021

The Woo team did a good job with this fix, it should correctly determine whether to use the new WP 5.5+ behavior or the previous behavior (for WP <=5.4 and also ClassicPress). Needs testing but I wouldn't expect any issues with including this in Classic Commerce.

@adegans
Copy link

adegans commented Aug 13, 2021

I don't recall this ever being broken on WooCommerce in the last 7-8 years (thats how long I've used WC) and I've relied on the 'Virtual', 'Downloadable', and 'Software' checkboxes since day one. The Software checkbox is added with the (Paid) Software Add-On extension for license keys on products. If you need a copy of this plugin to test, let me know.

I'm not very involved on the actual issue other than knowing it "always worked". So it sort of baffles me it's not working now 🤨

@nylen
Copy link
Contributor

nylen commented Aug 13, 2021

Unfortunately may not be a straightforward backport as it looks like Woo has made some other changes that are pre-requisites for including this fix, but we don't have those pre-requisite changes yet in CC.

@nylen
Copy link
Contributor

nylen commented Aug 13, 2021

So it sort of baffles me it's not working now raised_eyebrow

WP 5.5 made a change which broke this behavior. WooCommerce 4.4.0 added a workaround.

@adegans
Copy link

adegans commented Aug 13, 2021

So it sort of baffles me it's not working now raised_eyebrow

WP 5.5 made a change which broke this behavior. WooCommerce 4.4.0 added a workaround.

But I'm pretty sure I started out on WC 3.x and I never noticed it broken 🥳

Unfortunately may not be a straightforward backport as it looks like Woo has made some other changes that are pre-requisites for including this fix, but we don't have those pre-requisite changes yet in CC.

Perhaps the fixes, or functions, or whatever that make this work could be included in the "Fake WooCommerce" plugin so only those who need it get it. (Or something along those lines) Since you may want to avoid bloat in the main project.

@ghost
Copy link

ghost commented Sep 1, 2021

I feel like we'd just end up chasing out tail on this. What happens if CC stops working with the next version of WP? Do we then have to look at fixing that issue as well, and adding in any other backports that may be needed for that fix?

If we are planning to always try to keep CC working with WP then I guess we should look at this. But if we accept the fact that CC is a CP-specific plugin then we shouldn't bother. Especially given this...

may not be a straightforward backport as it looks like Woo has made some other changes that are pre-requisites for including this fix

@adegans
Copy link

adegans commented Sep 2, 2021

@simplycomputing if you decide to fix bugs based on 'what ifs', then nothing ever gets done.

What I find stranger/more disturbing is that nobody seems to consider the WordPress side of things.
Imagine someone who likes WooCommerce but doesn't want the modern stupid WooCommerce... ClassicCommerce is then the answer, but apparently growing both the ClassicPress and ClassicCommerce platforms are a one sided deal with little focus other than "MUST GET AWAY FROM WORDPRESS". To some extent that's fine, but it will hinder growth and attractiveness to the masses immensely.

@ghost
Copy link

ghost commented Sep 2, 2021

if you decide to fix bugs based on 'what ifs', then nothing ever gets done.

Well, it's more based on "who do we have?". If it was a simple one-file fix (like the other PR I did) then I can muddle through that myself and test it. But when James said there were other related changes to unpick before getting this fix to work, then I am lost.

I would personally like to see CC continue working with WP, but I just don't know who is going to make that happen.

@ghost
Copy link

ghost commented Sep 2, 2021

I had a look at the WooCommerce PR for this fix. It involves editing 3 files.

  • assets/css/admin.scss - we have admin.css so I guess we would need to work out what would be the matching change in our version of this file (?)
  • assets/js/admin/meta-boxes-product.js - we do have this file and it looks like the change could be added (?)
  • includes/admin/class-wc-admin.php - the change here is to a function we don't even have: include_admin_body_class. This was introduced in WC4.2.0. I have no idea how hard it would be to introduce this into CC.

@adegans
Copy link

adegans commented Sep 2, 2021

@simplycomputing Thanks for trying.
I find the WC code highly confusing and overly complex. So I'm not sure if I would be able to fix things.

However, seeing that the other checkboxes that go there, work. And in CC when used in ClassicPress it works. I don't think it's all that hard to extent/edit that code to have the missing checkboxes show up in WordPress as well. Since the code exists and 'sort of' works.
If the checkboxes work in the background (eg flip the switch in the database and see what happens), then it should be a simple CSS fix.
Regardless of the file, the code is somewhere in the older versions since it works when used in ClassicPress.

1-on-1 copying their fix is of-course most convenient/easy. But we're not always that lucky, hah.

@ghost
Copy link

ghost commented Sep 2, 2021

@adegans

1-on-1 copying their fix is of-course most convenient/easy.

Yes, that's all I know how to do. :-) But I tried this last night and may have made some progress. I got this far:
cc-product

Need to get some more eyes on this so I will do a PR and see what others think.

@adegans
Copy link

adegans commented Sep 2, 2021

@simplycomputing I'm happy to help test things, also other bugs and stuff like that.

Unfortunately, currently, I do not have time to actually sift through WC code for the most part.
Perhaps that'll change in the future, depending on how things develop in my life 🥳

So, if you need help testing let me know. And perhaps it's better to do this kind of thing via email or chat.
Then post the important bits and results on Github. Thoughts?

@ghost
Copy link

ghost commented Sep 2, 2021

I have just submitted a PR for this. It has some questions and needs more work, but it's a start. :-)

@ghost ghost added enhancement New feature or request Has PR Issue has a PR. labels Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Has PR Issue has a PR.
Projects
None yet
Development

No branches or pull requests

4 participants