Hi Marjorie,

some ideas:
- do you have superlibrarian permission?
- are your funds 'limited to' the library?

Thanks Katrin.
Yes, i have superlibrarian permissions and funds are limited to each branch.
May i test something else?

Marjorie

Maybe try removing your superlibrairan flag for a test? And keep all other permissions? I think that could be the difference, that a superlibriarian can always see the funds now.

(In reply to Katrin Fischer from comment #3)
> Maybe try removing your superlibrairan flag for a test? And keep all other
> permissions? I think that could be the difference, that a superlibriarian
> can always see the funds now.

I tested several flags but nothing change on acquisition home.
Have you tested it?

Hi Marjorie, I did test now - and I think you are right. Something about the visibility is broken. I removed the permission to manage all funds, I am not a superlibrarian, the fund access is restricted to another branch and I can still see it :( I am quite sure this used to work differently and it quite a problem. The view is also the same on the acquisition start page and the fund list for the budget.

Hm, I tried a git bisect, because I wondered when it changed and the change seems to have been introduced by:

203757e353cbbb2add934523fdbd7113c6180207 is the first bad commit
commit 203757e353cbbb2add934523fdbd7113c6180207
Author: Julian Maurice <julian.maurice@biblibre.com>
Date:   Fri May 18 09:12:43 2012 +0200

    Bug 7304: More permissions for budgets

Note: IndependentBranches also doesn't make any difference.

I have upgraded the severity, as this is quite a problem for bigger libraries that don't want other branches to see their funds.

Marjorie,
Try to define an owner for the fund. If no owner is defined, it seems there is no restriction.

(In reply to Jonathan Druart from comment #8)
> Marjorie,
> Try to define an owner for the fund. If no owner is defined, it seems there
> is no restriction.

Jonathan,
I defined an owner for the fund but for me it is the same on acquisition home page, i can see funds of the other branches and of the other owners.
I should see only the budgets that I am an owner, right?
We need to change something in the drop down menu restrictions?

Thanks!

Marjorie

(In reply to Marjorie Barry-Vila from comment #11)
> I defined an owner for the fund but for me it is the same on acquisition
> home page, i can see funds of the other branches and of the other owners.
> I should see only the budgets that I am an owner, right?
> We need to change something in the drop down menu restrictions?

Could you confirm the issue still appears with these patches?

(In reply to Jonathan Druart from comment #12)
> (In reply to Marjorie Barry-Vila from comment #11)
> > I defined an owner for the fund but for me it is the same on acquisition
> > home page, i can see funds of the other branches and of the other owners.
> > I should see only the budgets that I am an owner, right?
> > We need to change something in the drop down menu restrictions?
> 
> Could you confirm the issue still appears with these patches?

Hi Jonathan,
I tested it on a sandbox but it is not working for me. I think it's me I do something wrong. I added an owner, a library and a restriction to only owner and i can still see fund of the other library.
I changed flags too but without results.

See the video attached, maybe you will see what i do wrong

Marjorie

(In reply to Jonathan Druart from comment #12)
> (In reply to Marjorie Barry-Vila from comment #11)
> > I defined an owner for the fund but for me it is the same on acquisition
> > home page, i can see funds of the other branches and of the other owners.
> > I should see only the budgets that I am an owner, right?
> > We need to change something in the drop down menu restrictions?
> 
> Could you confirm the issue still appears with these patches?

see the video here http://bit.ly/1mU7Hlt

Marjorie

(In reply to Marjorie Barry-Vila from comment #13)
> Hi Jonathan,
> I tested it on a sandbox but it is not working for me. I think it's me I do
> something wrong. I added an owner, a library and a restriction to only owner
> and i can still see fund of the other library.
> I changed flags too but without results.

The "test" patron has the manage_all_buggets permission. So he can edit all budgets, even if a restriction is set.

(In reply to Jonathan Druart from comment #15)
> (In reply to Marjorie Barry-Vila from comment #13)
> > Hi Jonathan,
> > I tested it on a sandbox but it is not working for me. I think it's me I do
> > something wrong. I added an owner, a library and a restriction to only owner
> > and i can still see fund of the other library.
> > I changed flags too but without results.
> 
> The "test" patron has the manage_all_buggets permission. So he can edit all
> budgets, even if a restriction is set.

ok! it's works! Sorry for all my comments!
I signed off.

Marjorie

Patch tested with a sandbox, by marjorie barry-vila <marjorie.barry-vila@ccsr.qc.ca>

I think this works, but I am not sure that making the owner mandatory is the right fix.

We have funds in Koha that have been created before the owner feature has been introduced and we can't expect libraries to add an owner to all those now to bring back a feature that worked in the past. Especially if the owner has no real meaning - it would just be added, to make the library restriction take effect again.

I think we need to fix the visibility check to work without an owner.

The "owner" field has been introduced in 3.01.00.077 and the restriction by owner already existed before bug 7304.
Could you please detail what could be broken?

Ok, maybe if a restriction exist on "Owner, users and library" and no owner is defined.

Hi Jonathan, the git bisect I did led me back to bug 7304 - so somehow the behaviour change was introduced at this point. I am not trying to be difficult here, but I think fixing the logic would really be better. 

If you select owners, users and library, and there are no users, it still works - why rely on an owner if the library is set?

Created attachment 27282 [details] [review]
[PASSED QA] Bug 12019: FIX budget_owner_id node is duplicated

The input witht the budget_owner_id id is duplicated.
It breaks the jQuery selector
  $('#budget_owner_id').val('');

Signed-off-by: marjorie barry-vila <marjorie.barry-vila@ccsr.qc.ca>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 27283 [details] [review]
[PASSED QA] Bug 12019: It should not be mandatory to define an owner if a fund is restricted

Before this patch, the C4::Budgets::CanUserUseBudget assumed that
budget_owner_id was set if a restriction (budget_permission) exists.
see
        && $budget->{budget_owner_id}
        && $budget->{budget_owner_id} != $borrower->{borrowernumber}

Actually a restriction could exists on users and/or library without
being forced to define an owner.

Test plan:
Create a fund A without restriction
Create a fund B restricted to an owner
Create a fund C restricted to a non defined owner
Create a fund D restricted to owner and users (try defining/no
defining an owner and/or users)
Create a fund E restricted to owner, users and library (try
defining/no defined an owner and/or users)

With different logged in users, try to show/edit these differents funds.
The restriction should be correctly applied.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Tested various permission combinatons, visibility of funds is now ok.
- not superlibrarian, no buget_manage_all,
  no owner, no users, no library, no restrictions on the fund
  = visible
- changed: library = staff patron library,
  restriction = Owner, users and library
  = visible
- changed: library = not staff patron library
  = invisible
- changed: budget_manage_all
  = visible
- changed: owner = staff patron
  no budget_manage_all
  = visible
- changed: no owner, user = staff patron
  = visible
- changed: no user, owner = another user, restriction = owner
  = invisible
- changed: budget_manage_all
  = visible
- changed: no budget_manage_all but superlibrarian
  = visible
...

Passes tests and QA script, also t/Budgets/*

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Pushed to master.  Thanks, Jonathan!