Bug 34631 - Independentbranches - Non-superuser should not be able to write Additional contents for other branches
Summary: Independentbranches - Non-superuser should not be able to write Additional co...
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Hammat wele
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-08-28 17:10 UTC by Hammat wele
Modified: 2025-04-27 17:35 UTC (History)
11 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes additional contents when the IndependentBranches system preference is enabled - staff that are not super librarians can now only add, edit, and delete additional contents for their library (Tools > Additional contents (news, HTML customizations, pages). Previously, they could edit additional contents for other libraries and content added by super librarians.
Version(s) released in:
Circulation function:


Attachments
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches (7.28 KB, patch)
2023-08-28 17:17 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches (5.86 KB, patch)
2023-08-28 17:23 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 34631: Independentbranches - rebase to master (6.16 KB, patch)
2024-03-05 17:20 UTC, Matthias Le Gac
Details | Diff | Splinter Review
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches (1.28 KB, patch)
2024-03-05 19:01 UTC, Matthias Le Gac
Details | Diff | Splinter Review
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches (10.97 KB, patch)
2024-03-06 20:56 UTC, Matthias Le Gac
Details | Diff | Splinter Review
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches (11.01 KB, patch)
2024-03-07 19:16 UTC, Myka Kennedy Stephens
Details | Diff | Splinter Review
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches (5.95 KB, patch)
2024-03-08 15:37 UTC, Matthias Le Gac
Details | Diff | Splinter Review
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches (5.99 KB, patch)
2024-03-09 01:05 UTC, Myka Kennedy Stephens
Details | Diff | Splinter Review
Bug 34631: (Follow up) Changed variable name (4.26 KB, patch)
2024-09-30 16:25 UTC, Sukhmandeep
Details | Diff | Splinter Review
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches (6.00 KB, patch)
2024-12-30 02:02 UTC, David Nind
Details | Diff | Splinter Review
Bug 34631: (Follow up) Changed variable name (4.31 KB, patch)
2024-12-30 02:02 UTC, David Nind
Details | Diff | Splinter Review
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches (6.01 KB, patch)
2024-12-30 02:20 UTC, David Nind
Details | Diff | Splinter Review
Bug 34631: (Follow up) Changed variable name (4.31 KB, patch)
2024-12-30 02:20 UTC, David Nind
Details | Diff | Splinter Review
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches (6.07 KB, patch)
2025-03-28 18:39 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 34631: (Follow up) Changed variable name (4.39 KB, patch)
2025-03-28 18:39 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches (6.07 KB, patch)
2025-04-03 18:15 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 34631: (Follow up) Changed variable name (4.39 KB, patch)
2025-04-03 18:16 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 34631: Add IndependentBranchesAdditionalcontents preference and some checks into additional-contents.pl (8.24 KB, patch)
2025-04-03 18:16 UTC, Hammat wele
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Hammat wele 2023-08-28 17:10:56 UTC
When syspref Independentbranches is enabled Non-superuser should not be able to write Additional contents for other branches

To reproduce:
1- Connect to staff interface as superlibrarian
2- Enable IndependentBranches preference
3- Go to Tools -> HTML customizations
4- Crete an additional content for all libraries
5- Create or edit a borrower
    5-1- Set permission for the borrower 
        - catalogue
        - edit_additional_contents
4- Log in to the staff interface with the non-superlibrarian staff user
5- Go to Tools -> HTML customizations
    --> you can delete or edit the additional contents created by superlibrarian for All libraries 
6- Click on New entry 
    --> you can add additional content for other branches
Comment 1 Hammat wele 2023-08-28 17:17:21 UTC Comment hidden (obsolete)
Comment 2 Hammat wele 2023-08-28 17:23:00 UTC
Created attachment 154890 [details] [review]
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches

When syspref Independentbranches is enabled Non-superuser should not be able to write Additional contents for other branches

To reproduce:
1- Connect to staff interface as superlibrarian
2- Enable IndependentBranches preference
3- Go to Tools -> HTML customizations
4- Crete an additional content for all libraries
5- Create or edit a borrower
    5-1- Set permission for the borrower
        - catalogue
        - edit_additional_contents
4- Log in to the staff interface with the non-superlibrarian staff user
5- Go to Tools -> HTML customizations
    --> you can delete or edit the additional contents created by superlibrarian for All libraries
6- Click on New entry
    --> you can add additional content for other branches
7- Apply the patch
8- Repeat step 5
    --> you cannot delete or edit the additional contents create by superlibrarian for All libraries
9- Repeat step 6
    --> you cannot add additional content for other branches
10- submit the form
    --> the branch of the created additional content is the current user branch
Comment 3 Myka Kennedy Stephens 2024-03-05 01:44:23 UTC
Attempted to test using git bz with Koha Testing Docker. Received following error:

Applying: Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches
Using index info to reconstruct a base tree...
M       koha-tmpl/intranet-tmpl/prog/en/modules/tools/additional-contents.tt
M       tools/additional-contents.pl
Falling back to patching base and 3-way merge...
Auto-merging tools/additional-contents.pl
CONFLICT (content): Merge conflict in tools/additional-contents.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/tools/additional-contents.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/tools/additional-contents.tt
error: Failed to merge in the changes.
Patch failed at 0001 Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches
Comment 4 Matthias Le Gac 2024-03-05 17:20:41 UTC
Created attachment 162787 [details] [review]
Bug 34631: Independentbranches - rebase to master

When syspref Independentbranches is enabled Non-superuser should not be able to write Additional contents for other branches

To reproduce:
1- Connect to staff interface as superlibrarian
2- Enable IndependentBranches preference
3- Go to Tools -> HTML customizations
4- Crete an additional content for all libraries
5- Create or edit a borrower
    5-1- Set permission for the borrower
        - catalogue
        - edit_additional_contents
4- Log in to the staff interface with the non-superlibrarian staff user
5- Go to Tools -> HTML customizations
    --> you can delete or edit the additional contents created by superlibrarian for All libraries
6- Click on New entry
    --> you can add additional content for other branches
7- Apply the patch
8- Repeat step 5
    --> you cannot delete or edit the additional contents create by superlibrarian for All libraries
9- Repeat step 6
    --> you cannot add additional content for other branches
10- submit the form
    --> the branch of the created additional content is the current user branch
Comment 5 Matthias Le Gac 2024-03-05 17:27:30 UTC
rebase to master
Comment 6 Myka Kennedy Stephens 2024-03-05 18:20:29 UTC
Applied patch and received an error when returning to the HTML Customizations tool in the non-superlibrarian login (additional-contents.pl) A PDF of the full error trace is available here: https://drive.google.com/file/d/1GNaNWN-sMIKQcYvdns5i5406qZ2iGw23/view?usp=sharing

The same error persists across News and Pages for both a superlibrarian login and a non-superlibrarian login.
Comment 7 Matthias Le Gac 2024-03-05 19:01:17 UTC
Created attachment 162788 [details] [review]
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches

When syspref Independentbranches is enabled Non-superuser should not be able to write Additional contents for other branches

To reproduce:
1- Connect to staff interface as superlibrarian
2- Enable IndependentBranches preference
3- Go to Tools -> HTML customizations
4- Crete an additional content for all libraries
5- Create or edit a borrower
    5-1- Set permission for the borrower
        - catalogue
        - edit_additional_contents
4- Log in to the staff interface with the non-superlibrarian staff user
5- Go to Tools -> HTML customizations
    --> you can delete or edit the additional contents created by superlibrarian for All libraries
6- Click on New entry
    --> you can add additional content for other branches
7- Apply the patch
8- Repeat step 5
    --> you cannot delete or edit the additional contents create by superlibrarian for All libraries
9- Repeat step 6
    --> you cannot add additional content for other branches
10- submit the form
    --> the branch of the created additional content is the current user branch
Comment 8 Myka Kennedy Stephens 2024-03-05 22:00:30 UTC
@Matthias, I'm still getting the same error when applying. Git bz applied both patches that are attached here. Either one of them is still producing the error or I'm doing something wrong with Koha Testing Docker. Please advise.
Comment 9 Lucas Gass (lukeg) 2024-03-05 22:07:49 UTC
Matthias, your patch sets now include merge markers. Please use the QA tool!
Comment 10 Matthias Le Gac 2024-03-06 20:56:04 UTC
Created attachment 162856 [details] [review]
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches

When syspref Independentbranches is enabled Non-superuser should not be able to write Additional contents for other branches

To reproduce:
1- Connect to staff interface as superlibrarian
2- Enable IndependentBranches preference
3- Go to Tools -> HTML customizations
4- Crete an additional content for all libraries
5- Create or edit a borrower
    5-1- Set permission for the borrower
        - catalogue
        - edit_additional_contents
4- Log in to the staff interface with the non-superlibrarian staff user
5- Go to Tools -> HTML customizations
    --> you can delete or edit the additional contents created by superlibrarian for All libraries
6- Click on New entry
    --> you can add additional content for other branches
7- Apply the patch
8- Repeat step 5
    --> you cannot delete or edit the additional contents create by superlibrarian for All libraries
9- Repeat step 6
    --> you cannot add additional content for other branches
10- submit the form
    --> the branch of the created additional content is the current user branch
Comment 11 Matthias Le Gac 2024-03-06 20:58:17 UTC
I think it's good now to test

sorry for the trouble I've caused by trying to rebase
Comment 12 Myka Kennedy Stephens 2024-03-06 23:57:15 UTC
Different error, this time when attempting to apply the patch using git bz:

Applying: Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/tools/additional-contents.tt).
error: could not build fake ancestor
Patch failed at 0001 Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches
Comment 13 Matthias Le Gac 2024-03-07 18:35:57 UTC
Bug 34631 - Independentbranches - Non-superuser should not be able to write Additional contents for other branches

162856 - Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches

I have no problem to apply the patch, I don't understand your problem. Your master is up to date ?
Comment 14 Myka Kennedy Stephens 2024-03-07 18:49:15 UTC
Matthias: I don't understand it either. I tried again just now. Fresh boot of Koha Testing Docker, fresh git pull, and an extra git reset just for extra measure. When I did the hard reset it said:
    HEAD is now at fee4368cd8 Bug 35930: Add guards for plugins_enabled

Applying the patch with git bz, I get the same errors as yesterday:
    error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/tools/additional-contents.tt).
    error: could not build fake ancestor

I was able to test and sign off on another patch for another bug yesterday, so I'm fairly certain the issue is with this patch and not my setup. I'm sorry I don't have any more insights than that.
Comment 15 Blou 2024-03-07 18:55:15 UTC
Myka, Bug 35930 has been on the stack for a while.  My grep tells me it's number 486 on the master stack.

It's possible to test plenty of issue with an old master, but when it fails, it fails.  The issue right now seems with your setup, I'm sorry.
Comment 16 Myka Kennedy Stephens 2024-03-07 19:16:12 UTC
Created attachment 162929 [details] [review]
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches

When syspref Independentbranches is enabled Non-superuser should not be able to write Additional contents for other branches

To reproduce:
1- Connect to staff interface as superlibrarian
2- Enable IndependentBranches preference
3- Go to Tools -> HTML customizations
4- Crete an additional content for all libraries
5- Create or edit a borrower
    5-1- Set permission for the borrower
        - catalogue
        - edit_additional_contents
4- Log in to the staff interface with the non-superlibrarian staff user
5- Go to Tools -> HTML customizations
    --> you can delete or edit the additional contents created by superlibrarian for All libraries
6- Click on New entry
    --> you can add additional content for other branches
7- Apply the patch
8- Repeat step 5
    --> you cannot delete or edit the additional contents create by superlibrarian for All libraries
9- Repeat step 6
    --> you cannot add additional content for other branches
10- submit the form
    --> the branch of the created additional content is the current user branch

Signed-off-by: Myka Kennedy Stephens <mkstephens@fosgail.org>
Comment 17 Myka Kennedy Stephens 2024-03-07 19:38:12 UTC
I've been doing git pulls outside of Koha testing docker, which was making it look like my Koha repository was updating, but I guess it wasn't. I did a git pull from within the ktd shell, applied the patch, and then it worked!

Applying the patch has the desired effect that a non-superuser from another branch has the option to preview HTML content but the edit and delete buttons are hidden from content for other branches and all libraries.

After signing-off, I am having some trouble with steps 9 and 10 of the test plan. After trying to save new content with the limited-access user, I get the following error: Template process failed: undef error - The method Koha::AdditionalContent->idnew is not covered by tests!

This error persists to the superuser login. When I do a hard reset to master, the error goes away and I can see the content that I attempted to add and save before I got the error.

Hoping QA or a second tester can look into this further. I'm leaving it as signed off, but please change if there's a more appropriate status.
Comment 18 Blou 2024-03-07 19:51:19 UTC
Well, I'll fail it myself :-P
Matthias, the code for additional-contents.pl has been all reformated, like you passed it in perltidy.  It makes it very hard to see the actual modifications.

Please resend the patch with no extra formating, please (beside the lines with actual changes).
Comment 19 Matthias Le Gac 2024-03-08 15:37:28 UTC
Created attachment 162972 [details] [review]
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches

When syspref Independentbranches is enabled Non-superuser should not be able to write Additional contents for other branches

To reproduce:
1- Connect to staff interface as superlibrarian
2- Enable IndependentBranches preference
3- Go to Tools -> HTML customizations
4- Crete an additional content for all libraries
5- Create or edit a borrower
    5-1- Set permission for the borrower
        - catalogue
        - edit_additional_contents
4- Log in to the staff interface with the non-superlibrarian staff user
5- Go to Tools -> HTML customizations
    --> you can delete or edit the additional contents created by superlibrarian for All libraries
6- Click on New entry
    --> you can add additional content for other branches
7- Apply the patch
8- Repeat step 5
    --> you cannot delete or edit the additional contents create by superlibrarian for All libraries
9- Repeat step 6
    --> you cannot add additional content for other branches
10- submit the form
    --> the branch of the created additional content is the current user branch
Comment 20 Matthias Le Gac 2024-03-08 15:42:12 UTC
Sorry for the confusion, I was working on an old master's DB, so I hadn't seen the change of a field in the DB that broke everything. Normally the problem is solved I managed to make the test plan.
Comment 21 Myka Kennedy Stephens 2024-03-09 01:05:56 UTC
Created attachment 162995 [details] [review]
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches

When syspref Independentbranches is enabled Non-superuser should not be able to write Additional contents for other branches

To reproduce:
1- Connect to staff interface as superlibrarian
2- Enable IndependentBranches preference
3- Go to Tools -> HTML customizations
4- Crete an additional content for all libraries
5- Create or edit a borrower
    5-1- Set permission for the borrower
        - catalogue
        - edit_additional_contents
4- Log in to the staff interface with the non-superlibrarian staff user
5- Go to Tools -> HTML customizations
    --> you can delete or edit the additional contents created by superlibrarian for All libraries
6- Click on New entry
    --> you can add additional content for other branches
7- Apply the patch
8- Repeat step 5
    --> you cannot delete or edit the additional contents create by superlibrarian for All libraries
9- Repeat step 6
    --> you cannot add additional content for other branches
10- submit the form
    --> the branch of the created additional content is the current user branch

Signed-off-by: Myka Kennedy Stephens <mkstephens@fosgail.org>
Comment 22 Myka Kennedy Stephens 2024-03-09 01:12:35 UTC
This patch works beautifully! I completed the test plan and everything worked as expected this time, very smooth. With the patch applied, the non-superlibrarian staff user can preview HTML customizations for all libraries and for other branches -- the edit and delete buttons are suppressed. This user can also edit existing content for their branch. When creating new content, the drop-down to select all libraries or a particular branch is suppressed. The user can create new content successfully and when returned to the table of all HTML customizations, the new content appears under the user's branch and has the edit and delete buttons available.

Great work! Glad all the back-and-forth finally paid off and it sounds like we both learned a little in the process. :-)
Comment 23 Marcel de Rooy 2024-09-20 09:41:34 UTC
Signed-off-by: John Doe <you@example.com>
Comment 24 Marcel de Rooy 2024-09-20 09:48:26 UTC
-    my $branchcode = $cgi->param('branchcode') || undef;
+    my $branchcode = $my_branch ? $my_branch : $cgi->param('branchcode') || undef;

This looks odd at least. I am posting on branchcode and we will write to current branch?
Comment 25 Marcel de Rooy 2024-09-20 10:03:22 UTC
+                            [% UNLESS ( my_branch && my_branch != c.branchcode ) %]
                                 <input type="checkbox" name="ids" value="[% c.id | html %]" />
+                            [% END %]

Hiding an id based on an unclear condition (my_branch does not represent what we intuitively think that it is). 
Note that I found one counterargument:
tools/letter.pl:our $my_branch = C4::Context->preference("IndependentBranches") && !$staffflags->{'superlibrarian'}
Comment 26 Marcel de Rooy 2024-09-20 10:56:23 UTC
This still needs a bit more attention at both server side and client side.
Comment 27 Sukhmandeep 2024-09-30 16:25:02 UTC
Created attachment 172231 [details] [review]
Bug 34631: (Follow up) Changed variable name

Same test plan as before just changed variable name for clarity.
Comment 28 Sukhmandeep 2024-09-30 16:29:41 UTC
The logic for the branchcode has changed slightly, and it seems to be the right approach. You can access the staff interface with two roles: superlibrarian or staff. Previously, there were no checks, allowing unrestricted editing and deleting.

The main issue was with variable names, which weren’t intuitive. To clarify, the branchcode is linked to the library field (Tools -> HTML customization -> new entry). Now, superlibrarians have access to all libraries, while staff can only access their own.

The variable my_branch indicates whether the user is a staff, adjusting the interface accordingly. I've renamed it for clarity, but that appears to be the only necessary change at this point.

Regarding the hidden ID of the checkbox, it works similarly: if you're staff and your library is different, there’s no checkbox, as it allows deletion of selected items.
Comment 29 David Nind 2024-12-30 02:02:01 UTC Comment hidden (obsolete)
Comment 30 David Nind 2024-12-30 02:02:05 UTC Comment hidden (obsolete)
Comment 31 David Nind 2024-12-30 02:20:03 UTC
Created attachment 175975 [details] [review]
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches

When syspref Independentbranches is enabled Non-superuser should not be able to write Additional contents for other branches

To reproduce:
1- Connect to staff interface as superlibrarian
2- Enable IndependentBranches preference
3- Go to Tools -> HTML customizations
4- Crete an additional content for all libraries
5- Create or edit a borrower
    5-1- Set permission for the borrower
        - catalogue
        - edit_additional_contents
4- Log in to the staff interface with the non-superlibrarian staff user
5- Go to Tools -> HTML customizations
    --> you can delete or edit the additional contents created by superlibrarian for All libraries
6- Click on New entry
    --> you can add additional content for other branches
7- Apply the patch
8- Repeat step 5
    --> you cannot delete or edit the additional contents create by superlibrarian for All libraries
9- Repeat step 6
    --> you cannot add additional content for other branches
10- submit the form
    --> the branch of the created additional content is the current user branch

Signed-off-by: Myka Kennedy Stephens <mkstephens@fosgail.org>
Signed-off-by: David Nind <david@davidnind.com>
Comment 32 David Nind 2024-12-30 02:20:06 UTC
Created attachment 175976 [details] [review]
Bug 34631: (Follow up) Changed variable name

Same test plan as before just changed variable name for clarity.

Signed-off-by: David Nind <david@davidnind.com>
Comment 33 David Nind 2024-12-30 02:22:09 UTC
I fixed the original commit message sign off signature (changed John Doe to Myka Kennedy Stephens).
Comment 34 Marcel de Rooy 2025-02-07 07:19:14 UTC
This feels like a workaround to me. Harder to maintain code. There should be another way to do this better. I wont pass QA but leave room for another QA point of view. No coding guideline to FQA either.
Comment 35 David Nind 2025-02-07 20:47:44 UTC
Some comments from another IndependentBranches related bug that may be relevant for this bug - bug 38040 comment #13:

"...
The trend has been to move parts of Independent Branches to more granular system preferences ( IndependentBranchesPatronModifications ,  IndependentBranchesTransfers ) so maybe we need 'IndependentBranchesReserves'?
...."
Comment 36 Katrin Fischer 2025-02-09 16:10:28 UTC
(In reply to David Nind from comment #35)
> Some comments from another IndependentBranches related bug that may be
> relevant for this bug - bug 38040 comment #13:
> 
> "...
> The trend has been to move parts of Independent Branches to more granular
> system preferences ( IndependentBranchesPatronModifications , 
> IndependentBranchesTransfers ) so maybe we need
> 'IndependentBranchesReserves'?
> ...."

And there are also library groups that already does a lot of what independentbranches did, but in a better and more flexible way. It would be nice to be able to get rid of the preference at some point.
Comment 37 Nick Clemens (kidclamp) 2025-02-18 12:43:34 UTC
Agreeing with others here - we need a new pref or another way of determining access - this would be a large change of behavior.

Additionally, I can still edit additional contents through URL manipulation:
http://localhost:8081/cgi-bin/koha/tools/additional-contents.pl?op=add_form&id=7&editmode=wysiwyg

It doesn't matter if the id in question belongs to my branch, it will after the edit, but I can still affect them

Adding the delete button in manually as well:

<input type="checkbox" name="ids" value="2">


Allows me to delete any entry
Comment 38 Hammat wele 2025-03-28 18:39:03 UTC
Created attachment 179859 [details] [review]
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches

When syspref Independentbranches is enabled Non-superuser should not be able to write Additional contents for other branches

To reproduce:
1- Connect to staff interface as superlibrarian
2- Enable IndependentBranches preference
3- Go to Tools -> HTML customizations
4- Crete an additional content for all libraries
5- Create or edit a borrower
    5-1- Set permission for the borrower
        - catalogue
        - edit_additional_contents
4- Log in to the staff interface with the non-superlibrarian staff user
5- Go to Tools -> HTML customizations
    --> you can delete or edit the additional contents created by superlibrarian for All libraries
6- Click on New entry
    --> you can add additional content for other branches
7- Apply the patch
8- Repeat step 5
    --> you cannot delete or edit the additional contents create by superlibrarian for All libraries
9- Repeat step 6
    --> you cannot add additional content for other branches
10- submit the form
    --> the branch of the created additional content is the current user branch

Signed-off-by: Myka Kennedy Stephens <mkstephens@fosgail.org>
Signed-off-by: David Nind <david@davidnind.com>
Comment 39 Hammat wele 2025-03-28 18:39:09 UTC
Created attachment 179860 [details] [review]
Bug 34631: (Follow up) Changed variable name

Same test plan as before just changed variable name for clarity.

Signed-off-by: David Nind <david@davidnind.com>
Comment 40 Hammat wele 2025-04-03 18:15:58 UTC
Created attachment 180602 [details] [review]
Bug 34631: Independentbranches - Non-superuser should not be able to write Additional contents for other branches

When syspref Independentbranches is enabled Non-superuser should not be able to write Additional contents for other branches

To reproduce:
1- Connect to staff interface as superlibrarian
2- Enable IndependentBranches preference
3- Go to Tools -> HTML customizations
4- Crete an additional content for all libraries
5- Create or edit a borrower
    5-1- Set permission for the borrower
        - catalogue
        - edit_additional_contents
4- Log in to the staff interface with the non-superlibrarian staff user
5- Go to Tools -> HTML customizations
    --> you can delete or edit the additional contents created by superlibrarian for All libraries
6- Click on New entry
    --> you can add additional content for other branches
7- Apply the patch
8- Repeat step 5
    --> you cannot delete or edit the additional contents create by superlibrarian for All libraries
9- Repeat step 6
    --> you cannot add additional content for other branches
10- submit the form
    --> the branch of the created additional content is the current user branch

Signed-off-by: Myka Kennedy Stephens <mkstephens@fosgail.org>
Signed-off-by: David Nind <david@davidnind.com>
Comment 41 Hammat wele 2025-04-03 18:16:04 UTC
Created attachment 180603 [details] [review]
Bug 34631: (Follow up) Changed variable name

Same test plan as before just changed variable name for clarity.

Signed-off-by: David Nind <david@davidnind.com>
Comment 42 Hammat wele 2025-04-03 18:16:09 UTC
Created attachment 180604 [details] [review]
Bug 34631: Add IndependentBranchesAdditionalcontents preference and some checks into additional-contents.pl

To reproduce:
1- Connect to staff interface as superlibrarian
2- Enable IndependentBranches preference
3- Go to Tools -> HTML customizations
4- Crete an additional content for all libraries
5- Create or edit a borrower
    5-1- Set permission for the borrower
        - catalogue
        - edit_additional_contents
4- Log in to the staff interface with the non-superlibrarian staff user
5- Go to Tools -> HTML customizations
    --> you can delete or edit the additional contents created by superlibrarian for All libraries
6- Click on New entry
    --> you can add additional content for other branches
7- Apply the patch and run updatedatabase
8- Enable IndependentBranchesAdditionalcontents preference
9- Repeat step 5
    --> you cannot delete or edit the additional contents create by superlibrarian for All libraries
    even by url manipulation
10- Repeat step 6
    --> you cannot add additional content for other branches
11- submit the form
    --> the branch of the created additional content is the current user branch
Comment 43 Magnus Enger 2025-04-04 17:00:22 UTC
Not sure I understand the state of this bug. There are two signed off patches and one that is not signed off. The one that is not signed off has a very similar test plan to the main patch that is signed off. 

Should the two signed off patches be obsoleted?
Comment 44 Hammat wele 2025-04-04 17:59:28 UTC
(In reply to Magnus Enger from comment #43)
> Not sure I understand the state of this bug. There are two signed off
> patches and one that is not signed off. The one that is not signed off has a
> very similar test plan to the main patch that is signed off. 
> 
> Should the two signed off patches be obsoleted?

Hi Magnus, the one that is not signed off add corrections that is suggested in comment 37, that need to be signed off. I attached a test plan because there is a new preference IndependentBranchesAdditionalcontents, the two first patches should not be obsoleted
Comment 45 David Nind 2025-04-27 17:35:34 UTC
This only seems to partially work, if I have tested correctly.

I Failed QA because of 4.4 and 4.5 - I can't edit existing items for my specific library, but I can delete it.

1. I only tested with news.
2. Using the superlibrarian (koha), I created two news items:
   2.1 One for all libraries
   2.2 One for Midway only
3. I changed the permissions for Henry Acevedo.
4. With the patch applied, IndependentBranchesAdditionalcontents = Yes, and logged in as Henry:
   4.1 I can't edit or delete the news item for all libraries (as expected)
   4.2 I have options to edit and delete the news item for Midway (as expected)
   4.3 I can delete the news item for Midway (as expected)
   4.4 When I go to edit the news item for Midway, the edit form is not pre-populated with the news item details - the form title is "New news item":
       . For the Library field, the "All libraries" option is selected
       . There is no publication date
       . There are no news item details (title and content)
   4.5 If I create a news item:
       . For the library field, I can choose any library - when I save, it changes the library to Midway
       . When I edit the news item, the same as 4.4
       . If I fill in the fields and save, a new news item is created
  
Suggestion for the system preference description:
- Current: Prevent staff (but not superlibrarians) from editing Additional contents in other libraries: 
- Suggested: Prevent staff (but not superlibrarians) from editing additional contents for other libraries: