Right now it allows the Koha db admin login to attempt adding a new list. Only while trying to save it does it inform that it is not possible to do so. This is should be made pre-emptive right after the "+ New list" option is clicked.
Created attachment 62348 [details] [review] Bug 17218: Stop DB user from clicking 'New list' button To test: 1) Log in as DB user 2) Go to Lists -> New list 3) Attempt to create a new list 4) Notice that you are only stopped once you've submitted the form 5) Go back to lists, apply patch and refresh page 6) Notice the 'New list' button is now disabled 7) Click it and confirm alert shows up Sponsored-by: Catalyst IT
This works well (although it needs a quick rebase). But I think there should also be a warning on the list add page (/virtualshelves/shelves.pl?op=add_form&referer=add_form) in case the user navigates directly to it.
Created attachment 66604 [details] [review] Bug 17218: [FOLLOW-UP] Stop DB user from adding list by forcing URL To test: 1) Go to the url /virtualshelves/shelves.pl?op=add_form&referer=add_form 2) Confirm an error message shows and the Save button cannot be clicked Sponsored-by: Catalyst IT
Created attachment 68155 [details] [review] Bug 17218: Stop DB user from clicking 'New list' button To test: 1) Log in as DB user 2) Go to Lists -> New list 3) Attempt to create a new list 4) Notice that you are only stopped once you've submitted the form 5) Go back to lists, apply patch and refresh page 6) Notice the 'New list' button is now disabled 7) Click it and confirm alert shows up Sponsored-by: Catalyst IT Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 68156 [details] [review] Bug 17218: [FOLLOW-UP] Stop DB user from adding list by forcing URL To test: 1) Go to the url /virtualshelves/shelves.pl?op=add_form&referer=add_form 2) Confirm an error message shows and the Save button cannot be clicked Sponsored-by: Catalyst IT Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
I didn't realize at first that this was for the staff side. :)
Created attachment 72298 [details] [review] Bug 17218: Stop DB user from clicking 'New list' button To test: 1) Log in as DB user 2) Go to Lists -> New list 3) Attempt to create a new list 4) Notice that you are only stopped once you've submitted the form 5) Go back to lists, apply patch and refresh page 6) Notice the 'New list' button is now disabled 7) Click it and confirm alert shows up Sponsored-by: Catalyst IT Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 72299 [details] [review] Bug 17218: [FOLLOW-UP] Stop DB user from adding list by forcing URL To test: 1) Go to the url /virtualshelves/shelves.pl?op=add_form&referer=add_form 2) Confirm an error message shows and the Save button cannot be clicked Sponsored-by: Catalyst IT Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment on attachment 72298 [details] [review] Bug 17218: Stop DB user from clicking 'New list' button Review of attachment 72298 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/includes/virtualshelves-toolbar.inc @@ +10,5 @@ > + }); > + [% END %] > +}); > + > +[% IF op == 'view' %] I don't understand this whole IF block's purpose. It seems like new unrelated code.
http://localhost:8081/cgi-bin/koha/virtualshelves/addbybiblionumber.pl?newshelf=1&biblionumber=1 When you view the contents of an existing public list, there is a drop-down to add to cart or a new list. That creates a bunch of possible explosion points, like the kohadevbox link above.
Created attachment 72495 [details] [review] Bug 17218: [FOLLOW-UP] Removing pointless code and putting checks in missed places To test: 1) Log in as admin 2) View contents of a public list 3) Click drop-down for 'Add to' - there should be no [New list] option 4) Click on an item in the list 5) Click drop-down for 'Add to' in nav bar, click List 6) There should be no option to add a new list in the pop-up window 7) Do a catalog search that will return multiple results 8) Click drop-down for 'Add to list' - there should be no New list option Sponsored-by: Catalyst IT
Created attachment 72506 [details] [review] Bug 17218: Stop DB user from clicking 'New list' button To test: 1) Log in as DB user 2) Go to Lists -> New list 3) Attempt to create a new list 4) Notice that you are only stopped once you've submitted the form 5) Go back to lists, apply patch and refresh page 6) Notice the 'New list' button is now disabled 7) Click it and confirm alert shows up Sponsored-by: Catalyst IT Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 72507 [details] [review] Bug 17218: [FOLLOW-UP] Stop DB user from adding list by forcing URL To test: 1) Go to the url /virtualshelves/shelves.pl?op=add_form&referer=add_form 2) Confirm an error message shows and the Save button cannot be clicked Sponsored-by: Catalyst IT Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 72508 [details] [review] Bug 17218: [FOLLOW-UP] Removing pointless code and putting checks in missed places To test: 1) Log in as admin 2) View contents of a public list 3) Click drop-down for 'Add to' - there should be no [New list] option 4) Click on an item in the list 5) Click drop-down for 'Add to' in nav bar, click List 6) There should be no option to add a new list in the pop-up window 7) Do a catalog search that will return multiple results 8) Click drop-down for 'Add to list' - there should be no New list option Sponsored-by: Catalyst IT Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
NOTE: Javascript error, text/javascript in script tag not necessary for HTML5, and unless can get alert working I'll live with the disorienting click loading a page from a disabled button.
Moving to BLOCKED, since bug 20489 will no longer allow DB admin logins.
I believe this is resolved with bug 20489.
I've added the 17.11 RM, Fridolin Somers, to this bug. I'll let the RM decide if this is worthy of leaving as Resolved Fixed, or whether this should be put into 17.11, because if we still have an RM for 17.11, then the issue was not resolved in all supported versions.
(In reply to M. Tompsett from comment #18) > I've added the 17.11 RM, Fridolin Somers, to this bug. I'll let the RM > decide if this is worthy of leaving as Resolved Fixed, or whether this > should be put into 17.11, because if we still have an RM for 17.11, then the > issue was not resolved in all supported versions. Indeed, i'm on RMaint for 17.11, but this is a real enhancement so I decide not to backport.