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

Allow extensions in Add Products task #44892

Merged
merged 14 commits into from Mar 7, 2024
Merged

Conversation

dakota
Copy link
Contributor

@dakota dakota commented Feb 22, 2024

Submission Review Guidelines:

Changes proposed in this Pull Request:

As part of the project to allow canonical WooCommerce extensions to surface themselves as part of the WooCommerce onboarding tasklist. This PR adds a filter to allow for such extensions to add themselves to the task-list.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

  1. Install this WooCommerce Gift-Cards pre-release version
  2. On the Product task list (/wp-admin/admin.php?page=wc-admin&task=products) you should see "Gift Card" as an option at the end of the list (hidden under the "view more" link)
  3. Clicking it should create a new gift card product (This step isn't directly being tested as the primary work involved is in the gift cards plugin)

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Add support for extending product types on onboarding task list

Comment

@dakota dakota self-assigned this Feb 22, 2024
Copy link
Contributor

github-actions bot commented Feb 22, 2024

Test Results Summary

Commit SHA: e3a911e

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 38s
E2E Tests2330011803516m 56s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

Copy link
Member

@xristos3490 xristos3490 left a comment

Choose a reason for hiding this comment

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

Looks good to me, @dakota! Let's polish it so @ilyasfoo can take a final look as well before I approve. Also left a minor comment.

The only thing that stands out to me is that this filter will allow complete replacements of that list; which can be a nice-to-have feature for heavily customized stores.

@@ -0,0 +1,25 @@
/**
Copy link
Member

Choose a reason for hiding this comment

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

Is this file needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not. It was created by @ilyasfoo as part of the initial POC code. I thought of leaving it in as a example, but can remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, @dakota! I'd recommend removing this file for review.

For a more realistic testing, appreciate if you could provide the GC plugin build that utilizes the filter. Thanks!

Copy link
Contributor

github-actions bot commented Feb 26, 2024

Hi @xristos3490, @ilyasfoo,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Feb 28, 2024
@dakota dakota added focus: setup checklist Issue related to onboarding task list. team: SomewhereWarm labels Feb 28, 2024
@dakota
Copy link
Contributor Author

dakota commented Feb 28, 2024

The only thing that stands out to me is that this filter will allow complete replacements of that list; which can be a nice-to-have feature for heavily customized stores.

@xristos3490 - I've made a change to prevent replacing the list as per the project requirements.

@dakota dakota marked this pull request as ready for review February 28, 2024 16:04
@dakota
Copy link
Contributor Author

dakota commented Feb 28, 2024

@ilyasfoo @xristos3490 I believe this PR is now ready for CR and testing

@dakota dakota changed the title [DRAFT] Allow extensions in Add Products task Allow extensions in Add Products task Feb 29, 2024
Copy link
Contributor

@ilyasfoo ilyasfoo left a comment

Choose a reason for hiding this comment

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

Testing well! However, I have few comments:

dakota and others added 2 commits March 1, 2024 13:11
Co-authored-by: Ilyas Foo <foo.ilyas@gmail.com>
@dakota dakota requested a review from ilyasfoo March 4, 2024 09:35
Copy link
Contributor

@ilyasfoo ilyasfoo left a comment

Choose a reason for hiding this comment

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

@dakota LGTM but please remember to update the filter name in GC plugin!

@dakota dakota merged commit c509f42 into trunk Mar 7, 2024
32 checks passed
@dakota dakota deleted the dakota/product-types-extensibility branch March 7, 2024 10:04
@github-actions github-actions bot added this to the 8.8.0 milestone Mar 7, 2024
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Mar 7, 2024
@nigeljamesstevenson nigeljamesstevenson added needs: internal testing Indicates if the PR requires further testing conducted by Solaris status: analysis complete Indicates if a PR has been analysed by Solaris release: highlight Issues that have a high user impact and need to be discussed/paid attention to. and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Mar 8, 2024
Konamiman pushed a commit that referenced this pull request Mar 13, 2024
* Add POC for tasklist product types extensibility

* Replace create product with custom onClick when it exists

* Don't include dummy plugin

* Remove dummy file

* Cleanup product type filter for tasklist

* Create unit test for product type filter

* Added changelog

* Move changelog file

* Is Enhancement

* Add changefile(s) from automation for the following project(s): woocommerce

* Delete plugins/woocommerce/changelog/extend-product-task

* Fix lint errors

* Apply suggestions from code review

Co-authored-by: Ilyas Foo <foo.ilyas@gmail.com>

* CR responses

---------

Co-authored-by: Ilyas Foo <foo.ilyas@gmail.com>
Co-authored-by: github-actions <github-actions@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: setup checklist Issue related to onboarding task list. needs: internal testing Indicates if the PR requires further testing conducted by Solaris plugin: woocommerce Issues related to the WooCommerce Core plugin. release: highlight Issues that have a high user impact and need to be discussed/paid attention to. status: analysis complete Indicates if a PR has been analysed by Solaris team: SomewhereWarm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants