Bug 33864 - Problems in order receive modal
Summary: Problems in order receive modal
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on: 8179 33784
Blocks: 33867 33863 35004
  Show dependency treegraph
 
Reported: 2023-05-30 13:24 UTC by Andrii Nugged
Modified: 2023-10-06 16:15 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00


Attachments
Bug 33864: Set default date in all acqcreateitem scenarios (2.41 KB, patch)
2023-05-30 13:33 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33864: Better handling of confirmation button (2.79 KB, patch)
2023-05-30 15:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33864: Better handling of confirmation button (2.79 KB, patch)
2023-05-30 16:13 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33864: Set default date in all acqcreateitem scenarios (2.52 KB, patch)
2023-05-30 16:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33864: Better handling of confirmation button (2.90 KB, patch)
2023-05-30 16:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33864: Restore inputmode="numeric" (bug 28283) (1.41 KB, patch)
2023-05-30 16:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33864: Fix to store shadow value for input as well (1.62 KB, patch)
2023-05-30 18:17 UTC, Slava Shishkin
Details | Diff | Splinter Review
Bug 33864: Trigger the 'change' event when setting quantity (1.53 KB, patch)
2023-05-30 19:13 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33864: Set default date in all acqcreateitem scenarios (2.57 KB, patch)
2023-05-31 02:36 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 33864: Better handling of confirmation button (2.94 KB, patch)
2023-05-31 02:36 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 33864: Restore inputmode="numeric" (bug 28283) (1.46 KB, patch)
2023-05-31 02:36 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 33864: Trigger the 'change' event when setting quantity (1.58 KB, patch)
2023-05-31 02:36 UTC, Andrii Nugged
Details | Diff | Splinter Review
Bug 33864: Set default date in all acqcreateitem scenarios (2.57 KB, patch)
2023-05-31 05:48 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33864: Better handling of confirmation button (2.88 KB, patch)
2023-05-31 05:49 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33864: Restore inputmode="numeric" (bug 28283) (1.51 KB, patch)
2023-05-31 05:49 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33864: Trigger the 'change' event when setting quantity (1.63 KB, patch)
2023-05-31 05:49 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrii Nugged 2023-05-30 13:24:35 UTC
To test:

0. Set syspref to "AcqCreateItem" set to "cataloguing"
( /cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=AcqCreateItem )

1. Create a basket
   1.1. Go to Acquisitions
   1.2. Search for a vendor
   1.3. Click New > Basket
   1.4. Fill out the basket form
   1.5. Click Save

2. Add at least one order to the basket
   2.1. Click Add to basket
   2.2. Click From a new (empty) record
   2.3. Fill out the order form
   2.4. Click Save

3. Click Close basket and confirm

4. Click Receive shipments

5. THE PROBLEM IS HERE:
   - "Confirm" button never gets enabled even if we fill in the form fields,
   - "Date received:" not prefilled but better should be filled (auto? or from DB?),
   - "Quantity received:" not prefilled,
     but on 22.11 it was "1" by default and anyway saved clicks for librarians,
     and maybe it also should be filled with some cleverly 
     even in "AcqCreateItem" set to "cataloguing" (=1?)


NOTE: "AcqCreateItem" set to "cataloguing" is used in our libraries because they receive newspapers and magazines which not then have physical items but by quantity per-biblio, that's why and hos this option is used in our case.
Comment 1 Tomás Cohen Arazi 2023-05-30 13:33:42 UTC
Created attachment 151799 [details] [review]
Bug 33864: Set default date in all acqcreateitem scenarios

On b0aab2aeef9a025d3907d0b723b5ea71eaf94542 the flatpickr default to
'today' was restored, but only for acqcreateitem='ordering'. This patch
moves the initialization to a different stop for broader reach.

To test:
1. Have 3 baskets on with each setting:
   - create_item = 'cateloguing'
   - create_item = 'ordering'
   - create_item = 'receiving'
2. Close those baskets
3. Go to 'Receive shipments'
4. Choose things from the 3 baskets and then 'Receive selected'
5. Navigate through the different orders
=> FAIL: Only the one that has items created on ordering has the date
filled by default
6. Apply this patch
7. Reload the page
8. Repeat 5
=> SUCCESS: Dates are pre-filled!
9. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Andrii Nugged 2023-05-30 13:41:00 UTC
Extra note: in 22.11 quantity was hard coded to 1 when no quantityreceived and the date to "today" if there's no "datereceived" property for the order,
and that was more comfortable for our librarians than "0"/"empty".
Comment 3 Tomás Cohen Arazi 2023-05-30 15:11:34 UTC
Created attachment 151803 [details] [review]
Bug 33864: Better handling of confirmation button

This patch corrects the behavior of the 'Confirm' button on the three
possible scenarios regarding item creation.

Ordering:
- Quantity gets locked (only changeable when selecting items)
- 'Confirm' disabled, only gets enabled if quantity > 0

Receiving:
- Quantity gets locked (only changeable when creating items)
- 'Confirm' disabled, only gets enabled if quantity > 0

Cataloguing:
- It now defaults to 1
- 'Confirm' enabled by default (because of 1) if quantity is set to 0,
  it gets disabled.

To test:
1. Verify the described behavior with the sample orders for the previous
   patch.
=> SUCCESS: It does the job!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Sam Lau 2023-05-30 15:39:42 UTC
error: could not build fake ancestor
Patch failed at 0001 Bug 33864: Set default date in all acqcreateitem scenarios
Comment 5 Katrin Fischer 2023-05-30 15:43:20 UTC
Hi Sam, please make sure you do a git pull before trying to apply patches, Koha is moving fast. Patches apply ok on most recent master branch, testing now.
Comment 6 Katrin Fischer 2023-05-30 15:59:18 UTC
2 things:

1) The date received patch works fine. No complaints.
2) I have an issue with quantity received: 
   I tested in 20.11.19 and the quantity is set to 0 zero there. 
   Could setting it to 1 have been a local customization?
   I am in favor of leaving it at 0 and keeping the existing behavior of 
   older versions (I couldn't verify in 22.05) 
   BUT: if we pre-populated I think 1 would still be wrong, it should be
   pre-set with quantity ordered.
3) BLOCKER: There is an issue if you have a basket with 
   * item create on order
   * standing order
   You'll have an empty item form on the left and quantity received is BLOCKED.
   There is no way to receive.
   This combination is kinda odd, but currently doesn't cause issues, we can
   expect libraries having 'order' as standard to forget to change it and they
   won't be able to fix this in the GUI as the item create value can only be set
   when the basket is created.
Comment 7 Katrin Fischer 2023-05-30 16:03:22 UTC
4) BLOCKER: if item create 'on receive':
   Don't lock 'Quantity received'. 
   The existing behaviour is like this:
   * Enter the number received if you don't care which items will be selected.
     X checkboxes will be set on the items from top to bottom.
   * Check uncheck items and the quantity will be adjusted accordingly.
  The first use case is the one used more often and it's no broken.
  This means if a library orders something in big numbers, they need to click
  the checkbox for each and every item (and hate it ;) )
Comment 8 Tomás Cohen Arazi 2023-05-30 16:13:39 UTC
Created attachment 151811 [details] [review]
Bug 33864: Better handling of confirmation button

This patch corrects the behavior of the 'Confirm' button on the three
possible scenarios regarding item creation.

Ordering:
- Quantity gets locked (only changeable when selecting items)
- 'Confirm' disabled, only gets enabled if quantity > 0

Receiving:
- Quantity gets locked (only changeable when creating items)
- 'Confirm' disabled, only gets enabled if quantity > 0

Cataloguing:
- It now defaults to 1
- 'Confirm' enabled by default (because of 1) if quantity is set to 0,
  it gets disabled.

To test:
1. Verify the described behavior with the sample orders for the previous
   patch.
=> SUCCESS: It does the job!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Tomás Cohen Arazi 2023-05-30 16:15:32 UTC
(In reply to Katrin Fischer from comment #6)
> 2 things:
> 
> 1) The date received patch works fine. No complaints.

Great.

> 2) I have an issue with quantity received: 
>    I tested in 20.11.19 and the quantity is set to 0 zero there. 
>    Could setting it to 1 have been a local customization?
>    I am in favor of leaving it at 0 and keeping the existing behavior of 
>    older versions (I couldn't verify in 22.05) 
>    BUT: if we pre-populated I think 1 would still be wrong, it should be
>    pre-set with quantity ordered.

Separate bug I think, we checked this is how it works in 22.11.

> 3) BLOCKER: There is an issue if you have a basket with 
>    * item create on order
>    * standing order
>    You'll have an empty item form on the left and quantity received is
> BLOCKED.
>    There is no way to receive.
>    This combination is kinda odd, but currently doesn't cause issues, we can
>    expect libraries having 'order' as standard to forget to change it and
> they
>    won't be able to fix this in the GUI as the item create value can only be
> set
>    when the basket is created.

Can you check with the new version?



(In reply to Katrin Fischer from comment #7)
> 4) BLOCKER: if item create 'on receive':
>    Don't lock 'Quantity received'. 
>    The existing behaviour is like this:
>    * Enter the number received if you don't care which items will be
> selected.
>      X checkboxes will be set on the items from top to bottom.
>    * Check uncheck items and the quantity will be adjusted accordingly.
>   The first use case is the one used more often and it's no broken.
>   This means if a library orders something in big numbers, they need to click
>   the checkbox for each and every item (and hate it ;) )

Can you check with the new version?
Comment 10 Katrin Fischer 2023-05-30 16:27:00 UTC
Created attachment 151812 [details] [review]
Bug 33864: Set default date in all acqcreateitem scenarios

On b0aab2aeef9a025d3907d0b723b5ea71eaf94542 the flatpickr default to
'today' was restored, but only for acqcreateitem='ordering'. This patch
moves the initialization to a different stop for broader reach.

To test:
1. Have 3 baskets on with each setting:
   - create_item = 'cateloguing'
   - create_item = 'ordering'
   - create_item = 'receiving'
2. Close those baskets
3. Go to 'Receive shipments'
4. Choose things from the 3 baskets and then 'Receive selected'
5. Navigate through the different orders
=> FAIL: Only the one that has items created on ordering has the date
filled by default
6. Apply this patch
7. Reload the page
8. Repeat 5
=> SUCCESS: Dates are pre-filled!
9. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Katrin Fischer 2023-05-30 16:27:03 UTC
Created attachment 151813 [details] [review]
Bug 33864: Better handling of confirmation button

This patch corrects the behavior of the 'Confirm' button on the three
possible scenarios regarding item creation.

Ordering:
- Quantity gets locked (only changeable when selecting items)
- 'Confirm' disabled, only gets enabled if quantity > 0

Receiving:
- Quantity gets locked (only changeable when creating items)
- 'Confirm' disabled, only gets enabled if quantity > 0

Cataloguing:
- It now defaults to 1
- 'Confirm' enabled by default (because of 1) if quantity is set to 0,
  it gets disabled.

To test:
1. Verify the described behavior with the sample orders for the previous
   patch.
=> SUCCESS: It does the job!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

https://bugs.koha-community.org/show_bug.cgi?id=33846
Comment 12 Andrii Nugged 2023-05-30 16:45:01 UTC
but to say how it behave before 23.05 - on our 22.11 there was such code:

    [% IF ( order.quantityreceived ) %]
        ...
    [% ELSE %]
        <input type="text" inputmode="numeric" pattern="[0-9]*" id="quantity" size="20" name="quantityrec" value="1" />
        <input id="origquantityrec" readonly="readonly" type="hidden" name="origquantityrec" value="0" />
    [% END %]

i.e. hardcoded `value="1"`. And I see same on 22.05 and since ages:

Bug 5917 : Converted templates
77b64098
Year 2011

    [% IF ( items ) %]
        <input id="quantityrec" READONLY type="text" size="20" name="quantityrec" value="1" />
    [% ELSE %]
        <input id="quantityrec" type="text" size="20" name="quantityrec" value="1" />
    [% END %]

======

>(In reply to Katrin Fischer from comment #6)
>    I am in favor of leaving it at 0 and keeping the existing behavior of 
>    older versions (I couldn't verify in 22.05) 
>    BUT: if we pre-populated I think 1 would still be wrong, it should be
>    pre-set with quantity ordered.

It would be a time saver if it were prepopulated with the quantity ordered,

but at least having it "1" covers most cases our librarians receiving something 1,
so good point to have it more clever but not zero.
Comment 13 Tomás Cohen Arazi 2023-05-30 16:58:57 UTC
Created attachment 151816 [details] [review]
Bug 33864: Restore inputmode="numeric" (bug 28283)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Slava Shishkin 2023-05-30 18:17:32 UTC
Created attachment 151821 [details] [review]
Bug 33864: Fix to store shadow value for input as well
Comment 15 Tomás Cohen Arazi 2023-05-30 19:13:27 UTC
Created attachment 151824 [details] [review]
Bug 33864: Trigger the 'change' event when setting quantity

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Katrin Fischer 2023-05-30 20:53:06 UTC
Can you explain the latest changes for retesting?
Comment 17 Tomás Cohen Arazi 2023-05-30 23:08:39 UTC
(In reply to Katrin Fischer from comment #16)
> Can you explain the latest changes for retesting?

The latest patch from Slava then replaced by mine, were talked about on Telegram...

When you're using the 'cataloguing' setting, the input is correctly filled with '1' (as per our agreement) but any changes to the UI, should be stored in the underlying JS object that tracks the state of the request.

When setting it, I force the 'change()' event, so that internal synced happens (by means of an event handler defined previously for the 'change' event.

If you want to test, verify that without the patch, '1' is set, and when you want to confirm, a brief message about receiving less than 1 is incorrect, and then the parcel.pl table displays nothing got received.

With the patch, that's fixed: no error, and the thing is received.
Comment 18 Andrii Nugged 2023-05-31 02:36:31 UTC
Created attachment 151852 [details] [review]
Bug 33864: Set default date in all acqcreateitem scenarios

On b0aab2aeef9a025d3907d0b723b5ea71eaf94542 the flatpickr default to
'today' was restored, but only for acqcreateitem='ordering'. This patch
moves the initialization to a different stop for broader reach.

To test:
1. Have 3 baskets on with each setting:
   - create_item = 'cateloguing'
   - create_item = 'ordering'
   - create_item = 'receiving'
2. Close those baskets
3. Go to 'Receive shipments'
4. Choose things from the 3 baskets and then 'Receive selected'
5. Navigate through the different orders
=> FAIL: Only the one that has items created on ordering has the date
filled by default
6. Apply this patch
7. Reload the page
8. Repeat 5
=> SUCCESS: Dates are pre-filled!
9. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

https://bugs.koha-community.org/show_bug.cgi?id=33846
Comment 19 Andrii Nugged 2023-05-31 02:36:34 UTC
Created attachment 151853 [details] [review]
Bug 33864: Better handling of confirmation button

This patch corrects the behavior of the 'Confirm' button on the three
possible scenarios regarding item creation.

Ordering:
- Quantity gets locked (only changeable when selecting items)
- 'Confirm' disabled, only gets enabled if quantity > 0

Receiving:
- Quantity gets locked (only changeable when creating items)
- 'Confirm' disabled, only gets enabled if quantity > 0

Cataloguing:
- It now defaults to 1
- 'Confirm' enabled by default (because of 1) if quantity is set to 0,
  it gets disabled.

To test:
1. Verify the described behavior with the sample orders for the previous
   patch.
=> SUCCESS: It does the job!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

https://bugs.koha-community.org/show_bug.cgi?id=33846
Comment 20 Andrii Nugged 2023-05-31 02:36:37 UTC
Created attachment 151854 [details] [review]
Bug 33864: Restore inputmode="numeric" (bug 28283)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 21 Andrii Nugged 2023-05-31 02:36:40 UTC
Created attachment 151855 [details] [review]
Bug 33864: Trigger the 'change' event when setting quantity

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 22 Katrin Fischer 2023-05-31 05:48:59 UTC
Created attachment 151856 [details] [review]
Bug 33864: Set default date in all acqcreateitem scenarios

On b0aab2aeef9a025d3907d0b723b5ea71eaf94542 the flatpickr default to
'today' was restored, but only for acqcreateitem='ordering'. This patch
moves the initialization to a different stop for broader reach.

To test:
1. Have 3 baskets on with each setting:
   - create_item = 'cateloguing'
   - create_item = 'ordering'
   - create_item = 'receiving'
2. Close those baskets
3. Go to 'Receive shipments'
4. Choose things from the 3 baskets and then 'Receive selected'
5. Navigate through the different orders
=> FAIL: Only the one that has items created on ordering has the date
filled by default
6. Apply this patch
7. Reload the page
8. Repeat 5
=> SUCCESS: Dates are pre-filled!
9. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

https://bugs.koha-community.org/show_bug.cgi?id=33846
Comment 23 Katrin Fischer 2023-05-31 05:49:02 UTC
Created attachment 151857 [details] [review]
Bug 33864: Better handling of confirmation button

This patch corrects the behavior of the 'Confirm' button on the three
possible scenarios regarding item creation.

Ordering:
- 'Confirm' disabled, only gets enabled if quantity > 0

Receiving:
- Quantity gets locked (only changeable when creating items)
- 'Confirm' disabled, only gets enabled if quantity > 0

Cataloguing:
- It now defaults to 1
- 'Confirm' enabled by default (because of 1) if quantity is set to 0,
  it gets disabled.

To test:
1. Verify the described behavior with the sample orders for the previous
   patch.
=> SUCCESS: It does the job!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

https://bugs.koha-community.org/show_bug.cgi?id=33846
Comment 24 Katrin Fischer 2023-05-31 05:49:05 UTC
Created attachment 151858 [details] [review]
Bug 33864: Restore inputmode="numeric" (bug 28283)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Katrin Fischer 2023-05-31 05:49:09 UTC
Created attachment 151859 [details] [review]
Bug 33864: Trigger the 'change' event when setting quantity

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 26 Katrin Fischer 2023-05-31 05:49:40 UTC
I did another round of testing and adjusted the test plan of the second patch (we no longer lock quantity for 'create on order')
Comment 27 Jonathan Druart 2023-05-31 08:20:30 UTC
Pushed to master for 23.05.00
Comment 28 Matt Blenkinsop 2023-06-08 17:21:51 UTC
Dependencies missing - not backporting to 22.11.x