Bug 19343

Summary: Private lists displayed in search results list
Product: Koha Reporter: SaCa <sanja.cancar>
Component: ListsAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: fridolin.somers, jonathan.druart, katrin.fischer, m.de.rooy, mtj
Version: 17.05   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18228
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 16551    
Bug Blocks:    
Attachments: Private list appears in the search results list
Bug 19343: [16.11.X] Remove private lists with edit permission from search results
Bug 19343: [17.05.X] Remove private lists with edit permission from search results
Bug 19343: Add tests - do not display a list when private
Bug 19343: [17.05.X] Add tests
Bug 19343: [16.11.X] Add tests
Bug 19343: [16.11.X] Remove private lists with edit permission from search results
Bug 19343: [17.05.X] Remove private lists with edit permission from search results

Description SaCa 2017-09-19 14:33:10 UTC
Created attachment 67215 [details]
Private list appears in the search results list

Hi all,

it seems that there is a bug when searching for titles on the opac.

On the search results lists you receive the information about lists that are set on private - the lists are from other users. Attached you will see a search results list that should not contain the information about this list. The list is from another user and set on private. Also the information is not displayed when you open the bibliographic record. And when trying to click on it, you receive this statement "You do not have permission to view this list.".


It seems that the only difference to other lists is that the permissions are set to "allow anyone else to add entries". When I change this setting, the list is not displayed in the search results anymore.

  
Although from my point of view the “private” setting should overrule the other settings, this isn't currently possible and would require some development work.

Kind regards,

Sanja
Comment 1 Marcel de Rooy 2017-10-03 14:03:32 UTC
Created attachment 67550 [details] [review]
Bug 19343: [16.11.X] Remove private lists with edit permission from search results

If the list is not shared, it should not be listed. We only need to remove
the allow_add line.
Comment 2 Marcel de Rooy 2017-10-03 14:10:40 UTC
Created attachment 67552 [details] [review]
Bug 19343: [17.05.X] Remove private lists with edit permission from search results

If the list is not shared, it should not be listed. We only need to remove
the allow_add line.
Comment 3 Marcel de Rooy 2017-10-03 14:12:43 UTC
(In reply to SaCa from comment #0)
> it seems that there is a bug when searching for titles on the opac.
> 
> On the search results lists you receive the information about lists that are
> set on private - the lists are from other users. Attached you will see a
> search results list that should not contain the information about this list.
> The list is from another user and set on private. Also the information is
> not displayed when you open the bibliographic record. And when trying to
> click on it, you receive this statement "You do not have permission to view
> this list.".
> 
> 
> It seems that the only difference to other lists is that the permissions are
> set to "allow anyone else to add entries". When I change this setting, the
> list is not displayed in the search results anymore.
> 
>   
> Although from my point of view the “private” setting should overrule the
> other settings, this isn't currently possible and would require some
> development work.

Good observation, Sanja.
This bug has been solved in master (17.11) by bug 18228. It is present in 16.11 and 17.05 (not in 16.05).
Are you in the position to test the attached patch? It should resolve your problem under 16.11. If not, you will have to wait until it gets attention from the release maintainer(s).
Comment 4 Marcel de Rooy 2017-10-03 14:17:52 UTC
Fridolin and Katrin:
This resolves a bug that is only on 17.05/16.11. It is a rather trivial oneliner. Can you test it yourself or does it need further outside attention?
Comment 5 Jonathan Druart 2017-10-03 14:57:11 UTC
Created attachment 67559 [details] [review]
Bug 19343: Add tests - do not display a list when private
Comment 6 Jonathan Druart 2017-10-03 14:58:22 UTC
Created attachment 67560 [details] [review]
Bug 19343: [17.05.X] Add tests
Comment 7 Jonathan Druart 2017-10-03 15:02:01 UTC
Created attachment 67561 [details] [review]
Bug 19343: [16.11.X] Add tests
Comment 8 Jonathan Druart 2017-10-03 15:04:11 UTC
Signed off on all patches (lazy to re-upload them with the signed-off-by line)
Comment 9 Jonathan Druart 2017-10-03 15:04:30 UTC
Upping severity a bit.
Comment 10 Marcel de Rooy 2017-10-04 11:17:59 UTC
-            allow_add    => 1,
+            allow_change_from_owner => 1,
+            allow_change_from_others => 1,

THis change in master does not apply.
And I wonder if it makes sense too, since not adding these lines does not make a difference. Test still passes. Please adjust.
Comment 11 Jonathan Druart 2017-10-04 15:51:00 UTC
(In reply to Marcel de Rooy from comment #10)
> -            allow_add    => 1,
> +            allow_change_from_owner => 1,
> +            allow_change_from_others => 1,
> 
> THis change in master does not apply.
> And I wonder if it makes sense too, since not adding these lines does not
> make a difference. Test still passes. Please adjust.

Yes right, this patch is not needed.
Comment 12 Marcel de Rooy 2017-10-05 07:17:56 UTC
(In reply to Jonathan Druart from comment #11)
> (In reply to Marcel de Rooy from comment #10)
> > -            allow_add    => 1,
> > +            allow_change_from_owner => 1,
> > +            allow_change_from_others => 1,
> > 
> > THis change in master does not apply.
> > And I wonder if it makes sense too, since not adding these lines does not
> > make a difference. Test still passes. Please adjust.
> 
> Yes right, this patch is not needed.

Obsoleting the other two patches too for the same reason.
Comment 13 Marcel de Rooy 2017-10-05 07:18:49 UTC
(In reply to Jonathan Druart from comment #8)
> Signed off on all patches (lazy to re-upload them with the signed-off-by
> line)

Will add your signoff line and update status now.
Comment 14 Marcel de Rooy 2017-10-05 07:20:26 UTC
Created attachment 67630 [details] [review]
Bug 19343: [16.11.X] Remove private lists with edit permission from search results

If the list is not shared, it should not be listed. We only need to remove
the allow_add line.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Marcel de Rooy 2017-10-05 07:21:26 UTC
Created attachment 67631 [details] [review]
Bug 19343: [17.05.X] Remove private lists with edit permission from search results

If the list is not shared, it should not be listed. We only need to remove
the allow_add line.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 16 Marcel de Rooy 2017-10-05 07:24:10 UTC
Fridolin: It is your turn now. The patch is not needed in master, but it is in 17.05.
Comment 17 Jonathan Druart 2017-10-05 13:51:05 UTC
(In reply to Marcel de Rooy from comment #12)
> Obsoleting the other two patches too for the same reason.

I unobsoleted them because the change in the tests makes sense to me, it fails without the fix.

    #   Failed test 'Only shelf1 should be displayed for patron 1 and biblio 1'
    #   at t/db_dependent/Virtualshelves.t line 481.
    #          got: '2'
    #     expected: '1'
    # Looks like you failed 1 test of 9.
Comment 18 Fridolin Somers 2017-10-10 08:32:20 UTC
Pushed to 17.05.x, will be in 17.05.05.
Comment 19 Katrin Fischer 2017-10-14 11:55:27 UTC
These patches have been pushed to 16.11.x and will be in 16.11.13.
Comment 20 Mason James 2017-11-23 23:10:19 UTC
Pushed to 16.05.x, for 16.05.18 release