Skip to content

Conversation

@barryhughes
Copy link
Member

@barryhughes barryhughes commented May 3, 2022

Adds a new list table implementation that works with COT, and otherwise strives to emulate the existing post-based admin list table.

cot-admin-list-table

  • Per the linked issue, we are not implementing all of the functionality in this PR (bulk actions, filters etc will not work).
  • We're aiming to get close to the look and feel of the existing posts based table, but the result may not be identical and that's ok.
  • In the above screenshot you may notice two WooCommerce > Orders menu entries: that's because, for testing purposes (remembering the new data-store is not 100% operational just yet):
    • I configured the WP Posts store as authoritative (therefore, the original admin list table and its menu entry remain accessible).
    • I temporarily tweaked this line of code by adding true || to the start of the condition, to make the new admin list table render anyway.
    • With a few exceptions, the new implementation mostly doesn't care which data store is authoritative (both implement the same interface), which is why this approach essentially works.

Closes #32385.

Testing

  1. For completeness, you may wish to follow the steps in the linked issue to enable and hydrate the new order table (though this probably isn't strictly required, since at time of writing we can't read from the new store in any case—however, that will change in the future).
  2. Per the above notes, temporarily tweak the code (inside the WC_Admin_Menus::orders_menu() method) as suggested and set the WP Posts store as default via WooCommerce > Settings > Advanced > Custom Data Stores.
  3. Navigate to the new admin list table (look for admin.php?page=wc-orders in the URL).
  4. Compare and contrast with the original post-based admin list table, remembering per the above notes that not everything is fully functional just yet.
  5. Check the no-results page. There are two ways to see this:
    • At time of writing, you can do this by setting COT to authoritative and removing the temporary shim from WC_Admin_Menus::orders_menu().
    • Or, keeping the shim in place (and keeping the post table as authoritative) you can instead simply hide your post table orders with a query as follows (to undo, just reverse the post type values):
wp db query "UPDATE $(wp db prefix)posts SET post_type = 'shop_order_hidden' WHERE post_type = 'shop_order'" ;\
wp cache flush

Notes/questions

  • You may need to rebuild assets and hard-refresh your browser for the relevant CSS+JS changes to show up.
  • We may (or may not) wish to expand e2e-core-tests to cover this area (I don't believe the existing list table is covered, either); but it probably makes sense to defer that until we're at a more advanced stage of the COT project.
  • Are we happy with the placement inside the Admin\Internal namespace (sidenote: its location is the reason an automated action applies the focus: react admin label) or might we prefer to place it in some other namespace?
  • Admin UI for the order editor is not yet implemented (so following the link to add a new order will fail when COT is authoritative).
  • Normally, if some orders are pending, a number will display next to the Orders menu entry, as in the following screenshot (something similar happens with the new WC Admin navigation feature) ... this will not currently work when COT is authoritative, because the necessary logic to perform order counts is not yet implemented.

orders-posts-needing-attention-bubble

@github-actions github-actions bot added focus: react admin plugin: woocommerce Issues related to the WooCommerce Core plugin. labels May 3, 2022
@barryhughes barryhughes marked this pull request as ready for review May 7, 2022 00:07
@barryhughes barryhughes requested review from a team and Konamiman and removed request for a team May 7, 2022 00:07
@Konamiman
Copy link
Contributor

I'm not sure if I'm testing correctly / if the behavior I'm seeing is expected. I have populated the orders table from the posts table using #32817 to start with. This is what I see:

If the new orders table is authoritative

There's no "Orders" menu in the WooCommerce menu, and I get the following notices:

PHP Notice:  Trying to access array offset on value of type null in /home/konamiman/woocommerce/plugins/woocommerce/src/Admin/Features/Navigation/CoreMenu.php on line 216
PHP Notice:  Trying to access array offset on value of type null in /home/konamiman/woocommerce/plugins/woocommerce/src/Admin/Features/Navigation/CoreMenu.php on line 217
You have attempted to register a duplicate item with WooCommerce Navigation: `orders`
PHP Notice:  Undefined index: woocommerce-orders in /home/konamiman/woocommerce/plugins/woocommerce/src/Admin/Features/Navigation/CoreMenu.php on line 365
PHP Notice:  Trying to access array offset on value of type null in /home/konamiman/woocommerce/plugins/woocommerce/src/Admin/Features/Navigation/CoreMenu.php on line 365

If I try and go to /admin.php?page=wc-orders manually the page loads but the list appears empty (it says that there are no orders, "When you receive a new order, it will appear here").

If the posts table is authoritative

This time the "Orders" menue entry is there and it redirects to /wp-admin/edit.php?post_type=shop_order as expected. If I go to /admin.php?page=wc-orders manually I get the following error:

Fatal error: Uncaught TypeError: Argument 1 passed to Automattic\WooCommerce\Internal\Admin\Orders\ListTable::column_order_number() must be an instance of WC_Order, instance of Automattic\WooCommerce\Admin\Overrides\OrderRefund given, called in /home/konamiman/wordpress/wp-admin/includes/class-wp-list-table.php on line 1445 and defined in /home/konamiman/woocommerce/plugins/woocommerce/src/Internal/Admin/Orders/ListTable.php:406
Stack trace:
#0 /home/konamiman/wordpress/wp-admin/includes/class-wp-list-table.php(1445): Automattic\WooCommerce\Internal\Admin\Orders\ListTable->column_order_number()
#1 /home/konamiman/wordpress/wp-admin/includes/class-wp-list-table.php(1390): WP_List_Table->single_row_columns()
#2 /home/konamiman/wordpress/wp-admin/includes/class-wp-list-table.php(1377): WP_List_Table->single_row()
#3 /home/konamiman/wordpress/wp-admin/includes/class-wp-list-table.php(1362): WP_List_Table->display_rows()
#4 /home/konamiman/wordpress/wp-admin/includes/class-wp-list-table.php(1289): WP_List_Table->display_rows_o in /home/konamiman/woocommerce/plugins/woocommerce/src/Internal/Admin/Orders/ListTable.php on line 406

Screenshot of how I see that error:

image

@barryhughes
Copy link
Member Author

barryhughes commented May 9, 2022

If the new orders table is authoritative
There's no "Orders" menu in the WooCommerce menu, and I get the following notices

Yes, I didn't have the new admin navigation feature (ie, via WooCommerce ▸ Settings ▸ Advanced ▸ Features) enabled (actually, I was blissfully unaware of this) but I'm assuming from the error you shared that you do. I guess we'll need to support both navigation systems.

If I try and go to /admin.php?page=wc-orders manually the page loads but the list appears empty

Yes, I'd expect it to be empty because the data store is not complete at this point. That's the reason the new view needs to be tested with the post table store.

@barryhughes barryhughes changed the title Admin list table [COT | Draft] Admin list table [COT] May 9, 2022
@barryhughes
Copy link
Member Author

barryhughes commented May 9, 2022

✍🏽 I've addressed the error in the last screenshot, but am still working on a few things relating to new nav experience and also an issue relating to when these settings are saved, versus when post types are registered/when menus are composed (will request a fresh review once done).

@barryhughes
Copy link
Member Author

barryhughes commented May 10, 2022

@Konamiman

You have attempted to register a duplicate item with WooCommerce Navigation: `orders`

If you're happy to, let's park this issue. There are a couple of ways to solve it but I wanted to get advice from Isotope, since they are the owners of the nav feature. Internal link: p1652220313065349-slack-CBKK5KM41

Edit: actually, unless I've just been looking at this for too long, which is possible, I'm not even sure it is specific to this PR. I seem able to replicate the same error with trunk.

@barryhughes barryhughes requested review from Konamiman and removed request for Konamiman May 12, 2022 15:27
@barryhughes barryhughes force-pushed the add/32385-cot-admin-list-table branch from 3537ff2 to c9ebffd Compare May 20, 2022 13:53
@barryhughes
Copy link
Member Author

@Konamiman — this is updated (rebased on latest trunk) and should be ready for another look.

Copy link
Contributor

@Konamiman Konamiman left a comment

Choose a reason for hiding this comment

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

@barryhughes Sorry, you wrote in your comment "let's park this issue" and I interpreted it as referring to this pr itself 😓

Looks good and works as expected, the only differences I can notice with the posts based list is that the preview buttons don't work and the list isn't sortable, but I understand that's expected at this point. So, approving.

@Konamiman Konamiman merged commit 484e790 into trunk May 23, 2022
@Konamiman Konamiman deleted the add/32385-cot-admin-list-table branch May 23, 2022 10:05
@github-actions github-actions bot added this to the 6.7.0 milestone May 23, 2022
@github-actions
Copy link
Contributor

Hi @Konamiman, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the release: add testing instructions label

@barryhughes
Copy link
Member Author

Yep, sorry, I should have been clearer (+thanks for reviewing).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugin: woocommerce Issues related to the WooCommerce Core plugin.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[COT] Implement order list admin screen using new tables.

4 participants