Fix compute_new_expense_amount to set 0.00 to taxes when subtotal is ''
The problem is that parse_price('', 2) returned NULL instead of throwing and exception: it seems that accessing var[1] of a text[] variable set to the empty array, {}, returns NULL, and NULL::integer is, of course, still NULL. Apparently, this is the only case, until now, that i had an empty subtotal, and i did not know what to do: should i keep the function as is and just handle its NULL return, change it to return 0 in that case, or raise an exception? The argument for the first two options, to leave it as is or to return zero, was that it was convenient for me to allow empty strings as input values, because that’s what i get from an empty <input>; returning zero would avoid an extra coalesce everywhere the function was used. The argument in favor to the last option, an exception, was that the empty string does not represent an integer, nor a “unknown” (NULL) integer, therefore the function should do the same when i pass in any other string that does not represent an integer, just as “a.b”. At the end i went for option three, because it is the one that breaks fewer expectatives: casting an empty string to integer, or passing an empty string as the first value to to_number() throw and exception in PostgreSQL; my function should do the same. Heck, that what **i** expected it to do because of the casting inside the function. To still allow empty strings as parameter to compute_new_expense_amount, the only case so far, i only had to check for that empty string and convert it to the string representation of zero, so that parse_price returns the value i want for that function. This, of course, breaks the same expectatives as returning NULL for to_price, but i think it is OK in this case because to_price is more general—used in many more cases—than compute_new_expense_amount, which is only intended for that HTML form. Closes #77.
This commit is contained in:
parent
1c6375b51d
commit
52256c3cb9
|
@ -14,6 +14,9 @@ $$
|
||||||
declare
|
declare
|
||||||
result new_expense_amount;
|
result new_expense_amount;
|
||||||
begin
|
begin
|
||||||
|
if trim(subtotal) = '' then
|
||||||
|
subtotal = '0';
|
||||||
|
end if;
|
||||||
if array_length(taxes, 1) > 0 then
|
if array_length(taxes, 1) > 0 then
|
||||||
with line as (
|
with line as (
|
||||||
select round(parse_price(subtotal, currency.decimal_digits)) as price
|
select round(parse_price(subtotal, currency.decimal_digits)) as price
|
||||||
|
|
|
@ -24,6 +24,9 @@ begin
|
||||||
end if;
|
end if;
|
||||||
|
|
||||||
result := parts[1]::integer;
|
result := parts[1]::integer;
|
||||||
|
if result is null then
|
||||||
|
raise invalid_parameter_value using message = price || ' is not a valid price representation.';
|
||||||
|
end if;
|
||||||
for d in 1..decimal_digits loop
|
for d in 1..decimal_digits loop
|
||||||
result := result * 10;
|
result := result * 10;
|
||||||
end loop;
|
end loop;
|
||||||
|
|
|
@ -0,0 +1,53 @@
|
||||||
|
-- Deploy numerus:parse_price to pg
|
||||||
|
-- requires: schema_public
|
||||||
|
|
||||||
|
begin;
|
||||||
|
|
||||||
|
set search_path to numerus, public;
|
||||||
|
|
||||||
|
create or replace function parse_price(price text, decimal_digits integer) returns integer as
|
||||||
|
$$
|
||||||
|
declare
|
||||||
|
parts text[];
|
||||||
|
result int;
|
||||||
|
frac_part text;
|
||||||
|
sign int := 1;
|
||||||
|
begin
|
||||||
|
if price like '-%' Then
|
||||||
|
sign := -1;
|
||||||
|
price := substring(price from 2);
|
||||||
|
end if;
|
||||||
|
|
||||||
|
parts := string_to_array(price, '.');
|
||||||
|
if array_length(parts, 1) > 2 then
|
||||||
|
raise invalid_parameter_value using message = price || ' is not a valid price representation.';
|
||||||
|
end if;
|
||||||
|
|
||||||
|
result := parts[1]::integer;
|
||||||
|
for d in 1..decimal_digits loop
|
||||||
|
result := result * 10;
|
||||||
|
end loop;
|
||||||
|
|
||||||
|
if array_length(parts, 1) = 2 then
|
||||||
|
frac_part := rtrim(parts[2], '0');
|
||||||
|
if length(frac_part) > decimal_digits then
|
||||||
|
raise invalid_parameter_value using message = price || ' has too many digits in the fraction part.';
|
||||||
|
end if;
|
||||||
|
frac_part := rpad(frac_part, decimal_digits, '0');
|
||||||
|
result := result + frac_part::integer;
|
||||||
|
end if;
|
||||||
|
|
||||||
|
return sign * result;
|
||||||
|
end;
|
||||||
|
$$
|
||||||
|
language plpgsql
|
||||||
|
immutable;
|
||||||
|
|
||||||
|
comment on function parse_price(text, integer) is
|
||||||
|
'Converts the string representation of a price in decimal form to cents, according to the number of decimal digits passed.';
|
||||||
|
|
||||||
|
revoke execute on function parse_price(text, integer) from public;
|
||||||
|
grant execute on function parse_price(text, integer) to invoicer;
|
||||||
|
grant execute on function parse_price(text, integer) to admin;
|
||||||
|
|
||||||
|
commit;
|
|
@ -1,7 +1,53 @@
|
||||||
-- Revert numerus:parse_price from pg
|
-- Deploy numerus:parse_price to pg
|
||||||
|
-- requires: schema_public
|
||||||
|
|
||||||
begin;
|
begin;
|
||||||
|
|
||||||
drop function if exists numerus.parse_price(text, integer);
|
set search_path to numerus, public;
|
||||||
|
|
||||||
|
create or replace function parse_price(price text, decimal_digits integer) returns integer as
|
||||||
|
$$
|
||||||
|
declare
|
||||||
|
parts text[];
|
||||||
|
result int;
|
||||||
|
frac_part text;
|
||||||
|
sign int := 1;
|
||||||
|
begin
|
||||||
|
if price like '-%' Then
|
||||||
|
sign := -1;
|
||||||
|
price := substring(price from 2);
|
||||||
|
end if;
|
||||||
|
|
||||||
|
parts := string_to_array(price, '.');
|
||||||
|
if array_length(parts, 1) > 2 then
|
||||||
|
raise invalid_parameter_value using message = price || ' is not a valid price representation.';
|
||||||
|
end if;
|
||||||
|
|
||||||
|
result := parts[1]::integer;
|
||||||
|
for d in 1..decimal_digits loop
|
||||||
|
result := result * 10;
|
||||||
|
end loop;
|
||||||
|
|
||||||
|
if array_length(parts, 1) = 2 then
|
||||||
|
frac_part := rtrim(parts[2], '0');
|
||||||
|
if length(frac_part) > decimal_digits then
|
||||||
|
raise invalid_parameter_value using message = price || ' has too many digits in the fraction part.';
|
||||||
|
end if;
|
||||||
|
frac_part := rpad(frac_part, decimal_digits, '0');
|
||||||
|
result := result + frac_part::integer;
|
||||||
|
end if;
|
||||||
|
|
||||||
|
return sign * result;
|
||||||
|
end;
|
||||||
|
$$
|
||||||
|
language plpgsql
|
||||||
|
immutable;
|
||||||
|
|
||||||
|
comment on function parse_price(text, integer) is
|
||||||
|
'Converts the string representation of a price in decimal form to cents, according to the number of decimal digits passed.';
|
||||||
|
|
||||||
|
revoke execute on function parse_price(text, integer) from public;
|
||||||
|
grant execute on function parse_price(text, integer) to invoicer;
|
||||||
|
grant execute on function parse_price(text, integer) to admin;
|
||||||
|
|
||||||
commit;
|
commit;
|
||||||
|
|
|
@ -0,0 +1,7 @@
|
||||||
|
-- Revert numerus:parse_price from pg
|
||||||
|
|
||||||
|
begin;
|
||||||
|
|
||||||
|
drop function if exists numerus.parse_price(text, integer);
|
||||||
|
|
||||||
|
commit;
|
|
@ -126,3 +126,4 @@ invoice_attachment [schema_numerus roles invoice] 2023-07-12T17:10:58Z jordi fit
|
||||||
attach_to_invoice [schema_numerus roles invoice invoice_attachment] 2023-07-12T17:21:19Z jordi fita mas <jordi@tandem.blog> # Add function to attachment a document to invoices
|
attach_to_invoice [schema_numerus roles invoice invoice_attachment] 2023-07-12T17:21:19Z jordi fita mas <jordi@tandem.blog> # Add function to attachment a document to invoices
|
||||||
new_expense_amount [schema_numerus] 2023-07-13T17:45:33Z jordi fita mas <jordi@tandem.blog> # Add type to return when computing new expense amounts
|
new_expense_amount [schema_numerus] 2023-07-13T17:45:33Z jordi fita mas <jordi@tandem.blog> # Add type to return when computing new expense amounts
|
||||||
compute_new_expense_amount [schema_numerus roles company tax new_expense_amount] 2023-07-13T17:34:12Z jordi fita mas <jordi@tandem.blog> # Add function to compute the taxes and total for a new expense
|
compute_new_expense_amount [schema_numerus roles company tax new_expense_amount] 2023-07-13T17:34:12Z jordi fita mas <jordi@tandem.blog> # Add function to compute the taxes and total for a new expense
|
||||||
|
parse_price [parse_price@v1] 2023-08-25T11:59:54Z jordi fita mas <jordi@tandem.blog> # Throw when subtotal is empty string
|
||||||
|
|
|
@ -5,7 +5,7 @@ reset client_min_messages;
|
||||||
|
|
||||||
begin;
|
begin;
|
||||||
|
|
||||||
select plan(14);
|
select plan(15);
|
||||||
|
|
||||||
set search_path to numerus, auth, public;
|
set search_path to numerus, auth, public;
|
||||||
|
|
||||||
|
@ -54,6 +54,11 @@ select is(
|
||||||
'("{}",0.00)'::new_expense_amount
|
'("{}",0.00)'::new_expense_amount
|
||||||
);
|
);
|
||||||
|
|
||||||
|
select is(
|
||||||
|
compute_new_expense_amount(1, '', array[2,5,3]::integer[]),
|
||||||
|
'("{{IRPF -15 %,0.00},{IVA 4 %,0.00},{IVA 21 %,0.00}}",0.00)'::new_expense_amount
|
||||||
|
);
|
||||||
|
|
||||||
select is(
|
select is(
|
||||||
compute_new_expense_amount(1, '4.60', array[2,5,3]),
|
compute_new_expense_amount(1, '4.60', array[2,5,3]),
|
||||||
'("{{IRPF -15 %,-0.69},{IVA 4 %,0.18},{IVA 21 %,0.97}}",5.06)'::new_expense_amount
|
'("{{IRPF -15 %,-0.69},{IVA 4 %,0.18},{IVA 21 %,0.97}}",5.06)'::new_expense_amount
|
||||||
|
|
|
@ -5,7 +5,7 @@ reset client_min_messages;
|
||||||
|
|
||||||
begin;
|
begin;
|
||||||
|
|
||||||
select plan(42);
|
select plan(44);
|
||||||
|
|
||||||
set search_path to auth, numerus, public;
|
set search_path to auth, numerus, public;
|
||||||
|
|
||||||
|
@ -52,6 +52,8 @@ select is( parse_price('00000000000123456.789000000000000000000', 3), 123456789
|
||||||
select throws_ok( $$ select parse_price('1,1', 2) $$ );
|
select throws_ok( $$ select parse_price('1,1', 2) $$ );
|
||||||
select throws_ok( $$ select parse_price('1.1.1', 2) $$ );
|
select throws_ok( $$ select parse_price('1.1.1', 2) $$ );
|
||||||
select throws_ok( $$ select parse_price('a.b', 2) $$ );
|
select throws_ok( $$ select parse_price('a.b', 2) $$ );
|
||||||
|
select throws_ok( $$ select parse_price('', 1) $$);
|
||||||
|
select throws_ok( $$ select parse_price(' ', 3) $$);
|
||||||
|
|
||||||
select *
|
select *
|
||||||
from finish();
|
from finish();
|
||||||
|
|
|
@ -0,0 +1,7 @@
|
||||||
|
-- Verify numerus:parse_price on pg
|
||||||
|
|
||||||
|
begin;
|
||||||
|
|
||||||
|
select has_function_privilege('numerus.parse_price(text, integer)', 'execute');
|
||||||
|
|
||||||
|
rollback;
|
Loading…
Reference in New Issue