Since reverting the PR at this point would be a mess I've gone ahead and removed the internals of the container. We should aim to keep the class since it's now part of our public API but it won't work as expected anymore. This is fine for now since we don't actually have anything in it!
1. Since our code style dictates that these be `final` methods, we shouldn't be concerned about overlap.
2. There is precedent for `init` methods as requirements before using class instances.
Since we need to maintain backwards compatibility for class constructors we should settle on using method injection instead of constructor injection. I've replaced the `Definition` class we're using with one that doesn't support constructor arguments and added a check for auto_arg addition. Note that we don't check for method existence in the extended container. This is because reflection is unnecessarily expensive and we should avoid it if at all possible.
This bug was introduced in #26260. The sequence is:
1. WC_Query::adjust_posts_count runs, to handle found_posts filter,
this indirectly executes wc_setup_loop.
2. At this point $GLOBALS['wp_query']->max_num_pages hasn't been set
yet, and has a value of 0. Thus the loop variable total_pages
is set to 0.
3. Later wc_setup_loop runs again and this time
$GLOBALS['wp_query']->max_num_pages is already set, but since
the loop variable total_pages already exists, it keeps its
value of 0.
4. The pagination controls never show if total_pages is less than 2.
The fix consists of hooking into the_posts to set the value of
total_pages again, at that point $GLOBALS['wp_query']->max_num_pages
is already set.
PR #26260 introduced a handler for 'found_posts' filter in WC_Query
class in order to adjust the count depending on the visibility
of variation products. However the handler incorrectly assumed
that the filter was triggered only when listing products, when
actually it's also triggered for any post type e.g. pages.
In these cases the post count was set to zero, which caused bugs.
Now the handler starts with the originally supplied posts count,
and only decrements it when a post is a product AND is not visible.
Now, if there are filters present the logic is as follows:
- For multiple filtering values of the same attribute:
the product is visible if there's at least one variation
that has one of the filtering values associated to the attribute,
or if there's at least one variation having the attribute
with a value of "Any".
- For filtering by more than one attribute:
the product is visible if there's at least one variation that
is visible for ALL the attributes according to the above rule.
Note that this is irrespective of the type of logic configured for
the filter (OR or AND).
Two adjustments were needed:
- Adjust the count even when there's no nav filtering in the query.
This is necessary to present the proper products count.
even when the woocommerce_product_is_visible filter is used.
- Account for the case where $GLOBALS['wp_query']->posts
returns objects instead of ids (for example when viewing
a product page).
After the change that registers variation attributes as terms
(in addition to reigstering them as post meta) it is now time
to modify the get_filtered_term_product_counts methods in
WC_Widget_Layered_Nav so that it works consistently for both
variable and non-variable products. The logic for the counters
is now as follows:
with OR operator:
- Simple products: count the attributes of all visible products
(unchanged behavior).
- Variable products: count attributes corresponding to
visible variations.
with AND operator:
- Simple products: count the attributes of visible products but only
for products that have all the selected (unchanged behavior).
- Variable products: find all the products for which all the variations
corresponding to the selected attributes exist and are visible,
then count the attributes corresponding to the visible variations
of those products.
A product is "visible" if it's published, not excluded for catalog,
and has stock. Additionally, a variable product will not be considered
visible if the parent product is not.
Product attributes are currently recorded as terms in
wp_term_relationships (product attributes are actually taxonomies).
In the case of variable products this is true for the main product,
but not for the variations. The attributes used to define variations
are stored as post meta, but nothing is recorded in the term
relationships table.
This is a problem when using the layered nav filtering plugin,
since the attribute counters displayed are calculated based solely
on the contents of the term relationships table. Adding meta queries
would be really messy (especially when the widget is configured
with AND operator) and would probably also hurt performance.
This commit adds a change to store the attributes for variations
as term relationships, additionally to storing them as post meta.
Terms are stored on variation creation, and updated/deleted together
with the variation as appropriate. "Any" variations (stored in meta
as empty values) are not stored as terms.
Additionally, a database upgrade is included in order to backfill
terms for already existing products.
The layered nav filtering doesn't work well with variable products
when some variations have stock and other don't. When a term is
selected in the widget, a variable product having no stock for
the variation corresponding to that term but having stock for
other variations will be displayed, but it shouldn't.
This commit fixes that by introducing two changes:
- A new override of "is_visible" for WC_Product_Variable that
looks at the supplied filters, compares them against the corresponding
available variations and calculates the visibility based on
the query type (OR or AND).
- A hook on the "found_posts" filter in WC_Query, that adjusts
the posts count based on the found products visibility
when there are filters available; this is needed to sync the
"displaying X posts" messages and the paging when variable
products are hidden due to stock status.
Additionally, the visibility calculated in "found_posts" is cached
as loop variables so that it isn't calculated again when actually
displaying the products.
It is possible for a later duplicate webhook to be fired too early if
the same webhook triggers in one request more than once with the updated
changes from the second one missing if it happens too quickly.
This queues all webhook to be register on shutdown instead of just
syncronous ones to make sure all data from the request is updated first
before the webhook gets queued.
test_static_mocks_can_be_used_via_injected_legacy_proxy_and_woocommerce_object
was failing in PHP 7.0, this replacement fixes it.
Suspected cause: https://bugs.php.net/bug.php?id=76505
- src/README.md largely expanded
- tests/README.md expanded
- includes/README.md added
- src/Internal/REAMDE.md added
src/README.md and includes/README.md have TODO placeholders to add
guidelines regarding to actions and filters.
If a class name is passed as a concrete, check that the class
constructor is public if it exists. If another type of concrete is
passed, check that it's valid (a callback or an object).
Also update the autoloader to check if the class file exists,
otherwise class_exists fails if a namespaced class doesn't exist.
The following methods are added and can be invoked using `WC()`,
they just redirect to the same methods in LegacyProxy:
call_function
call_static
get_instance_of
Since we need to load all of these files before WooCommerce has initialized we can't rely on Composer to handle the autoloading. We should take this namespace out of Composer altogether and just have our test autoloader take care of it.
- Method and class renames.
- Removed unnecessary autoloader registration.
- Add a unit test for classes with non-object type hints
in constructor arguments.
- `get_instance_of` accepts now arguments to be passed to the
class constructor if necessary.
- `get_special_instance_of` method removed, instead, now if a method
named `get_instance_of_<lowercased class name>` exists in the class,
it is used to get the instance of the class.
- A couple more unit tests added.
Also:
- Make the methods in `AbstractServiceProvider` protected.
- Add an autoloader for files in the `tests/php/src` directory.
- Fix a bug in the provisional (?) autoloader.
- camelCase methods changed to snake_case for consistency with WP.
- Added a check in `ExtendedContainer::get` that throws an informative
exception if a non-namespaced class name is passed.
- `container->reset_resolved()` is called during unit testing bootstrap.
- Added some utility methods in `WC_Unit_Test_Case`.
- Added a new class `ExtendedContainer` that extends League's container.
- `add` modified to reject classes not in the root Woo namespace.
- Has two new methods, `replace` and `reset_resolved`.
- It's used as the underlying container instead of League's one
in `Container`, but the new methods are not exposed.
- At unit test bootstrap time the globally registered container is
replaced with the extended one that `Container` stores
(grabbed from private property using reflection).
- A new `MockableLegacyProxy` is added. It inherits from `LegacyProxy`
and allows to mock functions, static methods and legacy classes.
- The registeed `LegacyProxy` is replaced with the mockable version
during unit test bootstrap.
- A PHPUnit hook is added to reset the mockable proxy to its initial
state (so that nothing is mocked) before each test.
- `WC_Unit_Test_Case` gets helper methods to mock functions, static
methods and classes without having to retrieve the proxy class.
At some point the 'change_stock' key is assumed to be present
in the request data, but it might not. Fixed to test for existence
before using the value.
The test added checks that stock status of variations when saving
a variable product is changed or not appropriately depending on
the request data supplied.
Those methods are a convenient replacement for
"this->factory->user->create". Tests that were using that to
simulate user login have been modified to use the new methods.
Some of our endpoints don't have an "<id>" parameter but we're expecting one in the CLI. Since the `id` is already part of the supported IDs we don't actually need this since it will pull it from the route.
In practice having the response class contained within the error feels a little backwards. We can instead have a structured APIError model that is contained in the APIResponse and have a consistent response format to consume.
When a product is saved its validate_props method is invoked,
and this recalculates the stock_status property based on whether
the product manages stock or not, the stock quantity, and the
value of the woocommerce_notify_no_stock_amount option.
In the case of variable products, and when stock is managed, the stock
was set to "instock" when the current stock was enough, but only
if the "stock_quantity" property was in the list of changed properties
for the object (the method in the base product class doen't check
for changed properties). This is a problem because the
wc_update_product_stock function updates stock_quantity but via direct
database modification, and thus stock_quantity isn't considered
modified. Therefore stock modifications via wc_update_product_stock
don't update stock_status on the product (e.g. when going from 0 to 1
after a refund the stock status will remain as "outofstock").
The fix consists of removing the check for changed properties since
it's not done anyway in the other cases (when stock is below the
woocommerce_notify_no_stock_amount threshold) nor in the base class.
Also, validate_props is refactored for readabiliyy, and an useless
set_stock_status() call placed right before save()
in wc_update_product_stock is removed.
One of the problems with synchronous webhooks is that they are executed as soon as the related action is. Since we may call an action multiple times in the process of updating something, this causes only the first action to trigger the hook. This differs from asynchronous execution because in that case, the web hook will be executed after the entire request has completed.
We've hid the use of Axios behind a service so that we're able to easily mock it out in factories as well as handle the creation and configuration of the client. This will make it easier on consumers in that they won't have to worry about things like authentication when using the API.
The mapping of the "Automattic\WooCommerce\Testing\Tools\" namespace
to the "tests/tools" directory is moved from manual registration
inside the tests bootstrap constructor to a declaration inside the
autload-dev section in composer.json.
The code hacker as originally designed, as a mechanism that allowed
to enable hacks at the individual test level, is flawed because it
assumes that code files are loaded before each test, but actually
the PHP engine loads code files only once.
Therefore this commit redesigns it so that the two existing main hacks,
the functions mocker and the static methods hacker, are applied
to all the relevant functions and classes at bootstrap time, and
mocks for each individual function/method can be registered at the
beginning of each test. See README for the full details.
Right now, when a product having a parent (e.g. a variation having a
parent variable product) is saved, wc_deferred_product_sync is
executed so that product sync is performed at the end of the request.
This commit implements the same when the product is deleted.
The testing tools (only the code hacker at this time) have been moved
from 'src' to 'tests/Tools', since many opcode cache plugins
load the whole src folder in production.
Also, an extra autoloader is set in the tests bootstrap so that
the 'tests/Tools' directory corresponds, using PSR4, to the
'Automattic\WooCommerce\Testing\Tools' namespace.
- Add methods to temporarily disable and reenable the code hacker.
The code hacker is causing issues in some tests that perform
write operations to the local filesystem. Since this happens only
in a few cases, the easiest fix is to temporarily disable the
code hacker when that happens. This commit adds two new methods
for that in `WC_Unit_Test_Case`: `disable_code_hacker` and
`reenable_code_hacker`.
These methods use a disabling requests count so that the hacker
isn't enabled before it should. E.g. you call `disable`, then
a helper method that does `disable` and `enable`, then `enable` -
then only the last `enable` will have effect.
- `CodeHacker::add_hack` has now a boolean `persistent` parameter.
Persistent hacks won't be cleared by `clear_hacks`.
- `CodeHackerTestHook::executeAfterTest` will now disable the hacker
only if no persistent hacks are registered.
- The existing `file_copy` method is made static for consistency.
- `CodeHacker::restore` method renamed to `disable` for clarity.
The unit testing bootstrap loads and initializes WooCommerce, this
loads a bunch of code files that can't then be hacked in the test hooks.
A workaround is provided in this commit for the case of hacking
static methods. A new StaticWrapper class is created that allows
defining mock methods after the code file has been loaded.
This is applied to all classes from a fixed list in the bootstrap,
before WooCommerce is initialized. The list should be kept up to date
with the list of classes that require such workaround.
- Fix how CodeHackerTestHook::executeBeforeTest parses the test name,
to account for warnings and tests with data sets.
- CodeHackerTestHook now includes a executeAfterTest hook that
disables the code hacker (needed to prevent it from inadvertently
altering further tests). Also, clear_hacks is executed in
executeBeforeTest for the same reason.
- CodeHacker gets restore, clear_hacks and is_enabled methods
to support the changes in CodeHackerTestHook.
- FunctionsMockerHack fixed so that it doesn't modify strings
that are class method definitions.
- Added the WC_Unit_Test_Case::file_copy method, it must be used
instead of the PHP built-in "copy" in tests, otherwise tests
that run with the code hacker active will fail.
This is something to investigate.
Now @hack class and method annotations can be used to register
code hacks as an alternative to using before_ methods.
The syntax is /* @hack HackClassName param1 param2 */
where parameters will be passed to the class constructor.
If the class name ends with "Hack", then that suffix can be
omitted (e.g. "Foo" can be specified instead of "FooHack").
The "code hacker" is a class that hooks on filesystem events
(using stream_wrapper_unregister) in order to allow for dynamically
modifying the content of PHP code files while they are loaded.
The code hacker class allows registering hacks, which are
functions that take source code as input and return the modified code.
A hack can be a standalone function or a class with a "hack" method.
A few hacks are provided off the shelf. One allows mocking standalone
PHP functions (WP, WOO or not), another one allows mocking static
methods, and there's the one that removes the "final" qualifier
from a class definition. This helps unit testing stuff that would
otherwise be quite hard to test.
Since we were converting the field to lowercase we ended up inserting meta in all lowercase, regardless of what it was in the CSV file. We should only be using the normalized field name when looking at the default columns, and should instead rely on a case-insensitive regex for the special columns.
One thing to note is that we're still defaulting the $headers array to the normalized field, as we don't want to change what is being passed to the filter for unmapped columns.
Since those Notes were created because of WC Admin and the display is handled by WC Admin, it does not make sense to test them without WC Admin.
In addition, the data store that handles these Notices is not loaded without WC Admin.
All titles and questions in the new onboarding wizards only capitalise names and the first letter of the sentence. This seemed a tiny bit off. (Literally tiny.)
Changed "Build a Better WooCommerce" to "Build a better WooCommerce"
* Merge wc api authorization headers with given headers
* Add put method to WC_Helper_API
* Add unit test coverage around WC_Helper_API request methods
* Add tests for WC_Helper_API url method
Since the tokens are replaced in a first-discovered first-replaced order, we may accidentally create tokenized paths like '{{ABSPATH}}/test' instead of the desired '{{WP_CONTENT_DIR}}test'. By ordering them according to specificity however, we ensure that we tokenize as much of the path as possible.
Initially it was made more precise to avoid calculating estimates for incomplete addresses. Due to some bad working in template files though, as well as historical precedent, we need to change it back.
Instead of comparing all tables that exist with all tables that are registered with `WC_Install::get_tables()` (which was only introduced a few versions ago in #19436), rewrite the test to verify that all of the tables *registered* actually exist within the database.
This will prevent tests from failing when, for example, they're run against databases that may or may not have additional plugins installed/activated. This also prevents tests from failing when running WooCommerce 4.x (e.g. with WooCommerce Admin) on WordPress < 5.3, since the tables are created but the callback to register the custom tables is never hooked.
The WooCommerce unit tests started failing when running using WP nightly
with the following error:
```
Fatal error: require_once(): Failed opening required '/tmp/wordpress-tests-lib/src//wp-includes/class-phpmailer.php' (include_path='.:/home/travis/.phpenv/versions/7.4.2/share/pear') in /tmp/wordpress-tests-lib/includes/mock-mailer.php on line 2
```
https://travis-ci.org/woocommerce/woocommerce/jobs/646855363#L1955
This was happening because WP changed the syntax of the line where
ABSPATH is defined in the context of the unit tests
(https://core.trac.wordpress.org/changeset/47198/trunk/wp-tests-config-sample.php)
and we rely on this syntax to change ABSPATH when running
`tests/bin/install.sh`.
To fix this problem, this commit updates the sed command used to change
ABSPATH to work when both the old and new syntaxes are used. In the
future we might want to consider a more robust solution to the problem
of updating ABSPATH.
For some reason unknown to me, the following commit added logic to make
sure that negative fees never make the order total negative: 804feb9333
Probably by accident, commit 4326e39250 changed this logic when `$fees_total += $item->get_total()` was replaced by `$fees_total += $fee_total`:
4326e39250 (diff-9b4164165828b26c4b7aec01c7b17884R1594)
This commit adds a failing test that ensures that the old behavior is
tested. This test needs to be fixed before we merge the changes proposed
in PR #25504.
Note: The tests fail in this commit because of the defines. Ran individually there are no problems, but I'm going to make another PR to merge in a package to make testing constants possible, and then come back to fix this after it gets merged.
This commit fixes the unit test
WC_Tests_API_Functions::test_wc_rest_upload_image_from_url_should_return_error_when_invalid_image_is_passed() which was failing due to a change in a WordPress core message with the following error:
```
2) WC_Tests_API_Functions::test_wc_rest_upload_image_from_url_should_return_error_when_invalid_image_is_passed
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'Invalid image: File is empty. Please upload something more substantial. This error could also be caused by uploads being disabled in your php.ini or by post_max_size being defined as smaller than upload_max_filesize in php.ini.'
+'Invalid image: File is empty. Please upload something more substantial. This error could also be caused by uploads being disabled in your php.ini file or by post_max_size being defined as smaller than upload_max_filesize in php.ini.'
/home/travis/build/woocommerce/woocommerce/tests/unit-tests/util/api-functions.php:89
```
(https://travis-ci.org/woocommerce/woocommerce/jobs/637353161#L402)
This test only fails when using WordPress nightly build (the yet to be
released WP 5.4).
To fix this problem, this commit adds an if statement to check for different error
messages depending on the WordPress version that is being used to run
the tests. In the future, we might want to refactor this test to remove
its dependency to a WordPress error message to make it more robust and
stable.
This replaces the corresponding geolocation class functionality. We are deprecating the woocommerce_geolocation_local_database_path filter in favor of the woocommerce_maxmind_geolocation_databse_path filter. Since we want the option to turn this into a feature plugin, it's wise to make sure nothing is too generic.
Shiki
Claudio Sanches
Ron Rennick
Tam Mullen
Christopher Allford
Néstor Soriano
Vedanshu Jain
dependabot[bot]
Jonathan Sadowski
Christopher Allford
Matt Harrison
Peter Fabian
Tam Mullen
Jeff Stieler
Mike Dent
Julia Amosova
Damir Ha
Boro Sitnikovski
Job
yasiendwieb
Joshua T Flowers
Nestor Soriano
Mike Jolley
Steve Grunwell
Jan-Paul Kleemans
Brent Sessions
Rodrigo Primo
Paul Sealock
Garrett Hyder