Bug 19343 - Private lists displayed in search results list
Summary: Private lists displayed in search results list
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Lists (show other bugs)
Version: 17.05
Hardware: All All
: P5 - low major (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 16551
Blocks:
  Show dependency treegraph
 
Reported: 2017-09-19 14:33 UTC by SaCa
Modified: 2017-10-14 11:55 UTC (History)
4 users (show)

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


Attachments
Private list appears in the search results list (272.15 KB, image/jpeg)
2017-09-19 14:33 UTC, SaCa
Details
Bug 19343: [16.11.X] Remove private lists with edit permission from search results (997 bytes, patch)
2017-10-03 14:03 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19343: [17.05.X] Remove private lists with edit permission from search results (997 bytes, patch)
2017-10-03 14:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19343: Add tests - do not display a list when private (978 bytes, patch)
2017-10-03 14:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19343: [17.05.X] Add tests (846 bytes, patch)
2017-10-03 14:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19343: [16.11.X] Add tests (846 bytes, patch)
2017-10-03 15:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19343: [16.11.X] Remove private lists with edit permission from search results (1.05 KB, patch)
2017-10-05 07:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19343: [17.05.X] Remove private lists with edit permission from search results (1.05 KB, patch)
2017-10-05 07:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.