woocommerce / woocommerce-admin

(Deprecated) This plugin has been merged to woocommerce/woocommerce
https://woocommerce.github.io/woocommerce-admin/#/
Other
361 stars 144 forks source link

[WC 4.9 RC2] If the Home Screen tasks were not completed prior to WC update, the task list disappears after the update #6045

Closed juliaamosova closed 3 years ago

juliaamosova commented 3 years ago

Describe the bug

If the Home Screen tasks were not completed prior to the WC update, the task list disappears after the update. I was able to reproduce this on the JN site while testing the following upgrade paths:

To Reproduce Steps to reproduce the behavior:

  1. Create a brand new JN site.
  2. Upload WC 4.7.1 or 4.8 on it.
  3. Upon OBW starting, skip it.
  4. Don't complete any tasks on the Home Screen but note that Task List is there.
  5. Install and activate the WooCommerce Beta Tester plugin.
  6. Switch WC to 4.9 using the WooCommerce Beta Tester plugin.
  7. Note that the Task List is no longer present on the Home Screen even though it was not completed prior to the update to 4.9.

Expected behavior

I expected the Task List to remain present on the Home Screen after updating from either 4.7.1 or 4.8 to 4.9 if the Task List was not completed prior to the update.

Screenshots

Gif showing the issue when updating from 4.8 to 4.9:

More detailed screen recording when updating from 4.7.1 to 4.9:

https://cloudup.com/cCgYKFjZMpL

Desktop (please complete the following information):

Additional context

juliaamosova commented 3 years ago

Note that @timmyc and I chatted in p1610383764342800-slack-C01DT6U03HC and decided that this is not a blocked for WC 4.9 release.

louwie17 commented 3 years ago

Thanks @juliaamosova I was able to reproduce it as well, a screenshot here of the option being set after update:

Screen Shot 2021-01-12 at 1 30 00 PM

@timmyc was correct and there is an action we are running on woocommerce_updated see: https://github.com/woocommerce/woocommerce-admin/blob/main/src/Features/Onboarding.php#L1149-L1164 Done as part of this ticket: https://github.com/woocommerce/woocommerce-admin/pull/3590

This logic is only run if we skip OBW, meaning it's not completed: https://github.com/woocommerce/woocommerce-admin/blob/main/src/Features/Onboarding.php#L1157

Tested this as well, completing OBW with the rest of the task list as uncompleted and then updating Woo keeps the task list displayed.

louwie17 commented 3 years ago

Looping in @joshuatf as the original ticket was created by him, I believe checking if OBW was skipped along with completed should fix this, and keep inline with the original reasoning for the implementation.

|| (isset( $onboarding_data['skipped'] ) && $onboarding_data['skipped'])

joshuatf commented 3 years ago

The reason behind the logic in https://github.com/woocommerce/woocommerce-admin/blob/main/src/Features/Onboarding.php#L1149-L1164 is for older versions of WC with the old OBW. This logic prevents store owners from having to re-complete the (new) OBW.

We could simply remove the line update_option( 'woocommerce_task_list_hidden', 'yes' ); to fix the issue. I think this is okay, but worth noting that it's possible a more established store undergoing an update will see the task list even though the store has been operating for some time and has already completed the old OBW.

On a side note, it would be great if core passed both the previous version and current version of WC when doing the action woocommerce_updated.

gglobalstep commented 3 years ago

Replicates - #5967

timmyc commented 3 years ago

Thanks for the thoughts on this one @joshuatf - I'm a bit on the fence of how to proceed here. My thoughts:

elizaan36 commented 3 years ago

We could simply remove the line update_option( 'woocommerce_task_list_hidden', 'yes' ); to fix the issue. I think this is okay, but worth noting that it's possible a more established store undergoing an update will see the task list even though the store has been operating for some time and has already completed the old OBW.

@joshuatf I'm curious how many people could be impacted by this. If it's a significant number of established stores suddenly seeing the new task list, I agree it's not ideal. @timmyc I like the logic you proposed as a solution.

Totally alternate approach: should we consider adding something to the home screen display menu that allows the setup checklist to be toggled back on if off ( cc @elizaan36 )

I remember chatting with @pmcpinto about this, but the problem is that once you're an established store, you may find it irrelevant or a nuisance in this location. It should be more discoverable than it currently is (Settings > General > Help accordion > Setup wizard), but I think including it in the Home screen display menu isn't necessary. I could see it being more useful to access from the general settings area maybe.

pmcpinto commented 3 years ago

Could we circumvent that possibility by also leaning on logic to see how "old" the wc-admin install is? Perhaps we only tweak that option if the site is less than 20 days old or something?

This path seems to be the best approach.

joshuatf commented 3 years ago

Could we circumvent that possibility by also leaning on logic to see how "old" the wc-admin install is? Perhaps we only tweak that option if the site is less than 20 days old or something?

I think this is a good idea. Maybe we could simplify this even further:

If a site has any WCA Install timestamp, don’t hide the task list since were upgrading from a version that included WCA.

We’ll need to make sure that timestamp doesn’t get created prior to this process but if it does, then we can use <1 day as a benchmark to see if they were upgrading from a very old version of WC.

Just a note that this would hide the list if a user installed an old version of WC and immediately upgraded to a new version, but I think this is an edge case.

Dev note: we should move the WCA timestamp checker to a utility file or class and deprecate the one used in Note.

github-actions[bot] commented 3 years ago

This issue 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[bot] commented 3 years ago

This issue was automatically closed due to being stale. Please feel free to re-open it if you still experience this problem.