Bug 14544 - Move the list related code to Koha::Virtualshelves
Summary: Move the list related code to Koha::Virtualshelves
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on: 13019
Blocks: 15449 17886 15394 15453 15584 16143 16151 16517 16695 17185 17316
  Show dependency treegraph
 
Reported: 2015-07-16 17:07 UTC by Jonathan Druart
Modified: 2017-03-17 14:29 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Large patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14544: Add Koha::Virtualshelf and Koha::Virtualshelves (2.94 KB, patch)
2015-07-16 17:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelf (17.07 KB, patch)
2015-07-16 17:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of AddShelf (31.20 KB, patch)
2015-07-16 17:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of DelShelf (4.36 KB, patch)
2015-07-16 17:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of ModShelf (4.58 KB, patch)
2015-07-16 17:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Move share routines to Koha::Virtualshelfshare and Koha::Virtualshelfshares (22.23 KB, patch)
2015-07-16 17:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetAllShelves (13.04 KB, patch)
2015-07-16 17:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Koha::Virtualshelfcontent[s] (21.67 KB, patch)
2015-07-16 17:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
[DO NOT APPLY] Bug 14544: WIP - Get rid of GetShelves (6.65 KB, patch)
2015-07-16 17:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Add Koha::Virtualshelf and Koha::Virtualshelves (2.94 KB, patch)
2015-08-06 14:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelf (17.07 KB, patch)
2015-08-06 14:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of AddShelf (31.20 KB, patch)
2015-08-06 14:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of DelShelf (4.36 KB, patch)
2015-08-06 14:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of ModShelf (4.58 KB, patch)
2015-08-06 14:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Move share routines to Koha::Virtualshelfshare and Koha::Virtualshelfshares (22.24 KB, patch)
2015-08-06 14:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetAllShelves (13.04 KB, patch)
2015-08-06 14:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Koha::Virtualshelfcontent[s] (27.62 KB, patch)
2015-08-06 14:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Make the intranet side independent of Page.pm (47.75 KB, patch)
2015-08-06 14:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetBibliosShelves (3.14 KB, patch)
2015-08-06 14:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Add Koha::Virtualshelf and Koha::Virtualshelves (5.05 KB, patch)
2015-08-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelf (17.07 KB, patch)
2015-08-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of AddShelf (31.23 KB, patch)
2015-08-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of DelShelf (4.36 KB, patch)
2015-08-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of ModShelf (4.58 KB, patch)
2015-08-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Move share routines to Koha::Virtualshelfshare and Koha::Virtualshelfshares (22.44 KB, patch)
2015-08-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetAllShelves (13.04 KB, patch)
2015-08-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Koha::Virtualshelfcontent[s] (27.67 KB, patch)
2015-08-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Make the intranet side independent of Page.pm (68.66 KB, patch)
2015-08-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetBibliosShelves (6.72 KB, patch)
2015-08-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelves (9.87 KB, patch)
2015-08-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Make the OPAC side independent of Page.pm (103.35 KB, patch)
2015-08-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of ShelfPossibleAction (10.03 KB, patch)
2015-08-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelfContent (17.67 KB, patch)
2015-08-18 13:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetSomeShelfNames (36.70 KB, patch)
2015-08-18 13:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Move HandleDelBorrower to C4::Members (3.56 KB, patch)
2015-08-18 13:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of C4::VirtualShelves and C4::VirtualShelves::Page (35.78 KB, patch)
2015-08-18 13:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Remove wrong tips about permissions (2.05 KB, patch)
2015-08-18 13:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Add Koha::Virtualshelf and Koha::Virtualshelves (5.05 KB, patch)
2015-08-20 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelf (17.07 KB, patch)
2015-08-20 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of AddShelf (31.23 KB, patch)
2015-08-20 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of DelShelf (4.36 KB, patch)
2015-08-20 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of ModShelf (4.58 KB, patch)
2015-08-20 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Move share routines to Koha::Virtualshelfshare and Koha::Virtualshelfshares (22.44 KB, patch)
2015-08-20 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetAllShelves (13.04 KB, patch)
2015-08-20 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Koha::Virtualshelfcontent[s] (27.67 KB, patch)
2015-08-20 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Make the intranet side independent of Page.pm (69.13 KB, patch)
2015-08-20 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetBibliosShelves (6.72 KB, patch)
2015-08-20 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelves (9.87 KB, patch)
2015-08-20 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Make the OPAC side independent of Page.pm (103.35 KB, patch)
2015-08-20 13:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of ShelfPossibleAction (10.03 KB, patch)
2015-08-20 13:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelfContent (17.67 KB, patch)
2015-08-20 13:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetSomeShelfNames (36.70 KB, patch)
2015-08-20 13:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Move HandleDelBorrower to C4::Members (3.56 KB, patch)
2015-08-20 13:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of C4::VirtualShelves and C4::VirtualShelves::Page (35.78 KB, patch)
2015-08-20 13:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Remove wrong tips about permissions (2.05 KB, patch)
2015-08-20 13:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: (follow-up) Get rid of ShelfPossibleAction (869 bytes, patch)
2015-08-21 10:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix several minor fixes at the OPAC (4.41 KB, patch)
2015-08-25 12:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix several minor bugs at the OPAC (4.41 KB, patch)
2015-08-25 12:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix several minor bugs at the OPAC (5.03 KB, patch)
2015-08-25 13:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix several minor bugs at the OPAC (5.31 KB, patch)
2015-08-25 13:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Do not use shelf as a variable loop (2.49 KB, patch)
2015-08-25 13:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Do not use shelf as a variable loop (18.87 KB, patch)
2015-08-25 14:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix redirect on editing a list (2.13 KB, patch)
2015-08-25 14:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Cosmetic: › should be a class divider (1.91 KB, patch)
2015-08-25 14:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix regression: checkboxes'values are empty (1.49 KB, patch)
2015-08-25 14:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Pass the itemtype description and notforloan to the template (2.60 KB, patch)
2015-08-25 14:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Allow a user to delete his own contents (1.92 KB, patch)
2015-08-25 15:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Remove warnings in tests (1.09 KB, patch)
2015-08-25 15:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Add Koha::Virtualshelf and Koha::Virtualshelves (5.05 KB, patch)
2015-08-26 12:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelf (17.07 KB, patch)
2015-08-26 12:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of AddShelf (31.23 KB, patch)
2015-08-26 12:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of DelShelf (4.36 KB, patch)
2015-08-26 12:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of ModShelf (4.58 KB, patch)
2015-08-26 12:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Move share routines to Koha::Virtualshelfshare and Koha::Virtualshelfshares (22.44 KB, patch)
2015-08-26 12:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetAllShelves (13.86 KB, patch)
2015-08-26 12:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Koha::Virtualshelfcontent[s] (27.82 KB, patch)
2015-08-26 12:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Make the intranet side independent of Page.pm (69.13 KB, patch)
2015-08-26 12:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetBibliosShelves (6.69 KB, patch)
2015-08-26 12:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelves (9.87 KB, patch)
2015-08-26 12:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Make the OPAC side independent of Page.pm (108.02 KB, patch)
2015-08-26 12:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of ShelfPossibleAction (10.06 KB, patch)
2015-08-26 12:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelfContent (23.66 KB, patch)
2015-08-26 12:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetSomeShelfNames (36.98 KB, patch)
2015-08-26 12:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Move HandleDelBorrower to C4::Members (3.56 KB, patch)
2015-08-26 12:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of C4::VirtualShelves and C4::VirtualShelves::Page (35.78 KB, patch)
2015-08-26 12:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Remove wrong tips about permissions (2.07 KB, patch)
2015-08-26 12:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Make the intranet side independent of Page.pm (69.13 KB, patch)
2015-08-26 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetBibliosShelves (6.69 KB, patch)
2015-08-26 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelves (9.87 KB, patch)
2015-08-26 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Make the OPAC side independent of Page.pm (108.03 KB, patch)
2015-08-26 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of ShelfPossibleAction (10.06 KB, patch)
2015-08-26 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelfContent (23.66 KB, patch)
2015-08-26 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetSomeShelfNames (36.98 KB, patch)
2015-08-26 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Move HandleDelBorrower to C4::Members (3.56 KB, patch)
2015-08-26 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of C4::VirtualShelves and C4::VirtualShelves::Page (35.78 KB, patch)
2015-08-26 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Remove wrong tips about permissions (2.07 KB, patch)
2015-08-26 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix regression on adding a new list (4.91 KB, patch)
2015-08-26 13:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix regression on adding a new list (5.33 KB, patch)
2015-08-26 13:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: DB Changes: Set a default value for sortfield (2.12 KB, patch)
2015-08-26 13:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544 - Handle database error when creating a shelf with DB admin account (2.32 KB, patch)
2015-09-02 08:11 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Add Koha::Virtualshelf and Koha::Virtualshelves (5.11 KB, patch)
2015-09-02 12:34 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelf (17.14 KB, patch)
2015-09-02 12:34 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Get rid of AddShelf (31.29 KB, patch)
2015-09-02 12:34 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Get rid of DelShelf (4.42 KB, patch)
2015-09-02 12:34 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Get rid of ModShelf (4.64 KB, patch)
2015-09-02 12:35 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Move share routines to Koha::Virtualshelfshare and Koha::Virtualshelfshares (22.51 KB, patch)
2015-09-02 12:35 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Get rid of GetAllShelves (13.92 KB, patch)
2015-09-02 12:35 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Koha::Virtualshelfcontent[s] (27.88 KB, patch)
2015-09-02 12:35 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Make the intranet side independent of Page.pm (69.20 KB, patch)
2015-09-02 12:35 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Get rid of GetBibliosShelves (6.75 KB, patch)
2015-09-02 12:35 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelves (9.93 KB, patch)
2015-09-02 12:35 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Make the OPAC side independent of Page.pm (108.10 KB, patch)
2015-09-02 12:35 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Get rid of ShelfPossibleAction (10.12 KB, patch)
2015-09-02 12:36 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelfContent (23.70 KB, patch)
2015-09-02 12:36 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Get rid of GetSomeShelfNames (37.05 KB, patch)
2015-09-02 12:36 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Move HandleDelBorrower to C4::Members (3.62 KB, patch)
2015-09-02 12:36 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Get rid of C4::VirtualShelves and C4::VirtualShelves::Page (35.86 KB, patch)
2015-09-02 12:36 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Remove wrong tips about permissions (2.13 KB, patch)
2015-09-02 12:36 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: Fix regression on adding a new list (5.39 KB, patch)
2015-09-02 12:36 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544: DB Changes: Set a default value for sortfield (2.18 KB, patch)
2015-09-02 12:36 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544 - Handle database error when creating a shelf with DB admin account (2.32 KB, patch)
2015-09-02 12:36 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 14544 - Handle database error when creating a shelf with DB admin account (2.51 KB, patch)
2015-09-17 08:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Add Koha::Virtualshelf and Koha::Virtualshelves (5.11 KB, patch)
2015-10-08 09:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelf (17.12 KB, patch)
2015-10-08 09:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of AddShelf (31.28 KB, patch)
2015-10-08 09:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of DelShelf (4.41 KB, patch)
2015-10-08 09:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of ModShelf (4.64 KB, patch)
2015-10-08 09:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Move share routines to Koha::Virtualshelfshare and Koha::Virtualshelfshares (22.50 KB, patch)
2015-10-08 09:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetAllShelves (13.92 KB, patch)
2015-10-08 09:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Koha::Virtualshelfcontent[s] (27.87 KB, patch)
2015-10-08 09:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Make the intranet side independent of Page.pm (69.18 KB, patch)
2015-10-08 09:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetBibliosShelves (6.75 KB, patch)
2015-10-08 09:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelves (9.93 KB, patch)
2015-10-08 09:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Make the OPAC side independent of Page.pm (108.08 KB, patch)
2015-10-08 09:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of ShelfPossibleAction (10.12 KB, patch)
2015-10-08 09:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelfContent (23.69 KB, patch)
2015-10-08 09:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetSomeShelfNames (37.04 KB, patch)
2015-10-08 09:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Move HandleDelBorrower to C4::Members (3.61 KB, patch)
2015-10-08 09:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of C4::VirtualShelves and C4::VirtualShelves::Page (35.84 KB, patch)
2015-10-08 09:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Remove wrong tips about permissions (2.12 KB, patch)
2015-10-08 09:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix regression on adding a new list (5.39 KB, patch)
2015-10-08 09:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: DB Changes: Set a default value for sortfield (2.17 KB, patch)
2015-10-08 09:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544 - Handle database error when creating a shelf with DB admin account (2.51 KB, patch)
2015-10-08 09:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: [QA Follow-up] Fix warning about transaction (1.18 KB, patch)
2015-10-09 11:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Add missing biblio <=> biblioitems <=> items relationships to the schema (1.54 KB, patch)
2015-10-12 09:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: QA fixes - some minor bug fixes (12.61 KB, patch)
2015-10-12 09:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: allow_delete_own == 0 could represent a read-only list (5.94 KB, patch)
2015-10-14 14:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Add Koha::Virtualshelf and Koha::Virtualshelves (5.21 KB, patch)
2015-10-16 08:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelf (17.23 KB, patch)
2015-10-16 08:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Get rid of AddShelf (31.38 KB, patch)
2015-10-16 08:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Get rid of DelShelf (4.51 KB, patch)
2015-10-16 08:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Get rid of ModShelf (4.74 KB, patch)
2015-10-16 08:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Move share routines to Koha::Virtualshelfshare and Koha::Virtualshelfshares (22.60 KB, patch)
2015-10-16 08:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Get rid of GetAllShelves (14.02 KB, patch)
2015-10-16 08:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Koha::Virtualshelfcontent[s] (27.98 KB, patch)
2015-10-16 08:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Make the intranet side independent of Page.pm (69.30 KB, patch)
2015-10-16 08:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Get rid of GetBibliosShelves (6.85 KB, patch)
2015-10-16 08:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelves (10.03 KB, patch)
2015-10-16 08:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Make the OPAC side independent of Page.pm (108.19 KB, patch)
2015-10-16 08:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Get rid of ShelfPossibleAction (10.22 KB, patch)
2015-10-16 08:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelfContent (23.80 KB, patch)
2015-10-16 08:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Get rid of GetSomeShelfNames (37.15 KB, patch)
2015-10-16 08:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Move HandleDelBorrower to C4::Members (3.72 KB, patch)
2015-10-16 08:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Get rid of C4::VirtualShelves and C4::VirtualShelves::Page (35.96 KB, patch)
2015-10-16 08:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Remove wrong tips about permissions (2.23 KB, patch)
2015-10-16 08:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Fix regression on adding a new list (5.49 KB, patch)
2015-10-16 08:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: DB Changes: Set a default value for sortfield (2.27 KB, patch)
2015-10-16 08:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544 - Handle database error when creating a shelf with DB admin account (2.62 KB, patch)
2015-10-16 08:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: [QA Follow-up] Fix warning about transaction (1.18 KB, patch)
2015-10-16 08:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Add missing biblio <=> biblioitems <=> items relationships to the schema (1.67 KB, patch)
2015-10-16 08:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: QA fixes - some minor bug fixes (12.72 KB, patch)
2015-10-16 08:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: [QA Follow-up] Added some test descriptions (4.51 KB, patch)
2015-10-16 08:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: [QA Follow-up] DBIx::Class changes (1.85 KB, patch)
2015-10-16 08:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14544: Fix 'back to lists' link (759 bytes, patch)
2015-10-16 09:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fixes regression: display author and dateadded on the list view [intranet] (1.13 KB, patch)
2015-10-16 10:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix 'Back to lists' links on sharing a shelf (1.78 KB, patch)
2015-10-16 10:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Make the intranet side independent of Page.pm (69.00 KB, patch)
2015-11-05 11:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetBibliosShelves (6.81 KB, patch)
2015-11-05 11:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelves (9.98 KB, patch)
2015-11-05 11:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Make the OPAC side independent of Page.pm (108.16 KB, patch)
2015-11-05 11:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of ShelfPossibleAction (10.17 KB, patch)
2015-11-05 11:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetShelfContent (23.75 KB, patch)
2015-11-05 11:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of GetSomeShelfNames (37.10 KB, patch)
2015-11-05 11:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Move HandleDelBorrower to C4::Members (3.67 KB, patch)
2015-11-05 11:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Get rid of C4::VirtualShelves and C4::VirtualShelves::Page (35.90 KB, patch)
2015-11-05 11:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Remove wrong tips about permissions (2.18 KB, patch)
2015-11-05 11:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix regression on adding a new list (5.44 KB, patch)
2015-11-05 11:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: DB Changes: Set a default value for sortfield (2.23 KB, patch)
2015-11-05 11:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544 - Handle database error when creating a shelf with DB admin account (2.57 KB, patch)
2015-11-05 11:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: [QA Follow-up] Fix warning about transaction (1.13 KB, patch)
2015-11-05 11:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Add missing biblio <=> biblioitems <=> items relationships to the schema (1.62 KB, patch)
2015-11-05 11:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: QA fixes - some minor bug fixes (12.67 KB, patch)
2015-11-05 11:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: [QA Follow-up] Added some test descriptions (4.46 KB, patch)
2015-11-05 11:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: [QA Follow-up] DBIx::Class changes (1.80 KB, patch)
2015-11-05 11:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix 'back to lists' link (759 bytes, patch)
2015-11-05 11:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fixes regression: display author and dateadded on the list view [intranet] (1.13 KB, patch)
2015-11-05 11:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix 'Back to lists' links on sharing a shelf (1.78 KB, patch)
2015-11-05 11:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Remove debug line (702 bytes, patch)
2015-11-11 13:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix regression on sending an email from staff (1.81 KB, patch)
2015-11-13 15:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14544: Fix regression on sending an email from staff (1.29 KB, patch)
2015-11-13 16:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14544: Fix regression on sending an email from staff (1.40 KB, patch)
2015-11-14 14:00 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[PASSED QA] Bug 14544: Fix regression on sending an email from staff (1.45 KB, patch)
2015-11-15 23:05 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2015-07-16 17:07:14 UTC
There are some bugs in the list module which are not fixable easily, because of the (very) ugly C4::VirtualShelves::Page package.
This module is completely stuck and there is no other way to rewrite it completely to make it move again.
Comment 1 Jonathan Druart 2015-07-16 17:37:03 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2015-07-16 17:37:06 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2015-07-16 17:37:09 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-07-16 17:37:12 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2015-07-16 17:37:14 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2015-07-16 17:37:17 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2015-07-16 17:37:20 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2015-07-16 17:37:23 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2015-07-16 17:37:26 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall 2015-07-16 18:09:00 UTC
Based on my initial skimming of the patches, I think this looks great!

The only very minor nitpicky suggestion would be the naming of the clases. You could rename Koha::Virtualshelf to Koha::List and Koha::Virtualshelfshares to Koha::List::Sharess or something along those lines so that we can start down the road to having our internal nomenclature match our external nomenclature.

What do you think of that Jonathan?
Comment 12 Katrin Fischer 2015-07-16 22:23:48 UTC
I really like the idea of not naming them virtualshelves - but we have quite some lists now in the interface. Only problem might be that we also have patron lists and possible other types of lists?
Comment 13 Jonathan Druart 2015-08-04 09:07:30 UTC
(In reply to Kyle M Hall from comment #11)
> Based on my initial skimming of the patches, I think this looks great!
> 
> The only very minor nitpicky suggestion would be the naming of the clases.
> You could rename Koha::Virtualshelf to Koha::List and
> Koha::Virtualshelfshares to Koha::List::Sharess or something along those
> lines so that we can start down the road to having our internal nomenclature
> match our external nomenclature.
> 
> What do you think of that Jonathan?

It seems that a consensus on the name is not reach :)
And I really don't want to rewrite the job already done or wait to find a name...
I am sure it will be quite easy to rename everything (tables and classes) when this change will be done.
Comment 14 Jonathan Druart 2015-08-06 14:32:42 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2015-08-06 14:32:47 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2015-08-06 14:32:51 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2015-08-06 14:32:55 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2015-08-06 14:32:59 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2015-08-06 14:33:04 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2015-08-06 14:33:08 UTC Comment hidden (obsolete)
Comment 21 Jonathan Druart 2015-08-06 14:33:13 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2015-08-06 14:33:18 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2015-08-06 14:33:22 UTC Comment hidden (obsolete)
Comment 24 Jonathan Druart 2015-08-18 13:54:16 UTC Comment hidden (obsolete)
Comment 25 Jonathan Druart 2015-08-18 13:54:21 UTC Comment hidden (obsolete)
Comment 26 Jonathan Druart 2015-08-18 13:54:24 UTC Comment hidden (obsolete)
Comment 27 Jonathan Druart 2015-08-18 13:54:27 UTC Comment hidden (obsolete)
Comment 28 Jonathan Druart 2015-08-18 13:54:31 UTC Comment hidden (obsolete)
Comment 29 Jonathan Druart 2015-08-18 13:54:34 UTC Comment hidden (obsolete)
Comment 30 Jonathan Druart 2015-08-18 13:54:37 UTC Comment hidden (obsolete)
Comment 31 Jonathan Druart 2015-08-18 13:54:40 UTC Comment hidden (obsolete)
Comment 32 Jonathan Druart 2015-08-18 13:54:44 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2015-08-18 13:54:47 UTC Comment hidden (obsolete)
Comment 34 Jonathan Druart 2015-08-18 13:54:50 UTC Comment hidden (obsolete)
Comment 35 Jonathan Druart 2015-08-18 13:54:54 UTC Comment hidden (obsolete)
Comment 36 Jonathan Druart 2015-08-18 13:54:57 UTC Comment hidden (obsolete)
Comment 37 Jonathan Druart 2015-08-18 13:55:00 UTC Comment hidden (obsolete)
Comment 38 Jonathan Druart 2015-08-18 13:55:04 UTC Comment hidden (obsolete)
Comment 39 Jonathan Druart 2015-08-18 13:55:07 UTC Comment hidden (obsolete)
Comment 40 Jonathan Druart 2015-08-18 13:55:10 UTC Comment hidden (obsolete)
Comment 41 Jonathan Druart 2015-08-18 13:55:14 UTC Comment hidden (obsolete)
Comment 42 Fridolin SOMERS 2015-08-20 13:15:19 UTC
Patch "Make the intranet side independent of Page.pm" needs rebase since Bug 14517 is pushed.

Nice job btw
Comment 43 Jonathan Druart 2015-08-20 13:39:23 UTC Comment hidden (obsolete)
Comment 44 Jonathan Druart 2015-08-20 13:39:27 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2015-08-20 13:39:30 UTC Comment hidden (obsolete)
Comment 46 Jonathan Druart 2015-08-20 13:39:33 UTC Comment hidden (obsolete)
Comment 47 Jonathan Druart 2015-08-20 13:39:36 UTC Comment hidden (obsolete)
Comment 48 Jonathan Druart 2015-08-20 13:39:39 UTC Comment hidden (obsolete)
Comment 49 Jonathan Druart 2015-08-20 13:39:43 UTC Comment hidden (obsolete)
Comment 50 Jonathan Druart 2015-08-20 13:39:47 UTC Comment hidden (obsolete)
Comment 51 Jonathan Druart 2015-08-20 13:39:51 UTC Comment hidden (obsolete)
Comment 52 Jonathan Druart 2015-08-20 13:39:54 UTC Comment hidden (obsolete)
Comment 53 Jonathan Druart 2015-08-20 13:39:57 UTC Comment hidden (obsolete)
Comment 54 Jonathan Druart 2015-08-20 13:40:01 UTC Comment hidden (obsolete)
Comment 55 Jonathan Druart 2015-08-20 13:40:04 UTC Comment hidden (obsolete)
Comment 56 Jonathan Druart 2015-08-20 13:40:07 UTC Comment hidden (obsolete)
Comment 57 Jonathan Druart 2015-08-20 13:40:11 UTC Comment hidden (obsolete)
Comment 58 Jonathan Druart 2015-08-20 13:40:14 UTC Comment hidden (obsolete)
Comment 59 Jonathan Druart 2015-08-20 13:40:17 UTC Comment hidden (obsolete)
Comment 60 Jonathan Druart 2015-08-20 13:40:20 UTC Comment hidden (obsolete)
Comment 61 Jonathan Druart 2015-08-20 13:42:29 UTC
Last patchset fixes conflicts with bug 14517 and bug 14529.
Comment 62 Fridolin SOMERS 2015-08-21 09:02:01 UTC
There is a little error in opac/opac-addbybiblionumber.pl : 
  Line 120 : $shelf->
This brakes the addition of records in a list.

Other point :
I see in opac/opac-addbybiblionumber.pl :
    } else {
        # TODO
    }
Why not using $errcode ? like in virtualshelves/addbybiblionumber.pl ?
Comment 63 Jonathan Druart 2015-08-21 10:18:44 UTC Comment hidden (obsolete)
Comment 64 Jonathan Druart 2015-08-21 10:20:55 UTC
(In reply to Fridolin SOMERS from comment #62)
> There is a little error in opac/opac-addbybiblionumber.pl : 
>   Line 120 : $shelf->
> This brakes the addition of records in a list.

Oops, fixed!

> Other point :
> I see in opac/opac-addbybiblionumber.pl :
>     } else {
>         # TODO
>     }
> Why not using $errcode ? like in virtualshelves/addbybiblionumber.pl ?

Because it didn't before the changes, and I would like to submit another patchset to fix the 2 addbybiblionumber scripts. Not in the scope of this bug report.
Comment 65 Fridolin SOMERS 2015-08-21 12:30:33 UTC
(In reply to Jonathan Druart from comment #64)
> (In reply to Fridolin SOMERS from comment #62)
> > There is a little error in opac/opac-addbybiblionumber.pl : 
> >   Line 120 : $shelf->
> > This brakes the addition of records in a list.
> 
> Oops, fixed!
> 
It works fine, i continue testing
Comment 66 Fridolin SOMERS 2015-08-25 12:00:20 UTC
Small bugs in : koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt

1/
Line 20, the closing span tag is missing :
  &nbsp;<span>anyone else to add entries.

2/
In [% BLOCK list_permissions %]
the permissions vars must be called from "shelf" :
  [% IF allow_add %] must be [% IF shelf.allow_add %]
Idem for allow_delete_own and allow_delete_other

3/
In breadcrumbs, a <li></li> tag has been removed.
It was : 
  <li><a href="/cgi-bin/koha/opac-shelves.pl?display=privateshelves">Your lists</a></li>
Direct text is not correct inside a <ul></ul>.
Comment 67 Jonathan Druart 2015-08-25 12:15:41 UTC Comment hidden (obsolete)
Comment 68 Jonathan Druart 2015-08-25 12:16:14 UTC Comment hidden (obsolete)
Comment 69 Fridolin SOMERS 2015-08-25 12:41:11 UTC
(In reply to Jonathan Druart from comment #68)
> Created attachment 41910 [details] [review] [review]
> Bug 14544: Fix several minor bugs at the OPAC
> 
In breadcrumbs, &rsaquo; must also be inside a <li>.
Like in ISBD view : 
          <li><a href="/cgi-bin/koha/opac-main.pl">Home</a> <span class="divider">&rsaquo;</span></li>
        <li><a href="#">ISBD view</a></li>
Comment 70 Fridolin SOMERS 2015-08-25 12:47:05 UTC
4/
I've noticed that when editing a list, the HTML warnings show there are 2 useless ending div tags.

I think the correction is :

diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt
index 245a8dd..e5c40db 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt
@@ -705,13 +705,13 @@
                                     <p>No private lists.</p>
                                 [% END %]
                             [% END %]
+                        </div>
+                    </div>
                         [% ELSIF NOT loggedinusernumber %]
                             [% IF Koha.Preference( 'opacuserlogin' ) == 1 %]
                                 <div class="alert alert-info"><a href="/cgi-bin/koha/opac-user.pl">Log in</a> to create new lists.</div>
                             [% END %]
                         [% END  # IF loggedinusername %]
-                        </div>
-                    </div>
                 </div> <!-- / #usershelves -->
                 </div> <!-- / .span10/12 -->
       </div> <!-- / .row-fluid -->
Comment 71 Jonathan Druart 2015-08-25 13:13:57 UTC Comment hidden (obsolete)
Comment 72 Jonathan Druart 2015-08-25 13:15:28 UTC Comment hidden (obsolete)
Comment 73 Jonathan Druart 2015-08-25 13:58:22 UTC Comment hidden (obsolete)
Comment 74 Jonathan Druart 2015-08-25 14:03:35 UTC Comment hidden (obsolete)
Comment 75 Jonathan Druart 2015-08-25 14:12:30 UTC Comment hidden (obsolete)
Comment 76 Jonathan Druart 2015-08-25 14:18:25 UTC Comment hidden (obsolete)
Comment 77 Jonathan Druart 2015-08-25 14:33:03 UTC Comment hidden (obsolete)
Comment 78 Jonathan Druart 2015-08-25 14:53:28 UTC Comment hidden (obsolete)
Comment 79 Jonathan Druart 2015-08-25 15:10:02 UTC Comment hidden (obsolete)
Comment 80 Jonathan Druart 2015-08-25 15:10:06 UTC Comment hidden (obsolete)
Comment 81 Fridolin SOMERS 2015-08-26 08:15:49 UTC
New day, new bug :D

The creation of a new list after selecting some records fails :

It is because after creating Koha::Virtualshelf->new, one must call store() so that it impacts database, used by AddBibliosToShelf().

diff --git a/opac/opac-addbybiblionumber.pl b/opac/opac-addbybiblionumber.pl
index 10420fe..9181efd 100755
--- a/opac/opac-addbybiblionumber.pl
+++ b/opac/opac-addbybiblionumber.pl
@@ -106,7 +106,8 @@ sub HandleNewVirtualShelf {
             $errcode = 1;
             return;
         }
-        AddBibliosToShelf($shelfnumber, @biblionumber);
+        $shelf->store();
+        AddBibliosToShelf($shelf->shelfnumber, @biblionumber);
         #Reload the page where you came from
         print $query->header;
         print "<html><meta http-equiv=\"refresh\" content=\"0\" /><body onload=\"window.opener.location.reload(true);self.close();\"></body></html>";

Idem on staff interface.

You may also allow AddBibliosToShelf() to get a shelf object or a shelnumber as first argument.
Comment 82 Fridolin SOMERS 2015-08-26 10:00:08 UTC
Typo in "Bug 14544: Fix several minor bugs at the OPAC" : 
        [% IF op != 'list' %]
            <li><a href="/cgi-bin/koha/cgi-bin/koha/opac-shelves.pl">Lists</a></li>
        [% ELSE %]

/cgi-bin/koha twice
Comment 83 Fridolin SOMERS 2015-08-26 10:18:56 UTC
3/ Share some lists with other users
Note that it is managed by syspref OpacAllowSharingPrivateLists
Comment 84 Jonathan Druart 2015-08-26 12:45:06 UTC Comment hidden (obsolete)
Comment 85 Jonathan Druart 2015-08-26 12:45:11 UTC Comment hidden (obsolete)
Comment 86 Jonathan Druart 2015-08-26 12:45:15 UTC Comment hidden (obsolete)
Comment 87 Jonathan Druart 2015-08-26 12:45:19 UTC Comment hidden (obsolete)
Comment 88 Jonathan Druart 2015-08-26 12:45:23 UTC Comment hidden (obsolete)
Comment 89 Jonathan Druart 2015-08-26 12:45:27 UTC Comment hidden (obsolete)
Comment 90 Jonathan Druart 2015-08-26 12:45:31 UTC Comment hidden (obsolete)
Comment 91 Jonathan Druart 2015-08-26 12:45:34 UTC Comment hidden (obsolete)
Comment 92 Jonathan Druart 2015-08-26 12:45:39 UTC Comment hidden (obsolete)
Comment 93 Jonathan Druart 2015-08-26 12:45:44 UTC Comment hidden (obsolete)
Comment 94 Jonathan Druart 2015-08-26 12:45:49 UTC Comment hidden (obsolete)
Comment 95 Jonathan Druart 2015-08-26 12:45:55 UTC Comment hidden (obsolete)
Comment 96 Jonathan Druart 2015-08-26 12:45:59 UTC Comment hidden (obsolete)
Comment 97 Jonathan Druart 2015-08-26 12:46:05 UTC Comment hidden (obsolete)
Comment 98 Jonathan Druart 2015-08-26 12:46:10 UTC Comment hidden (obsolete)
Comment 99 Jonathan Druart 2015-08-26 12:46:16 UTC Comment hidden (obsolete)
Comment 100 Jonathan Druart 2015-08-26 12:46:22 UTC Comment hidden (obsolete)
Comment 101 Jonathan Druart 2015-08-26 12:46:28 UTC Comment hidden (obsolete)
Comment 102 Jonathan Druart 2015-08-26 13:09:13 UTC Comment hidden (obsolete)
Comment 103 Jonathan Druart 2015-08-26 13:09:17 UTC Comment hidden (obsolete)
Comment 104 Jonathan Druart 2015-08-26 13:09:21 UTC Comment hidden (obsolete)
Comment 105 Jonathan Druart 2015-08-26 13:09:26 UTC Comment hidden (obsolete)
Comment 106 Jonathan Druart 2015-08-26 13:09:29 UTC Comment hidden (obsolete)
Comment 107 Jonathan Druart 2015-08-26 13:09:33 UTC Comment hidden (obsolete)
Comment 108 Jonathan Druart 2015-08-26 13:09:37 UTC Comment hidden (obsolete)
Comment 109 Jonathan Druart 2015-08-26 13:09:40 UTC Comment hidden (obsolete)
Comment 110 Jonathan Druart 2015-08-26 13:09:44 UTC Comment hidden (obsolete)
Comment 111 Jonathan Druart 2015-08-26 13:09:48 UTC Comment hidden (obsolete)
Comment 112 Jonathan Druart 2015-08-26 13:25:25 UTC Comment hidden (obsolete)
Comment 113 Jonathan Druart 2015-08-26 13:36:57 UTC Comment hidden (obsolete)
Comment 114 Jonathan Druart 2015-08-26 13:48:47 UTC Comment hidden (obsolete)
Comment 115 Fridolin SOMERS 2015-08-26 14:39:35 UTC
A behaviour I'm not sure it is the scope of this bug :
- Log-in with a user A at intranet
- Create a public list
- Add some records
- Allow anyone else to add entries and anyone to remove his own contributed entries, do not allow any user to remove any entries
- Log-in with a user B at OPAC
- View the list
- You see links "Remove from this list" on every record, even if they where added by another user
=> It would be better to show this link only in records you can remove
- When clicked and alert accepted, you get the message "No biblio has been removed." (records are not removed)
=> You should get a message of "unauthorized_on_remove_biblios"
- Select some records
- Click on "Remove from list"
- You get no message (records are not removed)
=> Not sure what to do here because you may be allowed on some selected records
Comment 116 Jonathan Druart 2015-08-26 15:15:45 UTC
It's the expected behavior, at least the one before this patch.
But I completely agree, it could be improved.
Comment 117 Alex Arnaud 2015-09-02 08:11:43 UTC Comment hidden (obsolete)
Comment 118 Jonathan Druart 2015-09-02 08:48:06 UTC
(In reply to Alex Arnaud from comment #117)
> Created attachment 42192 [details] [review] [review]
> Bug 14544 - Handle database error when creating a shelf with DB admin account

I think this is not useful, the DB admin account should not be used for anything else than create a first superlibrarian user.
Comment 119 Alex Arnaud 2015-09-02 09:00:23 UTC
(In reply to Jonathan Druart from comment #118)
> I think this is not useful, the DB admin account should not be used for
> anything else than create a first superlibrarian user.
It was the behavior before this patches. And, isn't it better to have a message that says we can't created a shelf using db admin account instead of a MySql/DBIX error ?
Comment 120 Jonathan Druart 2015-09-02 11:14:22 UTC
We cannot track all the places this check should be done.
It's a waste of time.
But I am not against your patch, except that the Exception shouldn't be VirtualShelves specific (should be Koha::Exceptions::UseDbAdminAccount IMO).
Comment 121 Alex Arnaud 2015-09-02 12:34:18 UTC Comment hidden (obsolete)
Comment 122 Alex Arnaud 2015-09-02 12:34:32 UTC Comment hidden (obsolete)
Comment 123 Alex Arnaud 2015-09-02 12:34:41 UTC Comment hidden (obsolete)
Comment 124 Alex Arnaud 2015-09-02 12:34:56 UTC Comment hidden (obsolete)
Comment 125 Alex Arnaud 2015-09-02 12:35:04 UTC Comment hidden (obsolete)
Comment 126 Alex Arnaud 2015-09-02 12:35:10 UTC Comment hidden (obsolete)
Comment 127 Alex Arnaud 2015-09-02 12:35:17 UTC Comment hidden (obsolete)
Comment 128 Alex Arnaud 2015-09-02 12:35:27 UTC Comment hidden (obsolete)
Comment 129 Alex Arnaud 2015-09-02 12:35:37 UTC Comment hidden (obsolete)
Comment 130 Alex Arnaud 2015-09-02 12:35:43 UTC Comment hidden (obsolete)
Comment 131 Alex Arnaud 2015-09-02 12:35:50 UTC Comment hidden (obsolete)
Comment 132 Alex Arnaud 2015-09-02 12:35:56 UTC Comment hidden (obsolete)
Comment 133 Alex Arnaud 2015-09-02 12:36:02 UTC Comment hidden (obsolete)
Comment 134 Alex Arnaud 2015-09-02 12:36:11 UTC Comment hidden (obsolete)
Comment 135 Alex Arnaud 2015-09-02 12:36:17 UTC Comment hidden (obsolete)
Comment 136 Alex Arnaud 2015-09-02 12:36:24 UTC Comment hidden (obsolete)
Comment 137 Alex Arnaud 2015-09-02 12:36:32 UTC Comment hidden (obsolete)
Comment 138 Alex Arnaud 2015-09-02 12:36:38 UTC Comment hidden (obsolete)
Comment 139 Alex Arnaud 2015-09-02 12:36:45 UTC Comment hidden (obsolete)
Comment 140 Alex Arnaud 2015-09-02 12:36:52 UTC Comment hidden (obsolete)
Comment 141 Alex Arnaud 2015-09-02 12:36:58 UTC Comment hidden (obsolete)
Comment 142 Alex Arnaud 2015-09-02 12:42:26 UTC
Didn't see any *new* bugs/issues so i signed off all the patches (except mine). 

But Why don't use this BZ to fix existing problems on virtualshelves system (Like the one shown by Fridolin)?
Comment 143 Jonathan Druart 2015-09-02 12:45:31 UTC
(In reply to Alex Arnaud from comment #142)
> Didn't see any *new* bugs/issues so i signed off all the patches (except
> mine). 
> 
> But Why don't use this BZ to fix existing problems on virtualshelves system
> (Like the one shown by Fridolin)?

Because it's a huge change enough :)
Comment 144 Jonathan Druart 2015-09-17 08:43:40 UTC Comment hidden (obsolete)
Comment 145 Marcel de Rooy 2015-10-08 08:08:27 UTC
I am planning to look at this patch set tomorrow, but I still have:

Applying: Bug 14544: Get rid of C4::VirtualShelves and C4::VirtualShelves::Page
fatal: sha1 information is lacking or useless (catalogue/detail.pl).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Comment 146 Jonathan Druart 2015-10-08 09:09:54 UTC Comment hidden (obsolete)
Comment 147 Jonathan Druart 2015-10-08 09:09:59 UTC Comment hidden (obsolete)
Comment 148 Jonathan Druart 2015-10-08 09:10:03 UTC Comment hidden (obsolete)
Comment 149 Jonathan Druart 2015-10-08 09:10:07 UTC Comment hidden (obsolete)
Comment 150 Jonathan Druart 2015-10-08 09:10:12 UTC Comment hidden (obsolete)
Comment 151 Jonathan Druart 2015-10-08 09:10:16 UTC Comment hidden (obsolete)
Comment 152 Jonathan Druart 2015-10-08 09:10:21 UTC Comment hidden (obsolete)
Comment 153 Jonathan Druart 2015-10-08 09:10:26 UTC Comment hidden (obsolete)
Comment 154 Jonathan Druart 2015-10-08 09:10:30 UTC Comment hidden (obsolete)
Comment 155 Jonathan Druart 2015-10-08 09:10:35 UTC Comment hidden (obsolete)
Comment 156 Jonathan Druart 2015-10-08 09:10:39 UTC Comment hidden (obsolete)
Comment 157 Jonathan Druart 2015-10-08 09:10:44 UTC Comment hidden (obsolete)
Comment 158 Jonathan Druart 2015-10-08 09:10:49 UTC Comment hidden (obsolete)
Comment 159 Jonathan Druart 2015-10-08 09:10:53 UTC Comment hidden (obsolete)
Comment 160 Jonathan Druart 2015-10-08 09:10:58 UTC Comment hidden (obsolete)
Comment 161 Jonathan Druart 2015-10-08 09:11:02 UTC Comment hidden (obsolete)
Comment 162 Jonathan Druart 2015-10-08 09:11:07 UTC Comment hidden (obsolete)
Comment 163 Jonathan Druart 2015-10-08 09:11:12 UTC Comment hidden (obsolete)
Comment 164 Jonathan Druart 2015-10-08 09:11:16 UTC Comment hidden (obsolete)
Comment 165 Jonathan Druart 2015-10-08 09:11:21 UTC Comment hidden (obsolete)
Comment 166 Jonathan Druart 2015-10-08 09:11:25 UTC Comment hidden (obsolete)
Comment 167 Jonathan Druart 2015-10-08 09:12:31 UTC
(In reply to Marcel de Rooy from comment #145)
> I am planning to look at this patch set tomorrow, but I still have:
> 
> Applying: Bug 14544: Get rid of C4::VirtualShelves and
> C4::VirtualShelves::Page
> fatal: sha1 information is lacking or useless (catalogue/detail.pl).
> Repository lacks necessary blobs to fall back on 3-way merge.
> Cannot fall back to three-way merge.

Patches rebased against master.
Comment 168 Marcel de Rooy 2015-10-08 09:34:43 UTC
(In reply to Jonathan Druart from comment #167)
> (In reply to Marcel de Rooy from comment #145)
> > I am planning to look at this patch set tomorrow, but I still have:
> > 
> > Applying: Bug 14544: Get rid of C4::VirtualShelves and
> > C4::VirtualShelves::Page
> > fatal: sha1 information is lacking or useless (catalogue/detail.pl).
> > Repository lacks necessary blobs to fall back on 3-way merge.
> > Cannot fall back to three-way merge.
> 
> Patches rebased against master.

Thanks. Applies now.
Comment 169 Marcel de Rooy 2015-10-09 06:07:25 UTC
QA: Looking at this now.
Comment 170 Marcel de Rooy 2015-10-09 11:30:58 UTC Comment hidden (obsolete)
Comment 171 Marcel de Rooy 2015-10-09 11:54:10 UTC
QA Comment:
Great job !!! Lot of work involved.
Still have some points for consideration:

OPAC -- View contents of a list -- Send shelf - Crash
Can't call method "can_be_viewed" on an undefined value at /home/koha/testclone/opac/opac-sendshelf.pl line 55.
*BLOCKER*

OPAC - View contents of a list - Sort by callnumber - Bang
DBIx::Class::ResultSet::next(): Unknown column 'itemcallnumber' in 'order clause' at /home/koha/testclone/opac/opac-shelves.pl line 236
*BLOCKER*

OPAC - Lists- Open a shared list where you do not have permission to remove items of someone else. Try to remove such an item. No success, but no warning too! IIRC There was a warning in the past about some items not deleted..
Now try to remove a item you added yourself to that list while having permission to remove your own entries. This does not work ! No items are deleted, no message.
I am inclined to see this last point as a *BLOCKER* too.

OPAC - View list - Delete your own item while the perms are ADD or DDD. Should not be possible. But the owner is able to now. This is change of behavior and not consistent with the wording of those permissions "Do not allow anyone to remove his own contributed entries" (including your own!). I would view it as a sort of Readonly list
where you cannot accidentally delete your own items. (Note however that adding is still possible.) 
I am inclined to see this last point as a *BLOCKER* too (but I may not be completely neutral here :)
+                    # FIXME
+                    # This does not make sense, but it's has been backported from DelFromShelf.
+                    # Why shouldn't we allow to remove his own contribution if allow_delete_other is on?

Code Exceptions.pm module comes in now with a dependency Exception::Class.
The description => "poeut" still needs some attention apparently :) Not sure what it means btw..
Although this looks good to me on itself, this dev should probably better have gone on its own report. Note there is another report too (13995).
It kind of creeps in now.

OPAC - Share a list - Enter email address - Return to your lists: This list does not exist ??  op=view && category=1. Adjust the URL? 

OPAC - I shared a list with AAD perms. The other user could add a few entries. After that removed the share.
Now come back as owner of the list and try to remove one item of yourself and one of the other user.
You will get the message: The item has been removed from the list. 
This is not completely true: You could only remove one item; the other is not permitted. 

OPAC - Create a new list - Starts with permissions D D D - Would it not be better to default to D A D ?
Note that if I create a list with save to new list, it does default to DAD. So this is a minor inconsistency..

OPAC - View a list: For a list with ADD or DDD perms, the button Remove could be hidden or disabled? Might be the case now too btw.

OPAC Detail - List(s) this item appears in: [nothing filled while expected] 
I click from view a (private) list on one entry. So go to opac detail. Should I expect that list to be mentioned now? It is not.

Staff - View public lists [Related to another report]
With the delete public lists permission I can delete the public list of another user. OK The staff user probably has a good reason to do so. 
Just noting that it would perhaps be more friendly to switch it to Private instead of a rigourous Delete?

Code in opac-addbybiblionumber.pl: HandleSelectedShelf: Empty TODO in an else branch. Noting for the record. 

Code  t/db_dependent/Utils/Datatables_Virtualshelves.t and C4/Utils/DataTables/VirtualShelves.pm
This is only justified/used by svc/virtualshelves/search
Isn't this the time to move this svc script to Koha::Virtualshelves too ? Or at least on another report ? 

Database design: Do we still need (or use) virtualshelfcontents.flags  ? Could a dbrev remove it now?

An observation (which could be considered as outside the scope): Add an item to the list with a barcode (in staff) is somewhat strange. You cannot add items, only biblios. This is current situation already. Just for completeness :)

Moving the status to FQA now to reflect the need for some adjustments/clarifications.
Comment 172 Jonathan Druart 2015-10-12 09:39:23 UTC
(In reply to Marcel de Rooy from comment #171)
> QA Comment:
> Great job !!! Lot of work involved.

Thanks a lot for Qaing this!
Very useful feedback :)

> Still have some points for consideration:
> 
> OPAC -- View contents of a list -- Send shelf - Crash
> Can't call method "can_be_viewed" on an undefined value at
> /home/koha/testclone/opac/opac-sendshelf.pl line 55.
> *BLOCKER*

Fixed!
The shelfnumber param was not passed.
I have also add a test in the pl file to avoid this crash when viewing an non-existing shelf (now display 'not authorized', which is the current behavior).

> OPAC - View contents of a list - Sort by callnumber - Bang
> DBIx::Class::ResultSet::next(): Unknown column 'itemcallnumber' in 'order
> clause' at /home/koha/testclone/opac/opac-shelves.pl line 236
> *BLOCKER*

Fixed! But... This is a bad one.
In opac-shelves.pl and shelves.pl, the join was only done on biblio.
But itemcallnumber is in the items table.
I had to add the biblio<=>biblioitems<=>items relationship to the Schema and a PK on virtualshelfcontents table (DBIx::Class::Storage::DBIHacks::_adjust_select_args_for_complex_prefetch(): Unable to perform complex limited prefetch off Virtualshelfcontent without declared primary key).
Fixed in a separate patch. Note that bug 14819 is stuck in the NSO status but also adds the relationships.

> OPAC - Lists- Open a shared list where you do not have permission to remove
> items of someone else. Try to remove such an item. No success, but no
> warning too! IIRC There was a warning in the past about some items not
> deleted..
> Now try to remove a item you added yourself to that list while having
> permission to remove your own entries. This does not work ! No items are
> deleted, no message.
> I am inclined to see this last point as a *BLOCKER* too.

Fixed!
The "op" was "view" instead of "remove_biblios" on the submit button.
Note that the individual links work (and intranet is ok).

> OPAC - View list - Delete your own item while the perms are ADD or DDD.
> Should not be possible. But the owner is able to now. This is change of
> behavior and not consistent with the wording of those permissions "Do not
> allow anyone to remove his own contributed entries" (including your own!). I
> would view it as a sort of Readonly list
> where you cannot accidentally delete your own items. (Note however that
> adding is still possible.) 
> I am inclined to see this last point as a *BLOCKER* too (but I may not be
> completely neutral here :)
> +                    # FIXME
> +                    # This does not make sense, but it's has been
> backported from DelFromShelf.
> +                    # Why shouldn't we allow to remove his own contribution
> if allow_delete_other is on?

Well, not fixed.
I have asked on #koha and Nicole agreed (http://irc.koha-community.org/koha/2015-08-18#i_1716925).
So I am not sure what to do here.

> Code Exceptions.pm module comes in now with a dependency Exception::Class.
> The description => "poeut" still needs some attention apparently :) Not sure
> what it means btw..
> Although this looks good to me on itself, this dev should probably better
> have gone on its own report. Note there is another report too (13995).
> It kind of creeps in now.

poeut is a typo for pouet which does not mean anything :)
The exceptions implementation can be modified easily later (the question was: should we put all exceptions in 1 file, all exceptions of a given module in 1 file or 1 exception per file).

> OPAC - Share a list - Enter email address - Return to your lists: This list
> does not exist ??  op=view && category=1. Adjust the URL? 

Fixed!
Yes, the op is 'list', not 'view".

> OPAC - I shared a list with AAD perms. The other user could add a few
> entries. After that removed the share.
> Now come back as owner of the list and try to remove one item of yourself
> and one of the other user.
> You will get the message: The item has been removed from the list. 
> This is not completely true: You could only remove one item; the other is
> not permitted. 

This is a specific case and I would prefer to manage it in a separate bug report.
I am not sure this work on master :)

> OPAC - Create a new list - Starts with permissions D D D - Would it not be
> better to default to D A D ?
> Note that if I create a list with save to new list, it does default to DAD.
> So this is a minor inconsistency..

Yes, and it's a regression.
Fixed on both interface (I have c/p OPAC code for the intranet).

> OPAC - View a list: For a list with ADD or DDD perms, the button Remove
> could be hidden or disabled? Might be the case now too btw.

Not easily, and there is no regression here: it's how it works currently.
The problem is that the queries done in remove_biblios should be executed for each biblio in the list.
Maybe not a big deal, to investigate...

> OPAC Detail - List(s) this item appears in: [nothing filled while expected] 
> I click from view a (private) list on one entry. So go to opac detail.
> Should I expect that list to be mentioned now? It is not.

Fixed!
In the template shelves->count should be check, not only the shelves variable (which is always defined).

> Staff - View public lists [Related to another report]
> With the delete public lists permission I can delete the public list of
> another user. OK The staff user probably has a good reason to do so. 
> Just noting that it would perhaps be more friendly to switch it to Private
> instead of a rigourous Delete?

Todo later yes.

> Code in opac-addbybiblionumber.pl: HandleSelectedShelf: Empty TODO in an
> else branch. Noting for the record. 

Yes, I preferred to add the TODO marked than nothing, but no regression here.

> Code  t/db_dependent/Utils/Datatables_Virtualshelves.t and
> C4/Utils/DataTables/VirtualShelves.pm
> This is only justified/used by svc/virtualshelves/search
> Isn't this the time to move this svc script to Koha::Virtualshelves too ? Or
> at least on another report ? 

Not ready yet. The rewrite should use the REST api.

> Database design: Do we still need (or use) virtualshelfcontents.flags  ?
> Could a dbrev remove it now?

To investigate, but yes, I am not sure to have found any usefulness to this column.

> An observation (which could be considered as outside the scope): Add an item
> to the list with a barcode (in staff) is somewhat strange. You cannot add
> items, only biblios. This is current situation already. Just for
> completeness :)

Changing the wording could maybe fix the ambiguity.
Comment 173 Jonathan Druart 2015-10-12 09:46:58 UTC Comment hidden (obsolete)
Comment 174 Jonathan Druart 2015-10-12 09:47:03 UTC Comment hidden (obsolete)
Comment 175 Marcel de Rooy 2015-10-12 11:02:20 UTC
From IRC http://irc.koha-community.org/koha/2015-08-18
===
11:24 Joubu: nengard: Don't you think the following does not make sense? : "The owner of a list is always allowed to add entries, but needs permission to remove."
11:25 nengard that makes no sense at all
===

Well, i do not agree that it make no sense at all..
Although this is not the most important issue on lists, but just a detail, such a short conversation on irc is not the decisive factor for me to justify changing current behavior. Also note that the question is formulated rather suggestive.
Comment 176 Jonathan Druart 2015-10-12 11:12:40 UTC
(In reply to Marcel de Rooy from comment #175)
> From IRC http://irc.koha-community.org/koha/2015-08-18
> ===
> 11:24 Joubu: nengard: Don't you think the following does not make sense? :
> "The owner of a list is always allowed to add entries, but needs permission
> to remove."
> 11:25 nengard that makes no sense at all
> ===
> 
> Well, i do not agree that it make no sense at all..
> Although this is not the most important issue on lists, but just a detail,
> such a short conversation on irc is not the decisive factor for me to
> justify changing current behavior. Also note that the question is formulated
> rather suggestive.

I don't have a strong opinion on it.
I have to admit I did not see the "readonly list" feature, and it could make sense.
Comment 177 Jonathan Druart 2015-10-14 14:11:03 UTC Comment hidden (obsolete)
Comment 178 Jonathan Druart 2015-10-14 14:12:18 UTC
Marcel,
The last patch reintroduces the read-only list feature/bug :)
If we decide later to remove it, we would be able to do it reverting this patch!
Comment 179 Marcel de Rooy 2015-10-14 14:17:35 UTC
I plan to look at this again Friday.
Comment 180 Marcel de Rooy 2015-10-16 08:52:17 UTC
Created attachment 43456 [details] [review]
Bug 14544: Add Koha::Virtualshelf and Koha::Virtualshelves

The 2 modules C4::VirtualShelves and C4::VirtualShelves::Page are really badly
coded and buggy.
It became complicated to fix and enhance them.
This rewrite will make the code more robust and the list feature will be easier
to change.

Some new modules are added to the Koha namespace (Koha::Virtualshelf,
Koha::Virtualshelves, Koha::Virtualshelfshare[s] and Koha::Virtualshelfcontent[s])
to take advantage of Dbix::Class.

The patchset fixes some bugs:
1/ Bug 6279 - Can't delete lists from the second page of lists in the OPAC
2/ The owner of a list should not need any permission to remove content from
his lists.
3/ No feedback messages when actions are done

Test plan:
A) Intranet and OPAC
1/ Create more than 20 private and public lists with different users
2/ Edit some and confirm that the changes are taken into account
3/ Share some lists with other users
4/ Add / remove contents
Be sure you have some lists with more than 20 items (to test the pagination)
5/ Play with permissions and confirm it works as expected.
For private lists:
They should not be viewed by anyone else, if they are not shared
If they are shared (OPAC only):
a. "Allow anyone else to add entries"
  Should allow the user with whom the list is shared to add entries.
b. "Allow anyone to remove his own contributed entries."
  Should allow the user with whom the list is shared to remove his own entries.
c. "Allow anyone to remove other contributed entries."
  Should allow the user with whom the list is shared to remove any entries.

For public lists:
They should be viewed by anyone else.
a. "Allow anyone else to add entries"
  Should allow any user to add entries.
b. "Allow anyone to remove his own contributed entries."
  Should allow any user to remove his own entries.
c. "Allow anyone to remove other contributed entries."
  Should allow any user to remove any entries.

6/ Download and sent lists.

7/ Confirm that the "Add to" feature works as expected.

8/ Confirm that the "Add an item to" works as expected.

9/ Delete some lists.

10/ Print some lists and subscribe to the RSS feed (OPAC only) (with more than
20 items).

11/ Use the remove "Remove share" links.

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 181 Marcel de Rooy 2015-10-16 08:52:22 UTC
Created attachment 43457 [details] [review]
Bug 14544: Get rid of GetShelf

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 182 Marcel de Rooy 2015-10-16 08:52:29 UTC
Created attachment 43458 [details] [review]
Bug 14544: Get rid of AddShelf

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 183 Marcel de Rooy 2015-10-16 08:52:35 UTC
Created attachment 43459 [details] [review]
Bug 14544: Get rid of DelShelf

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 184 Marcel de Rooy 2015-10-16 08:52:40 UTC
Created attachment 43460 [details] [review]
Bug 14544: Get rid of ModShelf

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 185 Marcel de Rooy 2015-10-16 08:52:46 UTC
Created attachment 43461 [details] [review]
Bug 14544: Move share routines to Koha::Virtualshelfshare and Koha::Virtualshelfshares

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 186 Marcel de Rooy 2015-10-16 08:52:51 UTC
Created attachment 43462 [details] [review]
Bug 14544: Get rid of GetAllShelves

Signed-off-by: Koha Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 187 Marcel de Rooy 2015-10-16 08:52:56 UTC
Created attachment 43463 [details] [review]
Bug 14544: Koha::Virtualshelfcontent[s]

Get rid of AddToShelf and DelFromShelf

Bug 14544: Allow a user to delete his own contents

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 188 Marcel de Rooy 2015-10-16 08:53:02 UTC Comment hidden (obsolete)
Comment 189 Marcel de Rooy 2015-10-16 08:53:07 UTC Comment hidden (obsolete)
Comment 190 Marcel de Rooy 2015-10-16 08:53:12 UTC Comment hidden (obsolete)
Comment 191 Marcel de Rooy 2015-10-16 08:53:18 UTC Comment hidden (obsolete)
Comment 192 Marcel de Rooy 2015-10-16 08:53:23 UTC Comment hidden (obsolete)
Comment 193 Marcel de Rooy 2015-10-16 08:53:29 UTC Comment hidden (obsolete)
Comment 194 Marcel de Rooy 2015-10-16 08:53:34 UTC Comment hidden (obsolete)
Comment 195 Marcel de Rooy 2015-10-16 08:53:39 UTC Comment hidden (obsolete)
Comment 196 Marcel de Rooy 2015-10-16 08:53:45 UTC Comment hidden (obsolete)
Comment 197 Marcel de Rooy 2015-10-16 08:53:50 UTC Comment hidden (obsolete)
Comment 198 Marcel de Rooy 2015-10-16 08:53:56 UTC Comment hidden (obsolete)
Comment 199 Marcel de Rooy 2015-10-16 08:54:01 UTC Comment hidden (obsolete)
Comment 200 Marcel de Rooy 2015-10-16 08:54:06 UTC Comment hidden (obsolete)
Comment 201 Marcel de Rooy 2015-10-16 08:54:12 UTC Comment hidden (obsolete)
Comment 202 Marcel de Rooy 2015-10-16 08:54:17 UTC Comment hidden (obsolete)
Comment 203 Marcel de Rooy 2015-10-16 08:54:23 UTC Comment hidden (obsolete)
Comment 204 Marcel de Rooy 2015-10-16 08:54:28 UTC Comment hidden (obsolete)
Comment 205 Marcel de Rooy 2015-10-16 08:54:33 UTC Comment hidden (obsolete)
Comment 206 Marcel de Rooy 2015-10-16 08:55:43 UTC
QA Comment [Continuation]:

Thanks again for your hard work.

>> OPAC - Share a list - Enter email address - Return to your lists: This list
>> does not exist ??  op=view && category=1. Adjust the URL? 
> Fixed!
> Yes, the op is 'list', not 'view".
Fixed?? This is still a problem. Did you test it? (No blocker for such a large patch, but ... Do you miss the shelf number?) 

> Fixed! But... This is a bad one.
> In opac-shelves.pl and shelves.pl, the join was only done on biblio.
> But itemcallnumber is in the items table.
> I had to add the biblio<=>biblioitems<=>items relationship to the Schema and a PK on virtualshelfcontents table
> (DBIx::Class::Storage::DBIHacks::_adjust_select_args_for_complex_prefetch(): Unable to perform complex limited prefetch off Virtualshelfcontent 
> without declared primary key).
But this is not completely solved.
View a private list in opac or staff: Bang.
DBIx::Class::ResultSet::next(): Operation requires a primary key to be declared on 'Virtualshelfcontent' via set_primary_key
How did you test this? :) Apparently, you forgot to include this added PK in your patches.
Fixed by a hack now: added set_primary_key in Virtualshelfcontent in the local section.
Please adjust this by defining the PK on the table and regenerating the DBIx file. 
Note also that I removed the belongs_to from Biblioitem to Item. This is not a FK but a PK.

> Fixed!
> The "op" was "view" instead of "remove_biblios" on the submit button.
> Note that the individual links work (and intranet is ok).
I removed two items. One should not be removed, the other should. This works now, but the message is not really clear. No blocker.

> The last patch reintroduces the read-only list feature/bug :)
> If we decide later to remove it, we would be able to do it reverting this patch!
Applying: Bug 14544: allow_delete_own == 0 could represent a read-only list
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Virtualshelves.t
CONFLICT (content): Merge conflict in t/db_dependent/Virtualshelves.t
At this time I rather obsolete this additional patch. The wording was removed, the behavior slightly changed.
No need to test this now. It has been just a reminder that changing behavior may need more communication than one incidental IRC chat :)

Additional observation: The Author and [Date added] column in the shelf view on staff are empty. (With and without the last four follow-up patches.) 

Passed QA
Comment 207 Marcel de Rooy 2015-10-16 08:56:06 UTC
Note to RM
Another db rev may follow from Jonathan to correct the mentioned PK issue with DBIx.
Please note that you get Koha::Exception too now. (See earlier QA comment.) 
An early push may help us to detect if this patch set still contains some further issues. But it looks quite good.
Comment 208 Jonathan Druart 2015-10-16 09:50:35 UTC Comment hidden (obsolete)
Comment 209 Jonathan Druart 2015-10-16 09:53:12 UTC
(In reply to Marcel de Rooy from comment #206)
> QA Comment [Continuation]:
> 
> Thanks again for your hard work.
> 
> >> OPAC - Share a list - Enter email address - Return to your lists: This list
> >> does not exist ??  op=view && category=1. Adjust the URL? 
> > Fixed!
> > Yes, the op is 'list', not 'view".
> Fixed?? This is still a problem. Did you test it? (No blocker for such a
> large patch, but ... Do you miss the shelf number?) 

Hum no, I have tested the link just before the form submit!
The last patch fixes that.

> > Fixed! But... This is a bad one.
> > In opac-shelves.pl and shelves.pl, the join was only done on biblio.
> > But itemcallnumber is in the items table.
> > I had to add the biblio<=>biblioitems<=>items relationship to the Schema and a PK on virtualshelfcontents table
> > (DBIx::Class::Storage::DBIHacks::_adjust_select_args_for_complex_prefetch(): Unable to perform complex limited prefetch off Virtualshelfcontent 
> > without declared primary key).
> But this is not completely solved.
> View a private list in opac or staff: Bang.
> DBIx::Class::ResultSet::next(): Operation requires a primary key to be
> declared on 'Virtualshelfcontent' via set_primary_key
> How did you test this? :) Apparently, you forgot to include this added PK in
> your patches.
> Fixed by a hack now: added set_primary_key in Virtualshelfcontent in the
> local section.
> Please adjust this by defining the PK on the table and regenerating the DBIx
> file. 
> Note also that I removed the belongs_to from Biblioitem to Item. This is not
> a FK but a PK.

With my previous branch (so without your follow-ups), I have regenerated the schema and I don't get an error viewing a private list.
That's why I have added the belongs_to rs.
Comment 210 Jonathan Druart 2015-10-16 10:02:48 UTC Comment hidden (obsolete)
Comment 211 Jonathan Druart 2015-10-16 10:03:34 UTC
(In reply to Marcel de Rooy from comment #206)
> Additional observation: The Author and [Date added] column in the shelf view
> on staff are empty. (With and without the last four follow-up patches.) 

See last patch.
Comment 212 Marcel de Rooy 2015-10-16 10:24:24 UTC
(In reply to Jonathan Druart from comment #208)
> Created attachment 43485 [details] [review] [review]
> Bug 14544: Fix 'back to lists' link

Sure about the location? sendshelf? Creating a share is shareshelf?
Comment 213 Jonathan Druart 2015-10-16 10:26:30 UTC
(In reply to Marcel de Rooy from comment #212)
> (In reply to Jonathan Druart from comment #208)
> > Created attachment 43485 [details] [review] [review] [review]
> > Bug 14544: Fix 'back to lists' link
> 
> Sure about the location? sendshelf? Creating a share is shareshelf?

Erk, no! So there is another one missing.
Comment 214 Jonathan Druart 2015-10-16 10:30:09 UTC Comment hidden (obsolete)
Comment 215 Jonathan Druart 2015-10-16 10:31:16 UTC
(In reply to Marcel de Rooy from comment #212)
> (In reply to Jonathan Druart from comment #208)
> > Created attachment 43485 [details] [review] [review] [review]
> > Bug 14544: Fix 'back to lists' link
> 
> Sure about the location? sendshelf? Creating a share is shareshelf?

Sorry about that.
I am sure I have fixed it already but I have certainly lost it during a rebase -i.
Now it works!
Comment 216 Tomás Cohen Arazi 2015-11-04 14:07:08 UTC
Please rebase.
Comment 217 Jonathan Druart 2015-11-05 11:07:22 UTC
Created attachment 44467 [details] [review]
Bug 14544: Make the intranet side independent of Page.pm

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 218 Jonathan Druart 2015-11-05 11:07:27 UTC
Created attachment 44468 [details] [review]
Bug 14544: Get rid of GetBibliosShelves

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 219 Jonathan Druart 2015-11-05 11:07:32 UTC
Created attachment 44469 [details] [review]
Bug 14544: Get rid of GetShelves

This does not work for now, see later.

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 220 Jonathan Druart 2015-11-05 11:07:38 UTC
Created attachment 44470 [details] [review]
Bug 14544: Make the OPAC side independent of Page.pm

Bug 14544: Fix redirect on editing a list

If you edit a list from the list view, after saving the form, you are
not redirected to the list view (but on the edit form).

Bug 14544: Cosmetic: &rsaquo; should be a class divider

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 221 Jonathan Druart 2015-11-05 11:07:43 UTC
Created attachment 44471 [details] [review]
Bug 14544: Get rid of ShelfPossibleAction

Bug 14544: (follow-up) Get rid of ShelfPossibleAction

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 222 Jonathan Druart 2015-11-05 11:07:49 UTC
Created attachment 44472 [details] [review]
Bug 14544: Get rid of GetShelfContent

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 223 Jonathan Druart 2015-11-05 11:07:55 UTC
Created attachment 44473 [details] [review]
Bug 14544: Get rid of GetSomeShelfNames

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 224 Jonathan Druart 2015-11-05 11:08:00 UTC
Created attachment 44474 [details] [review]
Bug 14544: Move HandleDelBorrower to C4::Members

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 225 Jonathan Druart 2015-11-05 11:08:06 UTC
Created attachment 44475 [details] [review]
Bug 14544: Get rid of C4::VirtualShelves and C4::VirtualShelves::Page

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 226 Jonathan Druart 2015-11-05 11:08:11 UTC
Created attachment 44476 [details] [review]
Bug 14544: Remove wrong tips about permissions

The owner of a list should not need any permission to remove contents.

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 227 Jonathan Druart 2015-11-05 11:08:16 UTC
Created attachment 44477 [details] [review]
Bug 14544: Fix regression on adding a new list

The user should not be allowed to create a new list and get a friendly
error message if he tries.

The "New list" links are removed.

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 228 Jonathan Druart 2015-11-05 11:08:22 UTC
Created attachment 44478 [details] [review]
Bug 14544: DB Changes: Set a default value for sortfield

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 229 Jonathan Druart 2015-11-05 11:08:27 UTC
Created attachment 44479 [details] [review]
Bug 14544 - Handle database error when creating a shelf with DB admin account

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I don't think this is useful, it will be a mess if we have to check this
case everywhere. But it works!
I let the cut to the QAer or RM.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 230 Jonathan Druart 2015-11-05 11:08:32 UTC
Created attachment 44480 [details] [review]
Bug 14544: [QA Follow-up] Fix warning about transaction

When running Virtualshelves.t, you have this warning:
DBD::mysql::db begin_work failed: Already in a transaction at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1339.

By moving the DBI lines after creating the TestBuilder object, the warning
will be gone.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 231 Jonathan Druart 2015-11-05 11:08:37 UTC
Created attachment 44481 [details] [review]
Bug 14544: Add missing biblio <=> biblioitems <=> items relationships to the schema

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Adjusted by a follow-up.
Comment 232 Jonathan Druart 2015-11-05 11:08:43 UTC
Created attachment 44482 [details] [review]
Bug 14544: QA fixes - some minor bug fixes

See comment 171 and 172 for more details.

This patch mainly fixes typo or silly error in templates.

It also uses the relationships added by previous patch to join the
biblioitems and items tables (changes in opac-shelves.pl and
shelves.pl).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 233 Jonathan Druart 2015-11-05 11:08:48 UTC
Created attachment 44483 [details] [review]
Bug 14544: [QA Follow-up] Added some test descriptions

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 234 Jonathan Druart 2015-11-05 11:08:54 UTC
Created attachment 44484 [details] [review]
Bug 14544: [QA Follow-up] DBIx::Class changes

One of the follow-ups added a belongs_to from Biblioitem to Item just
as biblioitemnumber was a foreign key to Item. The relation goes the
other way.

As a temporary measure, I added set_primary_key in the local section of
Virtualshelfcontent. Should be moved later with a real db rev.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 235 Jonathan Druart 2015-11-05 11:08:59 UTC
Created attachment 44485 [details] [review]
Bug 14544: Fix 'back to lists' link
Comment 236 Jonathan Druart 2015-11-05 11:09:04 UTC
Created attachment 44486 [details] [review]
Bug 14544: Fixes regression: display author and dateadded on the list view [intranet]
Comment 237 Jonathan Druart 2015-11-05 11:09:09 UTC
Created attachment 44487 [details] [review]
Bug 14544: Fix 'Back to lists' links on sharing a shelf
Comment 238 Tomás Cohen Arazi 2015-11-05 13:37:33 UTC
Patches pushed to master.

Thanks Jonathan!
Comment 239 Jonathan Druart 2015-11-11 13:19:51 UTC
Created attachment 44758 [details] [review]
Bug 14544: Remove debug line
Comment 240 Tomás Cohen Arazi 2015-11-11 14:11:16 UTC
Thanks for the followup, Jonathan!
Comment 241 Jonathan Druart 2015-11-13 15:03:22 UTC Comment hidden (obsolete)
Comment 242 Jonathan Druart 2015-11-13 16:50:25 UTC Comment hidden (obsolete)
Comment 243 Bernardo Gonzalez Kriegel 2015-11-14 14:00:00 UTC Comment hidden (obsolete)
Comment 244 Katrin Fischer 2015-11-15 23:05:34 UTC
Created attachment 44872 [details] [review]
[PASSED QA] Bug 14544: Fix regression on sending an email from staff

The shelfnumber was not filled and result in a software error
Can't call method "get_contents" on an undefined value at
/home/koha/src/virtualshelves/sendshelf.pl line 74.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Whith patch mail is sent.
No errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 245 Katrin Fischer 2015-11-15 23:05:56 UTC
Last patch needs to be pushed.
Comment 246 Tomás Cohen Arazi 2015-11-16 18:58:40 UTC
Followup pushed to master.

Thanks Jonathan, again!
Comment 247 Jacek Ablewicz 2016-06-08 08:03:41 UTC
This patch added a new dependency (Exception::Class, version 1.39) - is version 1.39+ really necessary? Eq., in Debian Jessie, off-the-shelf Exception::Class is a bit older (1.38).

Upgrade attempt from 3.22.6 -> 16.05.01 (on Jessie, from packages) does not trigger installation of the more recent version (I guess it's not packaged) - looks like 1.38 will be listed as unsatisfied dependency after an upgrade?
Comment 248 Jonathan Druart 2016-06-08 13:55:49 UTC
(In reply to Jacek Ablewicz from comment #247)
> This patch added a new dependency (Exception::Class, version 1.39) - is
> version 1.39+ really necessary? Eq., in Debian Jessie, off-the-shelf
> Exception::Class is a bit older (1.38).
> 
> Upgrade attempt from 3.22.6 -> 16.05.01 (on Jessie, from packages) does not
> trigger installation of the more recent version (I guess it's not packaged)
> - looks like 1.38 will be listed as unsatisfied dependency after an upgrade?

I have just opened bug 16695. Thanks for reporting this!