Bug 40087

Summary: Remove unused C4::Scrubber profiles "tag" and "staff"
Product: Koha Reporter: David Cook <dcook>
Component: Architecture, internals, and plumbingAssignee: David Cook <dcook>
Status: Pushed to stable --- QA Contact: Marcel de Rooy <m.de.rooy>
Severity: minor    
Priority: P5 - low CC: david, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39860
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
25.11.00,25.05.01
Circulation function:
Bug Depends on:    
Bug Blocks: 40089    
Attachments: Bug 40087: Remove "tag" and "staff" scrubber profiles
Bug 40087: Fix t/Scrubber.t unit test
Bug 40087: Remove "tag" and "staff" scrubber profiles
Bug 40087: Fix t/Scrubber.t unit test
Bug 40087: Remove "tag" and "staff" scrubber profiles
Bug 40087: Fix t/Scrubber.t unit test

Description David Cook 2025-06-06 01:22:05 UTC
As far as I can tell, the C4::Scrubber profiles "staff" and "tag" are not actually used in Koha. 

"tag" is actually the same as "default", and opac-tags.pl just uses "default". There's no point to "tag". 

Now "staff" is actually dangerous, because it doesn't do any scrubbing. It allows all elements and all attributes. We never want to use it.
Comment 1 David Cook 2025-06-06 01:26:26 UTC
Created attachment 183021 [details] [review]
Bug 40087: Remove "tag" and "staff" scrubber profiles

This patch removes unused "tag" and "staff" scrubber profiles.

Test plan:
0. Apply the patch
1. prove t/Koha/Plugins/HtmlScrubber.t
2. prove t/Scrubber.t
3. Grep around the code for C4::Scrubber and make sure "tag"
and "staff" aren't used as arguments for C4::Scrubber
Comment 2 David Nind 2025-06-06 03:02:32 UTC
prove t/Scrubber.t fails for me (both tests pass before the patch):

t/Scrubber.t .. 1/32 New called with unrecognized type 'tag' at t/Scrubber.t line 60.
# Looks like your test exited with 255 just after 18.
t/Scrubber.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 14/32 subtests 

Test Summary Report
-------------------
t/Scrubber.t (Wstat: 65280 (exited 255) Tests: 18 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 32 tests but ran 18.
Files=1, Tests=18,  1 wallclock secs ( 0.01 usr  0.01 sys +  0.22 cusr  0.05 csys =  0.29 CPU)
Result: FAIL

Not sure what I'm looking for, for step 3 of the test plan.
Comment 3 David Cook 2025-06-06 03:20:28 UTC
(In reply to David Nind from comment #2)
> prove t/Scrubber.t fails for me (both tests pass before the patch):
> 
> t/Scrubber.t .. 1/32 New called with unrecognized type 'tag' at t/Scrubber.t
> line 60.
> # Looks like your test exited with 255 just after 18.
> t/Scrubber.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
> Failed 14/32 subtests 
> 
> Test Summary Report
> -------------------
> t/Scrubber.t (Wstat: 65280 (exited 255) Tests: 18 Failed: 0)
>   Non-zero exit status: 255
>   Parse errors: Bad plan.  You planned 32 tests but ran 18.
> Files=1, Tests=18,  1 wallclock secs ( 0.01 usr  0.01 sys +  0.22 cusr  0.05
> csys =  0.29 CPU)
> Result: FAIL
> 
> Not sure what I'm looking for, for step 3 of the test plan.

Thanks, David. 

I actually ran it with "perl t/Scrubber.t" originally and it looks like I missed the failure because of the lack of color-coding haha.

I'll get that fixed.
Comment 4 David Cook 2025-06-06 03:23:23 UTC
Created attachment 183032 [details] [review]
Bug 40087: Fix t/Scrubber.t unit test

Remove "tag" and "staff" and add the missing "note"
Comment 5 David Cook 2025-06-06 03:23:34 UTC
Round 2...
Comment 6 David Nind 2025-06-06 03:26:32 UTC
Created attachment 183033 [details] [review]
Bug 40087: Remove "tag" and "staff" scrubber profiles

This patch removes unused "tag" and "staff" scrubber profiles.

Test plan:
0. Apply the patch
1. prove t/Koha/Plugins/HtmlScrubber.t
2. prove t/Scrubber.t
3. Grep around the code for C4::Scrubber and make sure "tag"
and "staff" aren't used as arguments for C4::Scrubber

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2025-06-06 03:26:35 UTC
Created attachment 183034 [details] [review]
Bug 40087: Fix t/Scrubber.t unit test

Remove "tag" and "staff" and add the missing "note"

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2025-06-06 03:28:42 UTC
(In reply to David Cook from comment #3)

> I'll get that fixed.

Thanks David, signed off.

For step 3, not sure exactly what to look for. With a git grep "C4::Scrubber", I can't see any mention of tag and staff.
Comment 9 Marcel de Rooy 2025-06-06 06:35:34 UTC
Created attachment 183036 [details] [review]
Bug 40087: Remove "tag" and "staff" scrubber profiles

This patch removes unused "tag" and "staff" scrubber profiles.

Test plan:
0. Apply the patch
1. prove t/Koha/Plugins/HtmlScrubber.t
2. prove t/Scrubber.t
3. Grep around the code for C4::Scrubber and make sure "tag"
and "staff" aren't used as arguments for C4::Scrubber

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2025-06-06 06:35:37 UTC
Created attachment 183037 [details] [review]
Bug 40087: Fix t/Scrubber.t unit test

Remove "tag" and "staff" and add the missing "note"

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Lucas Gass (lukeg) 2025-06-06 15:47:00 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 12 Paul Derscheid 2025-06-22 21:47:34 UTC
Nice work everyone!

Pushed to 25.05.x for 25.05.03