Bug 15496

Summary: Delete bibliographic record after moving last item to another record(s)
Product: Koha Reporter: Nicole C. Engard <nengard>
Component: CatalogingAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Josef Moravec <josef.moravec>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, josef.moravec, katrin.fischer, m.de.rooy, martin.renvoize, mathsabypro, nick, oleonard, olli-antti.kivilahti, tomascohen, wizzyrea
Version: master   
Hardware: All   
OS: All   
URL: http://devs.bywatersolutions.com/projects/323/
Change sponsored?: Seeking cosponsors Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
When transferring items from one record to another you are now presented with a button to delete the original host record if there are no more items remaining.
Version(s) released in:
19.05.00
Bug Depends on:    
Bug Blocks: 22284, 24232, 33250    
Attachments: Bug 15496: Add API endoint for deleting a bib
Bug 15496: Tidy code block (Whitespace changes only)
Bug 15496: Delete Bib After Moving Item
Bug 15496: Add API endoint for deleting a bib
Bug 15496: Add API endoint for deleting a bib
Bug 15496: Tidy code block (Whitespace changes only)
Bug 15496: Delete Bib After Moving Item
Bug 15496: Add API endoint for deleting a bib
Bug 15496: Tidy code block (Whitespace changes only)
Bug 15496: Delete Bib After Moving Item
Bug 15496: Add API endoint for deleting a bib
Bug 15496: Tidy code block (Whitespace changes only)
Bug 15496: Delete Bib After Moving Item
Bug 15496: Add API endoint for deleting a bib
Bug 15496: Tidy code block (Whitespace changes only)
Bug 15496: Delete Bib After Moving Item
Bug 15496: (QA Follow up) remove text/javascript
Bug 15496: Add API endoint for deleting a bib
Bug 15496: Tidy code block (Whitespace changes only)
Bug 15496: Delete Bib After Moving Item
Bug 15496: Add API endoint for deleting a bib
Bug 15496: Tidy code block (Whitespace changes only)
Bug 15496: Delete Bib After Moving Item
Bug 15496: (QA Follow up) remove text/javascript
Bug 15496: (QA follow-up) Change success status on api
Bug 15496: (follow-up) Update interaction for deleting records
Bug 15496: Add API endoint for deleting a bib
Bug 15496: Tidy code block (Whitespace changes only)
Bug 15496: Delete Bib After Moving Item
Bug 15496: (QA Follow up) remove text/javascript
Bug 15496: (QA follow-up) Change success status on api
Bug 15496: (follow-up) Update interaction for deleting records
Bug 15496: Add API endoint for deleting a bib
Bug 15496: Tidy code block (Whitespace changes only)
Bug 15496: Delete Bib After Moving Item
Bug 15496: (QA Follow up) remove text/javascript
Bug 15496: (QA follow-up) Change success status on api
Bug 15496: (follow-up) Update interaction for deleting records
Bug 15496: (QA Followup) Fix new uses of Koha::Biblio::items in list context
Bug 15496: (QA follow-up) Fix new uses of Koha::Biblio::items in list context
Bug 15496: (QA follow-up) Restore Koha::Biblio->items broken by rebase
Bug 15496: (QA follow-up) Fix remaining cases
Bug 15496: (QA follow-up) Fix remaining cases

Description Nicole C. Engard 2016-01-06 16:34:08 UTC
Expected outcome of development:

* After item is moved from Record A to Record B, check to see if any items remain on Record A

* If no items remain on Record A, add a new button to the move item confirmation screen for “Delete record item was moved from”

* Clicking the button will spawn an ajax process to delete the record

* The button will be replaced with a confirmation of the success or failure of the record deletion

* The librarian will then be able to use the “OK” or “Attach another item” buttons as usual.


Seeking Co-sponsors: http://devs.bywatersolutions.com/projects/323/
Comment 1 Kyle M Hall 2019-04-15 14:22:28 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2019-04-15 14:22:38 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2019-04-15 14:22:41 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2019-04-15 14:44:10 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2019-04-15 14:54:37 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2019-04-15 14:54:49 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall 2019-04-15 14:54:51 UTC Comment hidden (obsolete)
Comment 8 Kyle M Hall 2019-04-15 15:07:17 UTC Comment hidden (obsolete)
Comment 9 Kyle M Hall 2019-04-15 15:08:04 UTC Comment hidden (obsolete)
Comment 10 Kyle M Hall 2019-04-15 15:08:07 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall 2019-04-15 17:45:01 UTC Comment hidden (obsolete)
Comment 12 Kyle M Hall 2019-04-15 17:45:09 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2019-04-15 17:45:12 UTC Comment hidden (obsolete)
Comment 14 Liz Rea 2019-04-15 18:01:37 UTC Comment hidden (obsolete)
Comment 15 Liz Rea 2019-04-15 18:01:40 UTC Comment hidden (obsolete)
Comment 16 Liz Rea 2019-04-15 18:01:43 UTC Comment hidden (obsolete)
Comment 17 Liz Rea 2019-04-15 18:01:46 UTC Comment hidden (obsolete)
Comment 18 Josef Moravec 2019-04-17 10:49:35 UTC Comment hidden (obsolete)
Comment 19 Josef Moravec 2019-04-17 10:49:38 UTC Comment hidden (obsolete)
Comment 20 Josef Moravec 2019-04-17 10:49:42 UTC Comment hidden (obsolete)
Comment 21 Josef Moravec 2019-04-17 10:50:01 UTC Comment hidden (obsolete)
Comment 22 Josef Moravec 2019-04-17 10:50:04 UTC Comment hidden (obsolete)
Comment 23 Josef Moravec 2019-04-17 10:50:07 UTC Comment hidden (obsolete)
Comment 24 Josef Moravec 2019-04-17 10:50:11 UTC Comment hidden (obsolete)
Comment 25 Josef Moravec 2019-04-17 10:50:15 UTC Comment hidden (obsolete)
Comment 26 Josef Moravec 2019-04-17 10:53:00 UTC
Looking good, passes QA tools, I just changed the 200 response code to 204, as needed by our API coding guidelines.

Only one thing could be improved - I would like to see some styling on response message in UI.

(span#del-bib-success, span#del-bib-failure selectors)

I am adding Owen, he could have some idea I hope ;)
Comment 27 Owen Leonard 2019-04-17 16:39:12 UTC Comment hidden (obsolete)
Comment 28 Tomás Cohen Arazi 2019-04-17 18:34:19 UTC
Created attachment 88225 [details] [review]
Bug 15496: Add API endoint for deleting a bib

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 29 Tomás Cohen Arazi 2019-04-17 18:34:23 UTC
Created attachment 88226 [details] [review]
Bug 15496: Tidy code block (Whitespace changes only)

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 30 Tomás Cohen Arazi 2019-04-17 18:34:26 UTC
Created attachment 88227 [details] [review]
Bug 15496: Delete Bib After Moving Item

Librarians have requested the ability to delete records with no items
quickly and easily from the "Attach item" results page as items are
moved from one record to another.

Test Plan:
1) Apply this patch
2) Create 2 records with 2 items each
3) Move one item from record A to record B
4) You should see no change from pre-patch behavior
5) Move the second item from record A to record B
6) You should now see a "Delete record" button
7) Click the button
8) Note the record in question has been deleted

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 31 Tomás Cohen Arazi 2019-04-17 18:34:30 UTC
Created attachment 88228 [details] [review]
Bug 15496: (QA Follow up) remove text/javascript

Make the QA tool happy for html5

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 32 Tomás Cohen Arazi 2019-04-17 18:34:34 UTC
Created attachment 88229 [details] [review]
Bug 15496: (QA follow-up) Change success status on api

DELETE response code must be 204, not 200 according to our API coding
guidelines

Test plan:

prove t/db_dependent/api/v1/biblios.t

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 33 Tomás Cohen Arazi 2019-04-17 18:34:37 UTC
Created attachment 88230 [details] [review]
Bug 15496: (follow-up) Update interaction for deleting records

This patch makes minor changes to markup and JavaScript to alter the
interaction for deleting the record. Now a successful deletion will
convert the dialog to a "message" type dialog and show only the success
message.

To test, apply the patch and test the process of attaching the last item
from a record and deleting that record.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 34 Tomás Cohen Arazi 2019-04-17 18:36:13 UTC
Great work!
Comment 35 Kyle M Hall 2019-04-26 11:22:22 UTC
Created attachment 88892 [details] [review]
Bug 15496: Add API endoint for deleting a bib

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 36 Kyle M Hall 2019-04-26 11:22:36 UTC
Created attachment 88893 [details] [review]
Bug 15496: Tidy code block (Whitespace changes only)

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 37 Kyle M Hall 2019-04-26 11:22:41 UTC
Created attachment 88894 [details] [review]
Bug 15496: Delete Bib After Moving Item

Librarians have requested the ability to delete records with no items
quickly and easily from the "Attach item" results page as items are
moved from one record to another.

Test Plan:
1) Apply this patch
2) Create 2 records with 2 items each
3) Move one item from record A to record B
4) You should see no change from pre-patch behavior
5) Move the second item from record A to record B
6) You should now see a "Delete record" button
7) Click the button
8) Note the record in question has been deleted

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 38 Kyle M Hall 2019-04-26 11:22:44 UTC
Created attachment 88895 [details] [review]
Bug 15496: (QA Follow up) remove text/javascript

Make the QA tool happy for html5

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 39 Kyle M Hall 2019-04-26 11:22:48 UTC
Created attachment 88896 [details] [review]
Bug 15496: (QA follow-up) Change success status on api

DELETE response code must be 204, not 200 according to our API coding
guidelines

Test plan:

prove t/db_dependent/api/v1/biblios.t

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 40 Kyle M Hall 2019-04-26 11:22:51 UTC
Created attachment 88897 [details] [review]
Bug 15496: (follow-up) Update interaction for deleting records

This patch makes minor changes to markup and JavaScript to alter the
interaction for deleting the record. Now a successful deletion will
convert the dialog to a "message" type dialog and show only the success
message.

To test, apply the patch and test the process of attaching the last item
from a record and deleting that record.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 41 Kyle M Hall 2019-04-26 11:44:58 UTC
Created attachment 88903 [details] [review]
Bug 15496: (QA Followup) Fix new uses of Koha::Biblio::items in list context
Comment 42 Kyle M Hall 2019-04-26 12:11:13 UTC
Created attachment 88905 [details] [review]
Bug 15496: (QA follow-up) Fix new uses of Koha::Biblio::items in list context
Comment 43 Nick Clemens 2019-04-26 12:40:37 UTC
Awesome work all!

Pushed to master for 19.05
Comment 44 Tomás Cohen Arazi 2019-04-26 13:06:01 UTC
Comment on attachment 88894 [details] [review]
Bug 15496: Delete Bib After Moving Item


> =head3 items
> 
>-my @items = $biblio->items();
> my $items = $biblio->items();
> 
>-Returns the related Koha::Items object for this biblio in scalar context,
>-or list of Koha::Item objects in list context.
>+Returns the related Koha::Items object for this biblio
> 
> =cut
> 
>@@ -363,10 +361,7 @@ sub items {
> 
>     my $items_rs = $self->_result->items;
> 
>-    return
>-        wantarray
>-        ? Koha::Items->_new_from_dbic($items_rs)->as_list
>-        : Koha::Items->_new_from_dbic($items_rs);
>+    return Koha::Items->_new_from_dbic( $items_rs );
> }

This was a bad rebase. We should revert the last follow-up, and fix this. I can provide such a follow-up.
Comment 45 Tomás Cohen Arazi 2019-04-26 13:20:45 UTC
Created attachment 88920 [details] [review]
Bug 15496: (QA follow-up) Restore Koha::Biblio->items broken by rebase

This patch restores the implementation introduced by 22701.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 46 Tomás Cohen Arazi 2019-04-26 13:21:44 UTC
(In reply to Kyle M Hall from comment #42)
> Created attachment 88905 [details] [review] [review]
> Bug 15496: (QA follow-up) Fix new uses of Koha::Biblio::items in list context

Work to be done, revert this one, and apply the last follow-up.
Comment 47 Martin Renvoize 2019-04-26 15:58:45 UTC
Dependants not in 18.11.x series.. will not be backported at this time.
Comment 48 Tomás Cohen Arazi 2019-04-26 17:10:47 UTC
Created attachment 88938 [details] [review]
Bug 15496: (QA follow-up) Fix remaining cases

This patch fixes remaining cases of the list => scalar change.

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/Holds/HoldFulfillmentPolicy.t t/db_dependent/Reserves.t
=> FAIL: Tests fail
- Apply this patch
- Run:
 k$ prove t/db_dependent/Holds/HoldFulfillmentPolicy.t t/db_dependent/Reserves.t
=> SUCCESS: Tests pass
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 49 Martin Renvoize 2019-04-26 17:45:23 UTC
Created attachment 88950 [details] [review]
Bug 15496: (QA follow-up) Fix remaining cases

This patch fixes remaining cases of the list => scalar change.

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/Holds/HoldFulfillmentPolicy.t t/db_dependent/Reserves.t
=> FAIL: Tests fail
- Apply this patch
- Run:
 k$ prove t/db_dependent/Holds/HoldFulfillmentPolicy.t t/db_dependent/Reserves.t
=> SUCCESS: Tests pass
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 50 Martin Renvoize 2019-04-26 17:46:03 UTC
Final follow-up tested.. working solidly for me, thanks Tomas!
Comment 51 Nick Clemens 2019-04-26 17:54:08 UTC
(In reply to Martin Renvoize from comment #50)
> Final follow-up tested.. working solidly for me, thanks Tomas!

Final follow-up pushed to master for 19.05
Comment 52 Katrin Fischer 2019-04-28 08:03:50 UTC
*** Bug 9538 has been marked as a duplicate of this bug. ***
Comment 53 Tomás Cohen Arazi 2019-05-07 19:24:23 UTC
*** Bug 14656 has been marked as a duplicate of this bug. ***
Comment 54 Marcel de Rooy 2019-10-04 09:27:56 UTC Comment hidden (obsolete)
Comment 55 Marcel de Rooy 2019-10-04 09:28:49 UTC Comment hidden (obsolete)
Comment 56 Katrin Fischer 2019-10-05 07:38:38 UTC
(In reply to Marcel de Rooy from comment #55)
> (In reply to Marcel de Rooy from comment #54)
> > Is this the first introduction of the REST API in regular code btw?
> 
> Or at least at intranet side?

I don't think it is - I remember the patch for changing the pickup location on the staff side as the first: Bug 8030
Comment 57 Marcel de Rooy 2019-10-07 09:45:00 UTC
(In reply to Katrin Fischer from comment #56)
> (In reply to Marcel de Rooy from comment #55)
> > (In reply to Marcel de Rooy from comment #54)
> > > Is this the first introduction of the REST API in regular code btw?
> > 
> > Or at least at intranet side?
> 
> I don't think it is - I remember the patch for changing the pickup location
> on the staff side as the first: Bug 8030

Thanks you're right
Comment 58 Jonathan Druart 2019-12-18 08:54:21 UTC
From bug 24232 comment 8:

Note that the message is meaningless: "The record has X attached items" is only displayed when X=0.