Description
Nicole C. Engard
2016-01-06 16:34:08 UTC
Created attachment 88004 [details] [review] Bug 15496: Add API endoint for deleting a bib Created attachment 88005 [details] [review] Bug 15496: Tidy code block (Whitespace changes only) Created attachment 88006 [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 Created attachment 88008 [details] [review] Bug 15496: Add API endoint for deleting a bib Created attachment 88015 [details] [review] Bug 15496: Add API endoint for deleting a bib Created attachment 88016 [details] [review] Bug 15496: Tidy code block (Whitespace changes only) Created attachment 88017 [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 Created attachment 88018 [details] [review] Bug 15496: Add API endoint for deleting a bib Created attachment 88019 [details] [review] Bug 15496: Tidy code block (Whitespace changes only) Created attachment 88020 [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 Created attachment 88026 [details] [review] Bug 15496: Add API endoint for deleting a bib Created attachment 88027 [details] [review] Bug 15496: Tidy code block (Whitespace changes only) Created attachment 88028 [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 Created attachment 88029 [details] [review] Bug 15496: Add API endoint for deleting a bib Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 88030 [details] [review] Bug 15496: Tidy code block (Whitespace changes only) Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 88031 [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> Created attachment 88032 [details] [review] Bug 15496: (QA Follow up) remove text/javascript Make the QA tool happy for html5 Created attachment 88165 [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> Created attachment 88166 [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> Created attachment 88167 [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> Created attachment 88168 [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> Created attachment 88169 [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> Created attachment 88170 [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> Created attachment 88171 [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> Created attachment 88172 [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> 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 ;) Created attachment 88215 [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. 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> 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> 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> 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> 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> 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> Great work! 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> 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> 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> 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> 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> 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> Created attachment 88903 [details] [review] Bug 15496: (QA Followup) Fix new uses of Koha::Biblio::items in list context Created attachment 88905 [details] [review] Bug 15496: (QA follow-up) Fix new uses of Koha::Biblio::items in list context Awesome work all! Pushed to master for 19.05 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. 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> (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. Dependants not in 18.11.x series.. will not be backported at this time. 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> 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> Final follow-up tested.. working solidly for me, thanks Tomas! (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 *** Bug 9538 has been marked as a duplicate of this bug. *** *** Bug 14656 has been marked as a duplicate of this bug. *** Is this the first introduction of the REST API in regular code btw? (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? (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 (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 From bug 24232 comment 8: Note that the message is meaningless: "The record has X attached items" is only displayed when X=0. |