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

Fatal error on non-numerical quantity #40

Closed
rvdsteege opened this issue Apr 17, 2024 · 3 comments · Fixed by #43
Closed

Fatal error on non-numerical quantity #40

rvdsteege opened this issue Apr 17, 2024 · 3 comments · Fixed by #43
Assignees

Comments

@rvdsteege
Copy link
Member

A fatal error occurs when a non-numerical value is provided as quantity.

PHP Fatal error:  Uncaught InvalidArgumentException: No numerical value: 1 box. in /wp-content/plugins/pronamic-pay-premium/packages/pronamic/wp-number/src/Number.php:442

Stack trace:
#0 /wp-content/plugins/pronamic-pay-premium/packages/pronamic/wp-number/src/Number.php(472): Pronamic\WordPress\Number\Number::parse_string('1 box')
#1 /wp-content/plugins/pronamic-pay-premium/packages/pronamic/wp-number/src/Number.php(320): Pronamic\WordPress\Number\Number::parse_mixed('1 box')
#2 /wp-content/plugins/pronamic-pay-premium/packages/wp-pay-extensions/gravityforms/src/Processor.php(359): Pronamic\WordPress\Number\Number::from_mixed('1 box')
#3 /wp-includes/class-wp-hook.php(326): Pronamic\WordPress\Pay\Extensions\GravityForms\Processor->entry_post_save(Array, Array)
#4 /wp-includes/plugin.php(205): WP_Hook->apply_filters(Array, Array)
#5 /wp-content/plugins/gravityforms/gravityforms.php(7031): apply_filters('gform_entry_pos...', Array, Array, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL)
#6 /wp-content/plugins/gravityforms/form_display.php(1932): gf_apply_filters('gform_entry_pos...', Array, Array)
#7 /wp-content/plugins/gravityforms/form_display.php(196): GFFormDisplay::handle_submission(Array, Array, false)
#8 /wp-content/plugins/gravityforms/gravityforms.php(869): GFFormDisplay::process_form(4, 1)
#9 /wp-includes/class-wp-hook.php(324): GFForms::maybe_process_form(Object(WP))
#10 /wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters(NULL, Array)
#11 /wp-includes/plugin.php(565): WP_Hook->do_action(Array)
#12 /wp-includes/class-wp.php(830): do_action_ref_array('wp', Array)
#13 /wp-includes/functions.php(1336): WP->main('')
#14 /wp-blog-header.php(16): wp()
#15 /index.php(17): require('wp-blog-header.php')
#16 {main}
  thrown in /wp-content/plugins/pronamic-pay-premium/packages/pronamic/wp-number/src/Number.php on line 442

Internal Help Scout ticket: https://secure.helpscout.net/conversation/2568111710/27116

@remcotolsma
Copy link
Member

remcotolsma commented May 1, 2024

Related code:

if ( array_key_exists( 'price', $product ) ) {
$value = Number::from_mixed( GFCommon::to_number( $product['price'] ) );
$line->set_unit_price( new Money( $value, $currency ) );
if ( array_key_exists( 'quantity', $product ) ) {
$quantity = Number::from_mixed( $product['quantity'] );
$value = $value->multiply( $quantity );
}
$line->set_total_amount( new Money( $value, $currency ) );
}

At first i noticed the quantity field is of the type number:

<input type="number" name="input_1.3" value="" id="input_1_1_1" class="ginput_quantity" size="10" min="0" aria-label="Quantity Product A" aria-describedby="ginput_product_price_1_1">
Scherm­afbeelding 2024-05-01 om 11 33 59

This field does not allows values like 1 box, but users can also change the quantity field to a 'Drop Down'. This allows for other values:

Scherm­afbeelding 2024-05-01 om 11 35 30

I know that in Moneybird you can also use other quantity values:
https://help.moneybird.nl/support/solutions/articles/103000075828-details-per-factuurregel

Een getal: Als je in het veld een getal zet, ziet Moneybird dat als aantal. Je kunt er ook korte tekst bij zetten, zoals bijvoorbeeld 1 x, 3 stuks, 5 dozen of 14 m^2. Als er meerdere getallen in het veld staan, wordt het eerste getal als aantal genomen, dus bij 14 m2 is 14 het daadwerkelijke aantal, de 2 aan het eind heeft geen invloed.

Perhaps we should process this in a similar way @rvdsteege?

ChatGPT provided the following:

function extractNumberFromString($input) {
    // Regular expression om het eerste numerieke getal in de string te vinden
    preg_match('/\d+/', $input, $matches);
    
    // Als er een match is gevonden, retourneer het eerste getal, anders retourneer 0
    if (!empty($matches)) {
        return intval($matches[0]);
    } else {
        return 0;
    }
}

// Testen van de functie met verschillende inputs
echo extractNumberFromString("1 x");        // Output: 1
echo extractNumberFromString("3 stuks");    // Output: 3
echo extractNumberFromString("5 dozen");    // Output: 5
echo extractNumberFromString("14 m^2");     // Output: 14

And with time 01:00 notation and a space thousands separator 1 000 000 support:

function extractNumberFromString($input) {
    // Replace spaces with empty strings to handle thousands separators
    $input = str_replace(' ', '', $input);
    
    // Regular expression to find the first numeric value in the string
    preg_match('/\d+(:\d+)?/', $input, $matches);
    
    // If a match is found
    if (!empty($matches)) {
        // Split the found number on ':' if it exists
        $number_parts = explode(':', $matches[0]);
        
        // If the number of parts is 1, simply return that number
        if (count($number_parts) == 1) {
            return intval($number_parts[0]);
        } 
        // If the number of parts is 2, calculate the number as a decimal
        elseif (count($number_parts) == 2) {
            return intval($number_parts[0]) + intval($number_parts[1]) / 60;
        }
    }
    
    // If no match is found, return 0
    return 0;
}

// Testing the function with various inputs
echo extractNumberFromString("1 x");        // Output: 1
echo "\n";
echo extractNumberFromString("3 stuks");    // Output: 3
echo "\n";
echo extractNumberFromString("5 dozen");    // Output: 5
echo "\n";
echo extractNumberFromString("14 m^2");     // Output: 14
echo "\n";
echo extractNumberFromString("01:00");      // Output: 1
echo "\n";
echo extractNumberFromString("01:30");      // Output: 1.5
echo "\n";
echo extractNumberFromString("1 000");      // Output: 1000
echo "\n";

In Moneybird they even store these values separate in amount (string) and amount_decimal (numeric):

  "details": [
    {
      "id": "416087652863837643",
      "administration_id": 123,
      "tax_rate_id": "416087461198825343",
      "ledger_account_id": "416087461088724843",
      "project_id": null,
      "product_id": null,
      "amount": "1 x",
      "amount_decimal": "1.0",
      "description": "Project X",
      "price": "300.0",
      "period": "20240301..20240331",
      "row_order": 1,
      "total_price_excl_tax_with_discount": "300.0",
      "total_price_excl_tax_with_discount_base": "300.0",
      "tax_report_reference": [
        "NL/1a"
      ],
      "mandatory_tax_text": null,
      "created_at": "2024-03-21T14:42:09.629Z",
      "updated_at": "2024-03-21T14:42:09.757Z"
    }
  ],

Source: https://developer.moneybird.com/api/sales_invoices/#get_sales_invoices_id

@rvdsteege
Copy link
Member Author

I think there is little use for us in storing the non-numerical amount? But a 'parser' like suggested seems like a good solution.

However, I know you're not a big fan of preg_match() 😉 Should we use that or explode() + is_numeric() or something? (which probably gives less good results)

@rvdsteege rvdsteege assigned remcotolsma and unassigned rvdsteege May 10, 2024
@remcotolsma
Copy link
Member

remcotolsma commented May 13, 2024

I'm not a fan of using complex regular expressions that are difficult to understand. Especially if it can also be solved with some string functions. In this case, a regular expression can be quite useful. However, I was still thinking about how we can properly handle decimal and thousand separators.

  • 2,5 stuks
  • 2,500 stuks
  • 2.500 stuks
  • 2.500,50 stuks
  • 0,000017 BTC
  • 0,00037 ETH
  • 0,037 ETH
  • 2,037 ETH
  • 2,721.51 EUR

We need to know the decimal and thousand separators to be able to parse a text properly? Doesn't https://github.com/pronamic/wp-number/blob/main/src/Parser.php already take care of this for us?

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants