|
Description
Owen Leonard
2017-01-11 18:30:07 UTC
Created attachment 59138 [details] [review] Bug 17886 Don't show option to add to existing list if there are no lists Test Plan 1: Make sure that you don't have any lists and then add a Biblical Record to a list 2: You should see that you can add it to an existing list even though you don't have one. 3: Apply patch 4: Add a Biblical record and if you don't have a list then you shouldn't see the add to existing list. 5: Create a list and then add a Bibical Record and you should see the add to existing list Created attachment 59142 [details] [review] Bug 17886 Don't show option to add to existing list if there are no lists Test Plan 1: Make sure that you don't have any lists and then add a Biblical Record to a list 2: You should see that you can add it to an existing list even though you don't have one. 3: Apply patch 4: Add a Biblical record and if you don't have a list then you shouldn't see the add to existing list. 5: Create a list and then add a Bibical Record and you should see the add to existing list Signed-off-by: Claire Gravely <claire_gravely@hotmail.com> Hello Caitling, we have the same problem at the OPAC, could you fix it as well? Created attachment 59282 [details] [review] Bug 17886 Don't show option to add to existing list if there are no lists Test Plan 1: Make sure that you don't have any lists and then add a Bibliographical Record to a list in OPAC 2: You should see that you can add it to an existing list even though you don't have one. 3: Apply patch 4: Add a Bibliographical record in OPAC and if you don't have a list then you shouldn't see the add to existing list. 5: Create a list and then add a Bibiographical Record in OPAC and you should see the add to existing list Comment on attachment 59282 [details] [review] Bug 17886 Don't show option to add to existing list if there are no I believe this is the wrong solution. It is much simpler. Created attachment 59381 [details] [review] Bug 17886: Forgot to count the lists shared to patron The private_shelves_shared_with_me structure is defined, but has 0 entries. The missing .count is the issue. TEST PLAN --------- 1) Make sure no lists exist at all. 2) Make there is at least one findable biblio record entered 3) Find that biblio record in the OPAC 4) Click 'Save to your lists' -- Notice that the whole save to list section is there. 5) apply this patch 6) Click 'Cancel' 7) Click 'Save to your lists' -- Notice you can only add now, which makes sense. 8) run koha qa test tools. (In reply to M. Tompsett from comment #5) > I believe this is the wrong solution. It is much simpler. Since you're obsoleting a patch from an Academy student in favor of your own I think it's worthwhile to give a detailed explanation of your reasoning. (In reply to Owen Leonard from comment #7) > (In reply to M. Tompsett from comment #5) > > > I believe this is the wrong solution. It is much simpler. > > Since you're obsoleting a patch from an Academy student in favor of your own > I think it's worthwhile to give a detailed explanation of your reasoning. Sorry for taking so long to notice this. Valid point, Owen. The problem is not the 'Add To' heading section (as hinted at by the 3rd obsolete patch). The problem is not around line 40ish (as hinted at by the 1st two obsolete patches). The problem is when there are no lists, it makes no sense to show that you can select a list. The problem is the code correctly checks the count method of the two shelf types, but fails to do so on the third on line 26. By simplying adding .count to the third shelf type, the options available are properly shown. There is no need to add a condition. The existing condition was broken. Though, what I do miss about the first two patches are corrections in indentation. Created attachment 61117 [details] [review] Bug 17886: Forgot to count the lists shared to patron The private_shelves_shared_with_me structure is defined, but has 0 entries. The missing .count is the issue. TEST PLAN --------- 1) Make sure no lists exist at all. 2) Make there is at least one findable biblio record entered 3) Find that biblio record in the OPAC 4) Click 'Save to your lists' -- Notice that the whole save to list section is there. 5) apply this patch 6) Click 'Cancel' 7) Click 'Save to your lists' -- Notice you can only add now, which makes sense. 8) run koha qa test tools. Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> I am lost here... First patch fixed the staff interface only, now the patch fixes the opac interface only... Can we have both please? Created attachment 75640 [details] [review] Bug 17886: Don't show option to add to existing list if there are no lists This patch modifies the page for adding a title to a list so that the "Add to existing list" option doesn't appear if there are no lists defined. TEST PLAN --------- 1) Make sure no lists exist at all. 2) Make there is at least one findable biblio record entered 3) Find that biblio record in the OPAC 4) Click 'Save to your lists' -- Notice that the whole save to list section is there. 5) Apply this patch 6) Click 'Cancel' 7) Click 'Save to your lists' -- Notice you can only add now, which makes sense. 8) Run koha qa test tools. Perform the same tests in the staff client. Created attachment 76306 [details] [review] Bug 17886: Don't show option to add to existing list if there are no lists This patch modifies the page for adding a title to a list so that the "Add to existing list" option doesn't appear if there are no lists defined. TEST PLAN --------- 1) Make sure no lists exist at all. 2) Make there is at least one findable biblio record entered 3) Find that biblio record in the OPAC 4) Click 'Save to your lists' -- Notice that the whole save to list section is there. 5) Apply this patch 6) Click 'Cancel' 7) Click 'Save to your lists' -- Notice you can only add now, which makes sense. 8) Run koha qa test tools. Perform the same tests in the staff client. Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Followed the test plan and the patch works. OPAC seems to be ok in my tests. If there is still an issue I suggest solving it with a separate bug and patch to have this moving on. Created attachment 76428 [details] [review] Bug 17886: Don't show option to add to existing list if there are no lists This patch modifies the page for adding a title to a list so that the "Add to existing list" option doesn't appear if there are no lists defined. TEST PLAN --------- 1) Make sure no lists exist at all. 2) Make there is at least one findable biblio record entered 3) Find that biblio record in the OPAC 4) Click 'Save to your lists' -- Notice that the whole save to list section is there. 5) Apply this patch 6) Click 'Cancel' 7) Click 'Save to your lists' -- Notice you can only add now, which makes sense. 8) Run koha qa test tools. Perform the same tests in the staff client. Signed-off-by: Maryse Simard <maryse.simard@inlibro.com> Followed the test plan and the patch works. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Awesome work all! Pushed to master for 18.11 Pushed to 18.05.x for 18.05.02 Pushed to 17.11.x for 17.11.08 |