You can create hierarchical groups of libraries you are not allowed to add several times the same library to a given group/subgroup. The libraries that are part of the group is not displayed on the interface, to prevent the user to select it. However this restriction is only done template-side, the controller and module does not handle it. To recreate the problem: - Go to Home › Administration › Library groups - Add a group - Add a library - reload the page several times => The same library appears several times in the group Note that we should also replace the "Add" and "Remove" links with buttons.
Created attachment 96199 [details] [review] Bug 21674: DB changes
Created attachment 96200 [details] [review] Bug 21674: Prevent to insert twice the same library in a group You can create hierarchical groups of libraries you are not allowed to add several times the same library to a given group/subgroup. The libraries that are part of the group is not displayed on the interface, to prevent the user to select it. However this restriction is only done template-side, the controller and module does not handle it. To prevent that this patchset enforces the constraint at DB level, and display a message to the end user. Test plan: - Go to Home › Administration › Library groups - Add a group - Add a library - reload the page several times => Without this patch the same library appears several times in the group => With this patch applied you will see a friendly error message
Tested on a sandbox, did not work
Created attachment 97658 [details] [review] Bug 21674: Prevent to insert twice the same library in a group You can create hierarchical groups of libraries you are not allowed to add several times the same library to a given group/subgroup. The libraries that are part of the group is not displayed on the interface, to prevent the user to select it. However this restriction is only done template-side, the controller and module does not handle it. To prevent that this patchset enforces the constraint at DB level, and display a message to the end user. Test plan: - Go to Home › Administration › Library groups - Add a group - Add a library - reload the page several times => Without this patch the same library appears several times in the group => With this patch applied you will see a friendly error message
(In reply to Nick Clemens from comment #3) > Tested on a sandbox, did not work Works for me, please try again with this rebased patch.
If a library is duplicated in the group before this patch then the DB update fails - we need to cleanup before adding the constraint
Created attachment 97667 [details] [review] Bug 21674: Remove duplicates from library_groups before adding the uniq constraint To avoid to make it fails if duplicates existed prior to this change
(In reply to Jonathan Druart from comment #7) > Created attachment 97667 [details] [review] [review] > Bug 21674: Remove duplicates from library_groups before adding the uniq > constraint > > To avoid to make it fails if duplicates existed prior to this change There is certainly a more efficient (SQL) way to do that.
(In reply to Jonathan Druart from comment #8) > (In reply to Jonathan Druart from comment #7) > > Created attachment 97667 [details] [review] [review] [review] > > Bug 21674: Remove duplicates from library_groups before adding the uniq > > constraint > > > > To avoid to make it fails if duplicates existed prior to this change > > There is certainly a more efficient (SQL) way to do that. DELETE FROM library_groups WHERE id NOT IN (SELECT MIN(id) FROM library_groups WHERE parent_id IS NOT NULL GROUP BY parent_id,branchcode) AND parent_id IS NOT NULL;
Created attachment 97811 [details] [review] Bug 21674: Remove duplicates from library_groups before adding the uniq constraint To avoid to make it fails if duplicates existed prior to this change Credit for the SQL query goes to Nick, thanks!
Created attachment 97837 [details] [review] Bug 21674: DB changes Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 97838 [details] [review] Bug 21674: Prevent to insert twice the same library in a group You can create hierarchical groups of libraries you are not allowed to add several times the same library to a given group/subgroup. The libraries that are part of the group is not displayed on the interface, to prevent the user to select it. However this restriction is only done template-side, the controller and module does not handle it. To prevent that this patchset enforces the constraint at DB level, and display a message to the end user. Test plan: - Go to Home › Administration › Library groups - Add a group - Add a library - reload the page several times => Without this patch the same library appears several times in the group => With this patch applied you will see a friendly error message Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 97839 [details] [review] Bug 21674: Remove duplicates from library_groups before adding the uniq constraint To avoid to make it fails if duplicates existed prior to this change Credit for the SQL query goes to Nick, thanks! Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 98170 [details] [review] Bug 21674: DB changes Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 98171 [details] [review] Bug 21674: Prevent to insert twice the same library in a group When you create hierarchical groups of libraries you are not allowed to add several times the same library to a given group/subgroup. The libraries that are part of the group is not displayed on the interface, to prevent the user to select it. However this restriction is only done template-side, the controller and module does not handle it. To prevent that this patchset enforces the constraint at DB level, and display a message to the end user. Test plan: - Go to Home › Administration › Library groups - Add a group - Add a library - reload the page several times => Without this patch the same library appears several times in the group => With this patch applied you will see a friendly error message Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 98172 [details] [review] Bug 21674: Remove duplicates from library_groups before adding the uniq constraint To avoid to make it fails if duplicates existed prior to this change Credit for the SQL query goes to Nick, thanks! Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Nice teamwork guys.. all works as expected. Passing QA
Nice work everyone! Pushed to master for 20.05
I'm getting a wierd DB error on this one: DBD::mysql::db do failed: Table 'library_groups' is specified twice, both as a target for 'DELETE' and as a separate source for data [for Statement " DELETE FROM library_groups WHERE id NOT IN ( SELECT MIN(id) FROM library_groups WHERE parent_id IS NOT NULL GROUP BY parent_id, branchcode ) AND parent_id IS NOT NULL; "] at ./installer/data/mysql/updatedatabase.pl line 20666.
Created attachment 98224 [details] [review] Bug 21674: (RM follow-up) Fix updatedatabase error