Bug 24412 - Attach waiting hold to desk
Summary: Attach waiting hold to desk
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nicolas Legrand
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 24201
Blocks: 14654 29103
  Show dependency treegraph
 
Reported: 2020-01-13 16:47 UTC by Nicolas Legrand
Modified: 2021-09-23 21:18 UTC (History)
9 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
When `UseCirculationDesks` is enabled and desks are defined and attached to a library, this feature makes it possible to attach a waiting reserve to a desk: when an item is checked in and marked as a waiting reserve, it is also attached to the current desk. The desk is then displayed in the intranet document request page, the intranet borrower holds tab, the item list of the document bibliographic details and the borrower's OPAC holds tab. You can move waiting reserve from desk to desk by checking in again the item at a different desk.
Version(s) released in:
20.11.00


Attachments
Bug 24412: Schema file DO NOT PUSH! (17.44 KB, patch)
2020-01-13 17:26 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.30 KB, patch)
2020-01-13 17:26 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.22 KB, patch)
2020-01-13 17:26 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Schema file DO NOT PUSH! (17.44 KB, patch)
2020-03-24 16:12 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.30 KB, patch)
2020-03-24 16:12 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.34 KB, patch)
2020-03-24 16:12 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Schema file DO NOT PUSH! (17.44 KB, patch)
2020-03-24 16:23 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.30 KB, patch)
2020-03-24 16:23 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.34 KB, patch)
2020-03-24 16:23 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Schema file DO NOT PUSH! (14.73 KB, patch)
2020-03-25 15:21 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.30 KB, patch)
2020-03-25 15:21 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.39 KB, patch)
2020-03-25 15:21 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.39 KB, patch)
2020-03-26 13:18 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Schema file DO NOT PUSH! (14.73 KB, patch)
2020-03-26 16:01 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.30 KB, patch)
2020-03-26 16:01 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.42 KB, patch)
2020-03-26 16:01 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Schema file DO NOT PUSH! (20.82 KB, patch)
2020-03-26 17:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.44 KB, patch)
2020-03-26 17:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.40 KB, patch)
2020-03-26 17:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: Schema file DO NOT PUSH! (4.09 KB, patch)
2020-03-27 08:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.44 KB, patch)
2020-03-27 08:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.40 KB, patch)
2020-03-27 08:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.50 KB, patch)
2020-07-06 15:13 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA (8.05 KB, patch)
2020-07-08 08:02 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.44 KB, patch)
2020-09-14 16:58 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.74 KB, patch)
2020-09-14 16:58 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA (8.24 KB, patch)
2020-09-14 16:58 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Schema file DO NOT PUSH! (4.09 KB, patch)
2020-09-14 16:58 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA (1.53 KB, patch)
2020-09-21 11:18 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.44 KB, patch)
2020-09-21 11:19 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.74 KB, patch)
2020-09-21 11:19 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA (8.24 KB, patch)
2020-09-21 11:20 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Schema file DO NOT PUSH! (4.09 KB, patch)
2020-09-21 11:20 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA: useless change (1.53 KB, patch)
2020-09-21 11:20 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.50 KB, patch)
2020-09-23 09:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.69 KB, patch)
2020-09-23 09:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA (7.64 KB, patch)
2020-09-23 09:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: Schema file DO NOT PUSH! (4.17 KB, patch)
2020-09-23 09:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA: useless change (2.20 KB, patch)
2020-09-23 09:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.51 KB, patch)
2020-10-20 15:24 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.56 KB, patch)
2020-10-20 15:24 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA (6.27 KB, patch)
2020-10-20 15:24 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Schema file DO NOT PUSH! (4.15 KB, patch)
2020-10-20 15:24 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA: useless change (2.19 KB, patch)
2020-10-20 15:24 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) tests (2.23 KB, patch)
2020-10-21 10:14 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) qa-tools (4.05 KB, patch)
2020-10-21 12:42 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.56 KB, patch)
2020-10-22 11:37 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.56 KB, patch)
2020-10-22 11:39 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA (6.27 KB, patch)
2020-10-22 11:39 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: Schema file DO NOT PUSH! (4.15 KB, patch)
2020-10-22 11:39 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA: useless change (2.19 KB, patch)
2020-10-22 11:40 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) tests (2.23 KB, patch)
2020-10-22 11:40 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) qa-tools (4.05 KB, patch)
2020-10-22 11:40 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) bad desk printing (2.56 KB, patch)
2020-10-22 11:40 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) prevent error in svc/holds (1.23 KB, patch)
2020-10-22 16:22 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.56 KB, patch)
2020-10-22 16:30 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA (6.27 KB, patch)
2020-10-22 16:30 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: Schema file DO NOT PUSH! (4.15 KB, patch)
2020-10-22 16:30 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA: useless change (2.19 KB, patch)
2020-10-22 16:30 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: (follow-up) tests (2.22 KB, patch)
2020-10-22 16:30 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: (follow-up) qa-tools (3.78 KB, patch)
2020-10-22 16:30 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: (follow-up) bad desk printing (2.56 KB, patch)
2020-10-22 16:30 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: (follow-up) prevent error in svc/holds (1.23 KB, patch)
2020-10-22 16:30 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: (follow-up) prevent error in svc/holds (1.21 KB, patch)
2020-10-22 16:32 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.71 KB, patch)
2020-10-22 17:17 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.71 KB, patch)
2020-10-22 17:18 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA (6.42 KB, patch)
2020-10-22 17:18 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: Schema file DO NOT PUSH! (4.29 KB, patch)
2020-10-22 17:18 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA: useless change (2.33 KB, patch)
2020-10-22 17:18 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: (follow-up) tests (2.37 KB, patch)
2020-10-22 17:18 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: (follow-up) qa-tools (3.93 KB, patch)
2020-10-22 17:18 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: (follow-up) bad desk printing (2.71 KB, patch)
2020-10-22 17:18 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: (follow-up) prevent error in svc/holds (1.36 KB, patch)
2020-10-22 17:18 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.71 KB, patch)
2020-10-23 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.75 KB, patch)
2020-10-23 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA (6.48 KB, patch)
2020-10-23 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: Schema file DO NOT PUSH! (4.38 KB, patch)
2020-10-23 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: (follow-up) QA: useless change (2.41 KB, patch)
2020-10-23 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: (follow-up) tests (2.44 KB, patch)
2020-10-23 13:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: (follow-up) qa-tools (4.00 KB, patch)
2020-10-23 13:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: (follow-up) bad desk printing (2.78 KB, patch)
2020-10-23 13:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: (follow-up) prevent error in svc/holds (1.43 KB, patch)
2020-10-23 13:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: (QA follow-up) Change CASCADE DELETE to SET NULL (2.86 KB, patch)
2020-10-23 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412: (QA follow-up) DBIC Schema Update (1.09 KB, patch)
2020-10-23 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24412 (follow-up): Fix POD (602 bytes, patch)
2020-11-05 13:19 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) prevent request.pl from failing (1.51 KB, patch)
2020-11-05 13:19 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: (follow-up) prevent js injection (2.14 KB, patch)
2020-11-05 13:58 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 24412: kohastructure and atomic update (4.71 KB, patch)
2020-11-06 10:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24412: Attach waiting reserve to desk (14.74 KB, patch)
2020-11-06 10:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24412: (follow-up) tests (2.44 KB, patch)
2020-11-06 10:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24412: (follow-up) qa-tools (4.00 KB, patch)
2020-11-06 10:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24412: (follow-up) bad desk printing (2.78 KB, patch)
2020-11-06 10:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24412: (follow-up) prevent error in svc/holds (1.43 KB, patch)
2020-11-06 10:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24412: (QA follow-up) Change CASCADE DELETE to SET NULL (2.86 KB, patch)
2020-11-06 10:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24412: (follow-up) prevent request.pl from failing (1.52 KB, patch)
2020-11-06 10:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24412: (follow-up) prevent js injection (2.15 KB, patch)
2020-11-06 10:59 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Legrand 2020-01-13 16:47:29 UTC
When an item is checked in and marked 'Waiting' or already 'Waiting' and there is a desk attached to the session, the item is marked waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The patron can then know at which desk he can retrieve his document.
Comment 1 Nicolas Legrand 2020-01-13 17:26:15 UTC Comment hidden (obsolete)
Comment 2 Nicolas Legrand 2020-01-13 17:26:18 UTC Comment hidden (obsolete)
Comment 3 Nicolas Legrand 2020-01-13 17:26:21 UTC Comment hidden (obsolete)
Comment 4 Nicolas Legrand 2020-03-24 16:12:00 UTC Comment hidden (obsolete)
Comment 5 Nicolas Legrand 2020-03-24 16:12:03 UTC Comment hidden (obsolete)
Comment 6 Nicolas Legrand 2020-03-24 16:12:06 UTC Comment hidden (obsolete)
Comment 7 Nicolas Legrand 2020-03-24 16:23:42 UTC
Created attachment 101628 [details] [review]
Bug 24412: Schema file DO NOT PUSH!
Comment 8 Nicolas Legrand 2020-03-24 16:23:45 UTC
Created attachment 101629 [details] [review]
Bug 24412: kohastructure and atomic update
Comment 9 Nicolas Legrand 2020-03-24 16:23:49 UTC
Created attachment 101630 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 13881 and then Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.
Comment 10 Nicolas Legrand 2020-03-25 15:21:49 UTC
Created attachment 101721 [details] [review]
Bug 24412: Schema file DO NOT PUSH!
Comment 11 Nicolas Legrand 2020-03-25 15:21:53 UTC
Created attachment 101722 [details] [review]
Bug 24412: kohastructure and atomic update
Comment 12 Nicolas Legrand 2020-03-25 15:21:56 UTC
Created attachment 101723 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 13881 and then Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.
Comment 13 Fridolin Somers 2020-03-26 12:08:33 UTC
Small typo :

 =head2 ModReserveAffect
 
-  &ModReserveAffect($itemnumber,$borrowernumber,$diffBranchSend,$reserve_id);
+  &ModReserveAffect($itemnumber,$borrowernumber,$diffBranchSend,$reserve_id, $desk);

 sub ModReserveAffect {
-    my ( $itemnumber, $borrowernumber, $transferToDo, $reserve_id ) = @_;
+    my ( $itemnumber, $borrowernumber, $transferToDo, $reserve_id, $desk_id ) = @_;

Doc line should say $desk_id instead of $desk right ?
Comment 14 Nicolas Legrand 2020-03-26 13:18:12 UTC
Created attachment 101825 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 13881 and then Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.
Comment 15 Nicolas Legrand 2020-03-26 13:18:52 UTC
(In reply to Fridolin SOMERS from comment #13)
> Small typo :
> 
>  =head2 ModReserveAffect
>  
> - 
> &ModReserveAffect($itemnumber,$borrowernumber,$diffBranchSend,$reserve_id);
> + 
> &ModReserveAffect($itemnumber,$borrowernumber,$diffBranchSend,$reserve_id,
> $desk);
> 
>  sub ModReserveAffect {
> -    my ( $itemnumber, $borrowernumber, $transferToDo, $reserve_id ) = @_;
> +    my ( $itemnumber, $borrowernumber, $transferToDo, $reserve_id, $desk_id
> ) = @_;
> 
> Doc line should say $desk_id instead of $desk right ?

right, thank you Frido!
Comment 16 Nicolas Legrand 2020-03-26 16:01:31 UTC
Created attachment 101884 [details] [review]
Bug 24412: Schema file DO NOT PUSH!
Comment 17 Nicolas Legrand 2020-03-26 16:01:35 UTC
Created attachment 101885 [details] [review]
Bug 24412: kohastructure and atomic update
Comment 18 Nicolas Legrand 2020-03-26 16:01:38 UTC
Created attachment 101886 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 13881 and then Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.
Comment 19 Martin Renvoize 2020-03-26 17:47:11 UTC
Created attachment 101898 [details] [review]
Bug 24412: Schema file DO NOT PUSH!
Comment 20 Martin Renvoize 2020-03-26 17:47:14 UTC
Created attachment 101899 [details] [review]
Bug 24412: kohastructure and atomic update
Comment 21 Martin Renvoize 2020-03-26 17:47:17 UTC
Created attachment 101900 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 13881 and then Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.
Comment 22 Martin Renvoize 2020-03-26 17:47:47 UTC
Fixed up the schema change and kohastructure change.
Comment 23 Martin Renvoize 2020-03-27 08:20:54 UTC
Created attachment 101955 [details] [review]
Bug 24412: Schema file DO NOT PUSH!
Comment 24 Martin Renvoize 2020-03-27 08:20:57 UTC
Created attachment 101956 [details] [review]
Bug 24412: kohastructure and atomic update
Comment 25 Martin Renvoize 2020-03-27 08:21:00 UTC
Created attachment 101957 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 13881 and then Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.
Comment 26 Martin Renvoize 2020-03-27 08:21:56 UTC
(In reply to Martin Renvoize from comment #22)
> Fixed up the schema change and kohastructure change.

Now it's really fixed.. doh!
Comment 27 Josef Moravec 2020-04-23 08:25:32 UTC
Comment on attachment 101956 [details] [review]
Bug 24412: kohastructure and atomic update

Review of attachment 101956 [details] [review]:
-----------------------------------------------------------------

::: installer/data/mysql/atomicupdate/bug_24412_Attach_waiting_reserve_to_desk.perl
@@ +1,4 @@
> +$DBversion = 'XXX';  # will be replaced by the RM
> +if( CheckVersion( $DBversion ) ) {
> +    # you can use $dbh here like:
> +    $dbh->do(q{

there should be check for column existance before adding it, something like:

if ( !column_exists( 'reserves', 'desk_id' ) ) {
    $dbh->do(...);
}

@@ +5,5 @@
> +ALTER TABLE reserves ADD COLUMN desk_id INT(11) DEFAULT NULL AFTER branchcode,
> +  ADD KEY desk_id (`desk_id`),
> +  ADD CONSTRAINT `reserves_ibfk_6` FOREIGN KEY (`desk_id`) REFERENCES `desks` (`desk_id`) ON DELETE CASCADE ON UPDATE CASCADE ;
> +             });
> +    $dbh->do(q{

there should be check for column existance too
Comment 28 Josef Moravec 2020-04-23 08:43:08 UTC
Comment on attachment 101957 [details] [review]
Bug 24412: Attach waiting reserve to desk

Review of attachment 101957 [details] [review]:
-----------------------------------------------------------------

::: Koha/Hold.pm
@@ +173,3 @@
>      };
>  
> +    if (defined $desk_id) { $values->{'desk_id'} = $desk_id };

the desk_id => $desk_id, setting in values above should be omitted, only the one in if should stay

@@ +323,5 @@
> +    my ($self) = @_;
> +
> +    $self->{_desk} ||= Koha::Desks->find( $self->desk_id() );
> +
> +    return $self->{_desk};

You should use relation and _new_from_dbic here, see https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL15:_Object-oriented_code_and_the_Koha::_namespace_.5BPROPOSAL_FOR_CHANGE_dev_meeting_2.2F2020.5D

::: Koha/Template/Plugin/Desks.pm
@@ +47,5 @@
> +
> +sub GetName {
> +    my ( $self, $desk_id ) = @_;
> +    my $d = Koha::Desks->find( $desk_id );
> +    return (defined $d) ? $d->unblessed->{'desk_name'} : q{};

should be something like:

return $d ? $d->desk_name : q{};

::: koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
@@ +120,4 @@
>                  [% IF ( hold.found ) %]
>                      [% IF ( hold.atdestination ) %]
>                          [% IF ( hold.found ) %]
> +                            Item waiting at <b> [% hold.wbrname | html %]</b>[% IF hold.desk_name %], [% hold.desk_name | html %],[% END %] <input type="hidden" name="pickup" value="[% hold.wbrcode | html %]" /> since [% hold.waiting_date | $KohaDates %]

Looks like you  don't need to use Desks template plugin.

::: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
@@ +381,4 @@
>                          [% SET hold = item.first_hold %]
>                          [% IF hold %]
>                              [% IF hold.waitingdate %]
> +                                Waiting at [% Branches.GetName( hold.branchcode ) | html %][% IF ( hold.desk_id ) %], [% Desks.GetName ( hold.desk_id ) | html %][% END %] since [% hold.waitingdate | $KohaDates %].

you don't new Desks template plugin, just use hold.desk.desk_name

::: koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
@@ +468,4 @@
>  
>                                                  <div class="modal-footer">
>                                                      <input type="hidden" name="cancel_reserve" value="0" />
> +                                                       <button type="submit" class="btn btn-default approve" [% IF (Desks.defined) %]data-dismiss="modal" [% END %]>

Will need update as Desks.defined was removed in previous patches on bug 24201

::: koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc
@@ +106,4 @@
>                                  <i class="fa fa-exclamation-circle text-warning"></i>
>                                  [% IF ( HOLD.is_at_destination ) %]
>                                      [% IF ( HOLD.found ) %]
> +                                        Item waiting at <b> [% HOLD.branch.branchname | html %]</b>[% IF ( HOLD.desk_id ) %], [% Desks.GetName ( HOLD.desk_id ) | html %],[% END %]

you don't new Desks template plugin, just use hold.desk.desk_name
Comment 29 Nicolas Legrand 2020-07-06 15:13:31 UTC
Created attachment 106586 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.
Comment 30 Nicolas Legrand 2020-07-06 15:15:14 UTC
This is just a rebase of the failed version :D. Corrected one coming soonishly.
Comment 31 Nicolas Legrand 2020-07-08 08:02:48 UTC
Created attachment 106648 [details] [review]
Bug 24412: (follow-up) QA

Following Josef Moravec QA comments :

- rewrite Koha::Hold->desk according to Object Oriented Koha
Guidelines and use it to fetch desk name in various templates
- remove unused Desks.GetName
- Check for columns existence in db update
Comment 32 Nicolas Legrand 2020-09-14 16:58:24 UTC
Created attachment 110067 [details] [review]
Bug 24412: kohastructure and atomic update
Comment 33 Nicolas Legrand 2020-09-14 16:58:30 UTC
Created attachment 110068 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.
Comment 34 Nicolas Legrand 2020-09-14 16:58:35 UTC
Created attachment 110069 [details] [review]
Bug 24412: (follow-up) QA

Following Josef Moravec QA comments :

- rewrite Koha::Hold->desk according to Object Oriented Koha
Guidelines and use it to fetch desk name in various templates
- remove unused Desks.GetName
- Check for columns existence in db update
Comment 35 Nicolas Legrand 2020-09-14 16:58:43 UTC
Created attachment 110070 [details] [review]
Bug 24412: Schema file DO NOT PUSH!
Comment 36 Nicolas Legrand 2020-09-14 17:00:53 UTC
Rebased!
Comment 37 Martin Renvoize 2020-09-16 13:14:59 UTC
Comment on attachment 110068 [details] [review]
Bug 24412: Attach waiting reserve to desk

Review of attachment 110068 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
@@ +462,4 @@
>                                                      <input type="hidden" name="cancel_reserve" value="0" />
>                                                      <input type="hidden" name="barcode" value="[% barcode | html %]" />
>  
> +                                                       <button type="submit" class="btn btn-default approve" [% IF ( Desks.ListForLibrary ) %]data-dismiss="modal" [% END %]>

Could you explain this change.. I'm not sure I understand it's purpose?
Comment 38 Nicolas Legrand 2020-09-21 08:32:41 UTC
Ho. I feel uncomfortable. When I saw it I thought “how peculiar... But maybe there is a reason anyway...” And I let it... I feel a bit ashamed. I'm going to revert this.
Comment 39 Nicolas Legrand 2020-09-21 11:18:14 UTC
Created attachment 110450 [details] [review]
Bug 24412: (follow-up) QA

useless change. Maybe it was a relic of something usefull... anyway
not anymore.
Comment 40 Nicolas Legrand 2020-09-21 11:19:51 UTC
Created attachment 110451 [details] [review]
Bug 24412: kohastructure and atomic update
Comment 41 Nicolas Legrand 2020-09-21 11:19:57 UTC
Created attachment 110452 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.
Comment 42 Nicolas Legrand 2020-09-21 11:20:02 UTC
Created attachment 110453 [details] [review]
Bug 24412: (follow-up) QA

Following Josef Moravec QA comments :

- rewrite Koha::Hold->desk according to Object Oriented Koha
Guidelines and use it to fetch desk name in various templates
- remove unused Desks.GetName
- Check for columns existence in db update
Comment 43 Nicolas Legrand 2020-09-21 11:20:11 UTC
Created attachment 110454 [details] [review]
Bug 24412: Schema file DO NOT PUSH!
Comment 44 Nicolas Legrand 2020-09-21 11:20:21 UTC
Created attachment 110455 [details] [review]
Bug 24412: (follow-up) QA: useless change

Maybe it was a relic of something usefull... anyway
not anymore.
Comment 45 Martin Renvoize 2020-09-23 09:22:07 UTC
Created attachment 110582 [details] [review]
Bug 24412: kohastructure and atomic update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 46 Martin Renvoize 2020-09-23 09:22:12 UTC
Created attachment 110583 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 47 Martin Renvoize 2020-09-23 09:22:23 UTC
Created attachment 110584 [details] [review]
Bug 24412: (follow-up) QA

Following Josef Moravec QA comments :

- rewrite Koha::Hold->desk according to Object Oriented Koha
Guidelines and use it to fetch desk name in various templates
- remove unused Desks.GetName
- Check for columns existence in db update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 48 Martin Renvoize 2020-09-23 09:22:32 UTC
Created attachment 110585 [details] [review]
Bug 24412: Schema file DO NOT PUSH!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 49 Martin Renvoize 2020-09-23 09:22:36 UTC
Created attachment 110586 [details] [review]
Bug 24412: (follow-up) QA: useless change

Maybe it was a relic of something usefull... anyway
not anymore.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 50 Martin Renvoize 2020-09-23 09:23:45 UTC
Works well, Signing off.

One minor query, would you ever need to alter which desk the item is located at.. either at return time or via another method?
Comment 51 Nicolas Legrand 2020-09-23 13:55:58 UTC
(In reply to Martin Renvoize from comment #50)
> Works well, Signing off.

^^ thanks!

> One minor query, would you ever need to alter which desk the item is located
> at.. either at return time or via another method?

I don't understand?

The behaviour right now is : if you re-check-in a waiting item with another desk in your session, it changes the desk where the reserved item is waiting. This is not a usecase for us, but I find this useful : it's easy to move the book internally.

Unfortunately, while I was looking at this, I found two bugs : (1) if you check in a book waiting at another library, it makes a server internal error. (2) When changing libraries, the menu proposes only one desk and not the entire set.

I'll look at (1) here and (2) in a separate bugzilla...
Comment 52 Martin Renvoize 2020-09-23 14:51:23 UTC
(In reply to Nicolas Legrand from comment #51)
> (In reply to Martin Renvoize from comment #50)
> > Works well, Signing off.
> 
> ^^ thanks!
> 
> > One minor query, would you ever need to alter which desk the item is located
> > at.. either at return time or via another method?
> 
> I don't understand?

I hadn't thought about a second checkin, but I'm glad that works as expected. I suppose I was wondering about whether there was a need to expose the desks as a selectable option at checkin should you want to pass the book to another desk there and then.. but actually I'm not sure there's really a use case for that.

> Unfortunately, while I was looking at this, I found two bugs : (1) if you
> check in a book waiting at another library, it makes a server internal
> error. (2) When changing libraries, the menu proposes only one desk and not
> the entire set.
> 
> I'll look at (1) here and (2) in a separate bugzilla...

Good catches 😊
Comment 53 Katrin Fischer 2020-09-26 20:06:02 UTC
> > Unfortunately, while I was looking at this, I found two bugs : (1) if you
> > check in a book waiting at another library, it makes a server internal
> > error. (2) When changing libraries, the menu proposes only one desk and not
> > the entire set.
> > 
> > I'll look at (1) here and (2) in a separate bugzilla...
> 
> Good catches 😊

Awaiting the follow-up!
Comment 54 Katrin Fischer 2020-09-26 20:06:35 UTC
I am also missing tests?
Comment 55 Nicolas Legrand 2020-10-20 15:24:31 UTC
Created attachment 112046 [details] [review]
Bug 24412: kohastructure and atomic update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 56 Nicolas Legrand 2020-10-20 15:24:35 UTC
Created attachment 112047 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 57 Nicolas Legrand 2020-10-20 15:24:39 UTC
Created attachment 112048 [details] [review]
Bug 24412: (follow-up) QA

Following Josef Moravec QA comments :

- rewrite Koha::Hold->desk according to Object Oriented Koha
Guidelines and use it to fetch desk name in various templates
- remove unused Desks.GetName
- Check for columns existence in db update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 58 Nicolas Legrand 2020-10-20 15:24:44 UTC
Created attachment 112049 [details] [review]
Bug 24412: Schema file DO NOT PUSH!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 59 Nicolas Legrand 2020-10-20 15:24:49 UTC
Created attachment 112050 [details] [review]
Bug 24412: (follow-up) QA: useless change

Maybe it was a relic of something usefull... anyway
not anymore.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 60 Nicolas Legrand 2020-10-20 15:28:47 UTC
this was a big rebase I didn't test (the Koha people worked a lot while I was in my tunnel!). I'll test tomorrow so please don't test until I switch to NEED SIGNOFF :).

Thanks!
Comment 61 Nicolas Legrand 2020-10-21 10:14:28 UTC
Created attachment 112092 [details] [review]
Bug 24412: (follow-up) tests

Add some test for Koha::Hold->desk and Koha::Hold->set_waiting.

Test plan:

    1. prove t/db_dependent/Koha/Holds.t
    2. should be green!
Comment 62 Nicolas Legrand 2020-10-21 12:42:29 UTC
Created attachment 112099 [details] [review]
Bug 24412: (follow-up) qa-tools

fix obvious problems:

    1. qa -c 6 -v 2
    2. should be green
Comment 63 Nicolas Legrand 2020-10-21 12:43:28 UTC
(In reply to Katrin Fischer from comment #54)
> I am also missing tests?

oups... Tests added!
Comment 64 Nicolas Legrand 2020-10-22 06:40:25 UTC
> Unfortunately, while I was looking at this, I found two bugs : (1) if you
> check in a book waiting at another library, it makes a server internal
> error. (2) When changing libraries, the menu proposes only one desk and not
> the entire set.
> 
> I'll look at (1) here and (2) in a separate bugzilla...

I forgot to mention it seems like (1) was fixed by Bug 12556!
Comment 65 Nicolas Legrand 2020-10-22 10:10:19 UTC
> 
> Unfortunately, while I was looking at this, I found two bugs : (1) if you
> check in a book waiting at another library, it makes a server internal
> error. (2) When changing libraries, the menu proposes only one desk and not
> the entire set.
> 
> I'll look at (1) here and (2) in a separate bugzilla...

fix proposed for (2) in Bug 26779
Comment 66 Nicolas Legrand 2020-10-22 11:13:39 UTC
Séverine spotted bugs.
Comment 67 Nicolas Legrand 2020-10-22 11:37:42 UTC
Created attachment 112165 [details] [review]
Bug 24412: kohastructure and atomic update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

https://bugs.koha-community.org/show_bug.cgi?id=26779
Comment 68 Nicolas Legrand 2020-10-22 11:39:47 UTC
Created attachment 112166 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 69 Nicolas Legrand 2020-10-22 11:39:52 UTC
Created attachment 112167 [details] [review]
Bug 24412: (follow-up) QA

Following Josef Moravec QA comments :

- rewrite Koha::Hold->desk according to Object Oriented Koha
Guidelines and use it to fetch desk name in various templates
- remove unused Desks.GetName
- Check for columns existence in db update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 70 Nicolas Legrand 2020-10-22 11:39:58 UTC
Created attachment 112168 [details] [review]
Bug 24412: Schema file DO NOT PUSH!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 71 Nicolas Legrand 2020-10-22 11:40:04 UTC
Created attachment 112169 [details] [review]
Bug 24412: (follow-up) QA: useless change

Maybe it was a relic of something usefull... anyway
not anymore.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 72 Nicolas Legrand 2020-10-22 11:40:10 UTC
Created attachment 112170 [details] [review]
Bug 24412: (follow-up) tests

Add some test for Koha::Hold->desk and Koha::Hold->set_waiting.

Test plan:

    1. prove t/db_dependent/Koha/Holds.t
    2. should be green!
Comment 73 Nicolas Legrand 2020-10-22 11:40:16 UTC
Created attachment 112171 [details] [review]
Bug 24412: (follow-up) qa-tools

fix obvious problems:

    1. qa -c 7 -v 2
    2. should be green
Comment 74 Nicolas Legrand 2020-10-22 11:40:21 UTC
Created attachment 112172 [details] [review]
Bug 24412: (follow-up) bad desk printing

forgot to update two pages when I wrote Hold->desk to replace
Desks.GetName.
Comment 75 Séverine Queune 2020-10-22 16:22:02 UTC
Created attachment 112201 [details] [review]
Bug 24412: (follow-up) prevent error in svc/holds

don't fail if a hold has no desk attached.
Comment 76 Séverine Queune 2020-10-22 16:30:10 UTC
Created attachment 112202 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 77 Séverine Queune 2020-10-22 16:30:16 UTC
Created attachment 112203 [details] [review]
Bug 24412: (follow-up) QA

Following Josef Moravec QA comments :

- rewrite Koha::Hold->desk according to Object Oriented Koha
Guidelines and use it to fetch desk name in various templates
- remove unused Desks.GetName
- Check for columns existence in db update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 78 Séverine Queune 2020-10-22 16:30:24 UTC
Created attachment 112204 [details] [review]
Bug 24412: Schema file DO NOT PUSH!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 79 Séverine Queune 2020-10-22 16:30:29 UTC
Created attachment 112205 [details] [review]
Bug 24412: (follow-up) QA: useless change

Maybe it was a relic of something usefull... anyway
not anymore.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 80 Séverine Queune 2020-10-22 16:30:36 UTC
Created attachment 112206 [details] [review]
Bug 24412: (follow-up) tests

Add some test for Koha::Hold->desk and Koha::Hold->set_waiting.

Test plan:

    1. prove t/db_dependent/Koha/Holds.t
    2. should be green!
Comment 81 Séverine Queune 2020-10-22 16:30:42 UTC
Created attachment 112207 [details] [review]
Bug 24412: (follow-up) qa-tools

fix obvious problems:

    1. qa -c 8 -v 2
    2. should be green
Comment 82 Séverine Queune 2020-10-22 16:30:47 UTC
Created attachment 112208 [details] [review]
Bug 24412: (follow-up) bad desk printing

forgot to update two pages when I wrote Hold->desk to replace
Desks.GetName.
Comment 83 Séverine Queune 2020-10-22 16:30:52 UTC
Created attachment 112209 [details] [review]
Bug 24412: (follow-up) prevent error in svc/holds

don't fail if a hold has no desk attached.
Comment 84 Séverine Queune 2020-10-22 16:32:15 UTC
Created attachment 112210 [details] [review]
Bug 24412: (follow-up) prevent error in svc/holds

don't fail if a hold has no desk attached.
Comment 85 Séverine Queune 2020-10-22 17:17:57 UTC
Created attachment 112211 [details] [review]
Bug 24412: kohastructure and atomic update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

https://bugs.koha-community.org/show_bug.cgi?id=26779
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 86 Séverine Queune 2020-10-22 17:18:03 UTC
Created attachment 112212 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 87 Séverine Queune 2020-10-22 17:18:08 UTC
Created attachment 112213 [details] [review]
Bug 24412: (follow-up) QA

Following Josef Moravec QA comments :

- rewrite Koha::Hold->desk according to Object Oriented Koha
Guidelines and use it to fetch desk name in various templates
- remove unused Desks.GetName
- Check for columns existence in db update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 88 Séverine Queune 2020-10-22 17:18:15 UTC
Created attachment 112214 [details] [review]
Bug 24412: Schema file DO NOT PUSH!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 89 Séverine Queune 2020-10-22 17:18:20 UTC
Created attachment 112215 [details] [review]
Bug 24412: (follow-up) QA: useless change

Maybe it was a relic of something usefull... anyway
not anymore.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 90 Séverine Queune 2020-10-22 17:18:25 UTC
Created attachment 112216 [details] [review]
Bug 24412: (follow-up) tests

Add some test for Koha::Hold->desk and Koha::Hold->set_waiting.

Test plan:

    1. prove t/db_dependent/Koha/Holds.t
    2. should be green!

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 91 Séverine Queune 2020-10-22 17:18:31 UTC
Created attachment 112217 [details] [review]
Bug 24412: (follow-up) qa-tools

fix obvious problems:

    1. qa -c 8 -v 2
    2. should be green

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 92 Séverine Queune 2020-10-22 17:18:36 UTC
Created attachment 112218 [details] [review]
Bug 24412: (follow-up) bad desk printing

forgot to update two pages when I wrote Hold->desk to replace
Desks.GetName.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 93 Séverine Queune 2020-10-22 17:18:42 UTC
Created attachment 112219 [details] [review]
Bug 24412: (follow-up) prevent error in svc/holds

don't fail if a hold has no desk attached.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 94 Martin Renvoize 2020-10-23 13:22:15 UTC
Created attachment 112264 [details] [review]
Bug 24412: kohastructure and atomic update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 95 Martin Renvoize 2020-10-23 13:22:22 UTC
Created attachment 112265 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 96 Martin Renvoize 2020-10-23 13:22:28 UTC
Created attachment 112266 [details] [review]
Bug 24412: (follow-up) QA

Following Josef Moravec QA comments :

- rewrite Koha::Hold->desk according to Object Oriented Koha
Guidelines and use it to fetch desk name in various templates
- remove unused Desks.GetName
- Check for columns existence in db update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 97 Martin Renvoize 2020-10-23 13:22:37 UTC
Created attachment 112267 [details] [review]
Bug 24412: Schema file DO NOT PUSH!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 98 Martin Renvoize 2020-10-23 13:22:48 UTC
Created attachment 112268 [details] [review]
Bug 24412: (follow-up) QA: useless change

Maybe it was a relic of something usefull... anyway
not anymore.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 99 Martin Renvoize 2020-10-23 13:22:54 UTC
Created attachment 112269 [details] [review]
Bug 24412: (follow-up) tests

Add some test for Koha::Hold->desk and Koha::Hold->set_waiting.

Test plan:

    1. prove t/db_dependent/Koha/Holds.t
    2. should be green!

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 100 Martin Renvoize 2020-10-23 13:23:01 UTC
Created attachment 112270 [details] [review]
Bug 24412: (follow-up) qa-tools

fix obvious problems:

    1. qa -c 8 -v 2
    2. should be green

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 101 Martin Renvoize 2020-10-23 13:23:07 UTC
Created attachment 112271 [details] [review]
Bug 24412: (follow-up) bad desk printing

forgot to update two pages when I wrote Hold->desk to replace
Desks.GetName.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 102 Martin Renvoize 2020-10-23 13:23:13 UTC
Created attachment 112272 [details] [review]
Bug 24412: (follow-up) prevent error in svc/holds

don't fail if a hold has no desk attached.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 103 Martin Renvoize 2020-10-23 13:30:23 UTC
I'm pretty much ready to PQA this.. once I have an answer to a couple of minor queries.

1/ Are you sure you want to delete a reserve that's attached to a desk when the desk is deleted.. rather than setting the desk_id to null? This may need a tiny followup ;)

2/ Would there ever be a requirement for desks to be 'required'.. in which case we should probably introduce a pref and check to enforce this. This can be a separate bug.

3/ Would there ever be a case where the librarian may want to override the desk at the time of setting the reserve as waiting?  This could also be in a followup bug.

4/ I'm contemplating asking for a check in the set_waiting that ensures we set a desk that corresponds to the branchcode to ensure we don't get mixed data.

I'm generally really happy with this development though, it's looking great. Good Job!
Comment 104 Martin Renvoize 2020-10-23 13:39:52 UTC
Hmmm... also beware that bug 12556 is PQA so this will likely need a rebase on top of it before it gets pushed :|
Comment 105 Martin Renvoize 2020-10-23 13:43:28 UTC
(In reply to Martin Renvoize from comment #104)
> Hmmm... also beware that bug 12556 is PQA so this will likely need a rebase
> on top of it before it gets pushed :|

Ignore that.. 12556 is already in master.. and this applies fine
Comment 106 Martin Renvoize 2020-10-23 14:50:01 UTC
Created attachment 112292 [details] [review]
Bug 24412: (QA follow-up) Change CASCADE DELETE to SET NULL

I don't believe waiting reserves should be removed if a desk is deleted,
we should instead simply unset the desk.
Comment 107 Martin Renvoize 2020-10-23 14:50:09 UTC
Created attachment 112293 [details] [review]
Bug 24412: (QA follow-up) DBIC Schema Update
Comment 108 Martin Renvoize 2020-10-23 14:54:03 UTC
(In reply to Martin Renvoize from comment #103)
> I'm pretty much ready to PQA this.. once I have an answer to a couple of
> minor queries.
> 
> 1/ Are you sure you want to delete a reserve that's attached to a desk when
> the desk is deleted.. rather than setting the desk_id to null? This may need
> a tiny followup ;)

I resolved this in my own QA follow-up

> 
> 2/ Would there ever be a requirement for desks to be 'required'.. in which
> case we should probably introduce a pref and check to enforce this. This can
> be a separate bug.

I decided this wouldn't be the case.  We need to allow AddReserve to continue passing through no desk for the 'Automatically set holds to waiting' option of 'ReservesNeedReturns'. It might be nice to have the option of using 'Processing' for that case in the future however.

> 
> 3/ Would there ever be a case where the librarian may want to override the
> desk at the time of setting the reserve as waiting?  This could also be in a
> followup bug.
 
Another one for another bug in the future.

> 4/ I'm contemplating asking for a check in the set_waiting that ensures we
> set a desk that corresponds to the branchcode to ensure we don't get mixed
> data.

This is over the top at this time I think.

After all that.. I'm Passing QA!
Comment 109 Jonathan Druart 2020-11-04 14:42:46 UTC
1. POD for Koha::Hold->desk is wrong

2. If not desk set, biblio hold list (reserves/request.pl) explodes with:
Can't call method "desk_name" on an undefined value at /kohadevbox/koha/reserve/request.pl line 661

3. The desk's name is not correctly escaped in the patron's hold list (circ/circulation.pl, "Holds" tab)
Try to use the following string as a desk name:
  <script>alert("❤");</script>
Search for "escapeHtml" in the same js file for the solution.

4. I am not sure about the desk_id parameter we passed to Koha::Hold->set_waiting.
We could also set it from ModReserveAffect, what's the benefit of this approach?

Failing QA for 1, 2, 3.
4 is for discussion.
Comment 110 Martin Renvoize 2020-11-05 08:59:57 UTC
(In reply to Jonathan Druart from comment #109)
> 4. I am not sure about the desk_id parameter we passed to
> Koha::Hold->set_waiting.
> We could also set it from ModReserveAffect, what's the benefit of this
> approach?

I felt it was a good thing having it at the set_waiting level. In part because it encourages code to be written cleanly in the Koha:: space and because I feel it's the set_waiting call that links closely with when a desk would be assigned to a reserve and as such having it in this function now means if we use set_waiting down the line without calling ModReserveAffect then we would be reminded to set desk_id if appropriate.
Comment 111 Nicolas Legrand 2020-11-05 13:19:03 UTC
Created attachment 113108 [details] [review]
Bug 24412 (follow-up): Fix POD

Koha::Desk and not Koha::Library...
Comment 112 Nicolas Legrand 2020-11-05 13:19:07 UTC
Created attachment 113109 [details] [review]
Bug 24412: (follow-up) prevent request.pl from failing

When no desk is defined request.pl returns an internal server
error. Check a desk is defined before asking for reservation deskname.

Plan test:

1. log in with a library with no desk
2. check in a reserved book so it'll be switch to “waiting reserve”
3. go to the book notice and click the Holds tab (request.pl page)
4. boum
5. apply patch, restart plack
6. refresh page
7. now loading properly
Comment 113 Nicolas Legrand 2020-11-05 13:58:43 UTC
Created attachment 113116 [details] [review]
Bug 24412: (follow-up) prevent js injection

Some js variables are not properly escaped and can be executed if
containing javascript.

1. have some waiting reserve attached to a desk
2. change this desk name to : <script>alert("❤");</script>
3. go to user's checkout page (circulation.pl) and click on the
Hold(s) tab
4. you should see some popup with a ❤ in it.
5. apply patch and refresh page
6. now you should see the desk name printed properly in the page:
<script>alert("❤");</script>
Comment 114 Nicolas Legrand 2020-11-05 14:21:44 UTC
(In reply to Jonathan Druart from comment #109)
[...]
> 
> 4. I am not sure about the desk_id parameter we passed to
> Koha::Hold->set_waiting.
> We could also set it from ModReserveAffect, what's the benefit of this
> approach?
> 
> Failing QA for 1, 2, 3.

I tried to address 1, 2, 3, a big thanks for spotting those. Especially 3...

> 4 is for discussion.

I'm a bit ashamed to say I don't really know. It seemed to me logical at the time. I'd say that if you think the day someone rewrite ModReserveAffect as a Koha::Hold method the desk attachment should go in this method, then I'll put it under set_waiting().
Comment 115 Jonathan Druart 2020-11-05 14:28:08 UTC
Thanks Nicolas!
Comment 116 Jonathan Druart 2020-11-06 10:59:08 UTC
Created attachment 113228 [details] [review]
Bug 24412: kohastructure and atomic update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 117 Jonathan Druart 2020-11-06 10:59:13 UTC
Created attachment 113229 [details] [review]
Bug 24412: Attach waiting reserve to desk

When an item is checked in and marked 'Waiting' or already 'Waiting'
and there is a desk attached to the session, the item is marked
waiting at the current desk of the current library.

The information is displayed on the OPAC and on the intranet. The
patron can then know at which desk he can retrieve his document.

Desk Management (Bug 13881) is now useful.

Test plan :

1. apply Bug 24201
2. $KOHA_PATH/installer/data/mysql/updatedatabase.pl
3. Check out some document to someone
4. make another one reserve this document
5. check in the document
6. you can see the document is attach to the current library
7. create some desks and attach one to your session (see Bug 13881 and
Bug 24201)
8. cancel the preceding reserve and redo steps 3 to 5
9. you should see the document is waiting at the current library and
current desk on:
  a. the intranet document request page
  b. the intranet borrower holds tab
  c. the item list where the document is listed on the bibliographic
    details
  d. the borrower's OPAC holds tab.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24412: (follow-up) QA

Following Josef Moravec QA comments :

- rewrite Koha::Hold->desk according to Object Oriented Koha
Guidelines and use it to fetch desk name in various templates
- remove unused Desks.GetName
- Check for columns existence in db update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24412: (follow-up) QA: useless change

Maybe it was a relic of something usefull... anyway
not anymore.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24412: (follow-up) Fix POD

Koha::Desk and not Koha::Library...
Comment 118 Jonathan Druart 2020-11-06 10:59:18 UTC
Created attachment 113230 [details] [review]
Bug 24412: (follow-up) tests

Add some test for Koha::Hold->desk and Koha::Hold->set_waiting.

Test plan:

    1. prove t/db_dependent/Koha/Holds.t
    2. should be green!

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 119 Jonathan Druart 2020-11-06 10:59:23 UTC
Created attachment 113231 [details] [review]
Bug 24412: (follow-up) qa-tools

fix obvious problems:

    1. qa -c 8 -v 2
    2. should be green

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 120 Jonathan Druart 2020-11-06 10:59:28 UTC
Created attachment 113232 [details] [review]
Bug 24412: (follow-up) bad desk printing

forgot to update two pages when I wrote Hold->desk to replace
Desks.GetName.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 121 Jonathan Druart 2020-11-06 10:59:32 UTC
Created attachment 113233 [details] [review]
Bug 24412: (follow-up) prevent error in svc/holds

don't fail if a hold has no desk attached.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 122 Jonathan Druart 2020-11-06 10:59:36 UTC
Created attachment 113234 [details] [review]
Bug 24412: (QA follow-up) Change CASCADE DELETE to SET NULL

I don't believe waiting reserves should be removed if a desk is deleted,
we should instead simply unset the desk.
Comment 123 Jonathan Druart 2020-11-06 10:59:41 UTC
Created attachment 113235 [details] [review]
Bug 24412: (follow-up) prevent request.pl from failing

When no desk is defined request.pl returns an internal server
error. Check a desk is defined before asking for reservation deskname.

Plan test:

1. log in with a library with no desk
2. check in a reserved book so it'll be switch to “waiting reserve”
3. go to the book notice and click the Holds tab (request.pl page)
4. boum
5. apply patch, restart plack
6. refresh page
7. now loading properly
Comment 124 Jonathan Druart 2020-11-06 10:59:45 UTC
Created attachment 113236 [details] [review]
Bug 24412: (follow-up) prevent js injection

Some js variables are not properly escaped and can be executed if
containing javascript.

1. have some waiting reserve attached to a desk
2. change this desk name to : <script>alert("❤");</script>
3. go to user's checkout page (circulation.pl) and click on the
Hold(s) tab
4. you should see some popup with a ❤ in it.
5. apply patch and refresh page
6. now you should see the desk name printed properly in the page:
<script>alert("❤");</script>
Comment 125 Jonathan Druart 2020-11-06 11:00:05 UTC
I've squashed some patches.
Comment 126 Jonathan Druart 2020-11-06 14:55:57 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 127 Nicolas Legrand 2020-11-09 08:26:59 UTC
Thanks a lot everyone! Thank you Séverine for testing, thank you, Martin, Josef and Jonathan for reviewing, questioning, advising and guiding. It's such a pleasure to share time with the Koha community! I'm so eager to meet you all in Marseille as soon as possible! See you all!
Comment 128 Lucas Gass 2020-11-16 22:26:03 UTC
enhancement will not be backported to 20.05.x