Fix "Out of stock threshold" when filed value is empty - Fix/issue 36960 #37855

bartech · 2023-04-19T18:35:52Z

Submission Review Guidelines:

I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have reviewed my code for security best practices.
Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.

Changes proposed in this Pull Request:

This PR

Cast "Out of stock threshold" field to absolute integer on save by using woocommerce_admin_settings_sanitize_option_woocommerce_notify_no_stock_amount filter.
Casts product stock quantity and no stock amount to absolute int before comparing.
Fix @param $notify_no_stock_amount type in tests

Closes #36960 .

How to test the changes in this Pull Request:

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

In WP Dashboard Go to WooCommerce Settings -> Products -> Inventory.
Remove the value of Out of stock threshold
Save settings
Field should have default value, that is 0.

NOTE:
We should consider do same casting on save for "Low stock threshold" field.
Or in general we should add new case for all number type fields and when empty set to default value.

woocommerce/plugins/woocommerce/includes/admin/class-wc-admin-settings.php

Lines 832 to 870 in 8ac3c29

    
           switch ( $option['type'] ) { 
        
           	case 'checkbox': 
        
           		$value = '1' === $raw_value || 'yes' === $raw_value ? 'yes' : 'no'; 
        
           		break; 
        
           	case 'textarea': 
        
           		$value = wp_kses_post( trim( $raw_value ) ); 
        
           		break; 
        
           	case 'multiselect': 
        
           	case 'multi_select_countries': 
        
           		$value = array_filter( array_map( 'wc_clean', (array) $raw_value ) ); 
        
           		break; 
        
           	case 'image_width': 
        
           		$value = array(); 
        
           		if ( isset( $raw_value['width'] ) ) { 
        
           			$value['width']  = wc_clean( $raw_value['width'] ); 
        
           			$value['height'] = wc_clean( $raw_value['height'] ); 
        
           			$value['crop']   = isset( $raw_value['crop'] ) ? 1 : 0; 
        
           		} else { 
        
           			$value['width']  = $option['default']['width']; 
        
           			$value['height'] = $option['default']['height']; 
        
           			$value['crop']   = $option['default']['crop']; 
        
           		} 
        
           		break; 
        
           	case 'select': 
        
           		$allowed_values = empty( $option['options'] ) ? array() : array_map( 'strval', array_keys( $option['options'] ) ); 
        
           		if ( empty( $option['default'] ) && empty( $allowed_values ) ) { 
        
           			$value = null; 
        
           			break; 
        
           		} 
        
           		$default = ( empty( $option['default'] ) ? $allowed_values[0] : $option['default'] ); 
        
           		$value   = in_array( $raw_value, $allowed_values, true ) ? $raw_value : $default; 
        
           		break; 
        
           	case 'relative_date_selector': 
        
           		$value = wc_parse_relative_date_option( $raw_value ); 
        
           		break; 
        
           	default: 
        
           		$value = wc_clean( $raw_value ); 
        
           		break; 
        
           }

codecov · 2023-04-19T19:15:03Z

Codecov Report

Merging #37855 (d32d086) into trunk (c5285ec) will increase coverage by 0.0%.
The diff coverage is 33.3%.

❗ Current head d32d086 differs from pull request most recent head 0da64a1. Consider uploading reports for the commit 0da64a1 to get more accurate results

Additional details and impacted files

@@           Coverage Diff            @@
##             trunk   #37855   +/-   ##
========================================
  Coverage     51.5%    51.5%           
+ Complexity   17283    17281    -2     
========================================
  Files          430      430           
  Lines        80037    80031    -6     
========================================
+ Hits         41216    41218    +2     
+ Misses       38821    38813    -8

Impacted Files	Coverage Δ
.../woocommerce/includes/admin/wc-admin-functions.php	`41.3% <0.0%> (-0.2%)`	⬇️
.../woocommerce/includes/class-wc-structured-data.php	`0.0% <0.0%> (ø)`
...ommerce/includes/abstracts/abstract-wc-product.php	`88.3% <100.0%> (ø)`
...udes/admin/settings/class-wc-settings-products.php	`98.6% <100.0%> (+<0.1%)`	⬆️
...ion2/class-wc-rest-order-refunds-v2-controller.php	`89.7% <100.0%> (ø)`
plugins/woocommerce/includes/wc-core-functions.php	`63.6% <100.0%> (ø)`

... and 1 file with indirect coverage changes

jorgeatorres

Hi @bartech,

Thank you for your contribution! This looks good to me. I just left a tiny little bit of feedback. Once that's addressed, we'll be good to merge! 💯

Thanks again!

jorgeatorres · 2023-04-19T22:57:27Z

plugins/woocommerce/includes/admin/settings/class-wc-settings-products.php

 		$this->label = __( 'Products', 'woocommerce' );

 		parent::__construct();
+		add_filter( 'woocommerce_admin_settings_sanitize_option_woocommerce_notify_no_stock_amount', 'absint' );


A while ago we added a class to handle this kind of "one off" sanitization, which might be better suited given we've been trying to modernize the codebase a bit. The class is called OptionSanitizer and you can find it in plugins/woocommerce/src/Internal/Settings/OptionSanitizer.php.

In the constructor over there, you can add this same add_filter().

@jorgeatorres Filter moved, please give it another look.

jorgeatorres

LGTM! 💯

Thank you again @bartech for your contribution.

bartech added 4 commits April 19, 2023 19:39

Cast stock qty and no stock amount to int before comparing

d2aaa39

Fix notify_no_stock_amount param type in tests

c4d1e55

Add changelog entry

2242840

Cast woocommerce_notify_no_stock_amount value to absint on save

dda5fe9

github-actions bot added plugin: woocommerce Issues related to the WooCommerce Core plugin. type: community contribution labels Apr 19, 2023

woocommercebot requested review from a team and jorgeatorres and removed request for a team April 19, 2023 18:36

Change cast from absint to int for product stock quantity

d32d086

bartech changed the title ~~Fix above notification threshold when "Out of stock threshold" filed value is empty - Fix/issue 36960~~ Fix "Out of stock threshold" when filed value is empty - Fix/issue 36960 Apr 19, 2023

laurendavissmith added the Contributor Day - H1 2023 label Apr 19, 2023

jorgeatorres requested changes Apr 19, 2023

View reviewed changes

Move filter to Option Sanitizer class

0da64a1

bartech requested a review from jorgeatorres April 20, 2023 09:13

jorgeatorres approved these changes Apr 25, 2023

View reviewed changes

jorgeatorres merged commit 226af50 into woocommerce:trunk Apr 25, 2023

github-actions bot added this to the 7.8.0 milestone Apr 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix "Out of stock threshold" when filed value is empty - Fix/issue 36960 #37855

Fix "Out of stock threshold" when filed value is empty - Fix/issue 36960 #37855

Uh oh!

bartech commented Apr 19, 2023

Uh oh!

codecov bot commented Apr 19, 2023 •

edited

Loading

Uh oh!

jorgeatorres left a comment

Uh oh!

jorgeatorres Apr 19, 2023

Uh oh!

bartech Apr 24, 2023

Uh oh!

jorgeatorres left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	switch ( $option['type'] ) {
	case 'checkbox':
	$value = '1' === $raw_value \|\| 'yes' === $raw_value ? 'yes' : 'no';
	break;
	case 'textarea':
	$value = wp_kses_post( trim( $raw_value ) );
	break;
	case 'multiselect':
	case 'multi_select_countries':
	$value = array_filter( array_map( 'wc_clean', (array) $raw_value ) );
	break;
	case 'image_width':
	$value = array();
	if ( isset( $raw_value['width'] ) ) {
	$value['width'] = wc_clean( $raw_value['width'] );
	$value['height'] = wc_clean( $raw_value['height'] );
	$value['crop'] = isset( $raw_value['crop'] ) ? 1 : 0;
	} else {
	$value['width'] = $option['default']['width'];
	$value['height'] = $option['default']['height'];
	$value['crop'] = $option['default']['crop'];
	}
	break;
	case 'select':
	$allowed_values = empty( $option['options'] ) ? array() : array_map( 'strval', array_keys( $option['options'] ) );
	if ( empty( $option['default'] ) && empty( $allowed_values ) ) {
	$value = null;
	break;
	}
	$default = ( empty( $option['default'] ) ? $allowed_values[0] : $option['default'] );
	$value = in_array( $raw_value, $allowed_values, true ) ? $raw_value : $default;
	break;
	case 'relative_date_selector':
	$value = wc_parse_relative_date_option( $raw_value );
	break;
	default:
	$value = wc_clean( $raw_value );
	break;
	}

Fix "Out of stock threshold" when filed value is empty - Fix/issue 36960 #37855

Fix "Out of stock threshold" when filed value is empty - Fix/issue 36960 #37855

Uh oh!

Conversation

bartech commented Apr 19, 2023

Submission Review Guidelines:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Uh oh!

codecov bot commented Apr 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jorgeatorres left a comment

Choose a reason for hiding this comment

Uh oh!

jorgeatorres Apr 19, 2023

Choose a reason for hiding this comment

Uh oh!

bartech Apr 24, 2023

Choose a reason for hiding this comment

Uh oh!

jorgeatorres left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Apr 19, 2023 •

edited

Loading