Bug 40659 - Allow "My virtual card" format and content to be customizable
Summary: Allow "My virtual card" format and content to be customizable
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Lucas Gass (lukeg)
QA Contact: Testopia
URL:
Keywords:
Depends on: 39860
Blocks: 41086
  Show dependency treegraph
 
Reported: 2025-08-15 12:55 UTC by Andrew Fuerste-Henry
Modified: 2025-11-17 15:59 UTC (History)
4 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 40659: Add new notice VIRTUALCARD (3.16 KB, patch)
2025-10-09 16:10 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 40659: Add ability to customize OPACVirtualCard (5.66 KB, patch)
2025-10-09 16:12 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 40659: Add ability to customize OPACVirtualCard (7.01 KB, patch)
2025-10-09 16:28 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 40659: Close two divs in sample notice (1.14 KB, patch)
2025-10-09 20:42 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 40659: Add new notice VIRTUALCARD (3.21 KB, patch)
2025-10-14 19:42 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 40659: Add ability to customize OPACVirtualCard (7.07 KB, patch)
2025-10-14 19:42 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 40659: Close two divs in sample notice (1.19 KB, patch)
2025-10-14 19:42 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 40659: Add Scrubber profile and scrub the contents of VIRTUALCARD template (2.06 KB, patch)
2025-10-23 13:19 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 40659: Unit tests for new Scrubber profile (5.84 KB, patch)
2025-10-23 13:48 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 40659: Unit tests for new Scrubber profile (5.83 KB, patch)
2025-10-23 13:49 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 40659: Change Scrubber profile name to opac_virtual_card (1.96 KB, patch)
2025-11-17 15:59 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2025-08-15 12:55:54 UTC
Libraries would like more flexibility in the virtual card shown to patrons via the OPAC. Making that virtual card use a notice template would make is highly customizable.
Comment 1 Lucas Gass (lukeg) 2025-10-09 16:10:36 UTC
Created attachment 187695 [details] [review]
Bug 40659: Add new notice VIRTUALCARD
Comment 2 Lucas Gass (lukeg) 2025-10-09 16:12:45 UTC
Created attachment 187696 [details] [review]
Bug 40659: Add ability to customize OPACVirtualCard
Comment 3 Lucas Gass (lukeg) 2025-10-09 16:28:56 UTC
Created attachment 187697 [details] [review]
Bug 40659: Add ability to customize OPACVirtualCard

To test:

1. Turn on the OPACVirtualCard.
2. Log in to the OPAC as a patron and go look at your Library card ( opac-virtual-card.pl )
3. APPLY PATCH, updatedatabase, restart_all
4. Go again at your patron's card on the OPAC ( opac-virtual-card.pl ), it should look exactly the same.
5. Go to Tools > Notices > VIRTUALCARD
6. You should see the default content for the library card.
7. Play around with different variables from borrower, like [% borrower.firstname%] and branches, like [% branch.branchcode %]
8. You can also move around the barcode by using the placeholder value [% my_barcode %].
9. Put the [% my_barcode %] variable anywhere you want in the notice then go to the sys pref "OPACVirtualCardBarcode".
10. Change to different barcode types and relaod the library card page ( opac-virtual-card.pl ). You should notice the barcode type is being honored.
11. Turn on OPACpatronimages AND patronimages, then upload a patron image for your patron.
12. Use the [% my_image %] variable anywhere you want in the notice to place the patron image.
13. Leaving the [% my_image %] variable as is, turn off OPACpatronimages.
14. Reload the OPAC page, no image should display.
15. Delete the VIRTUALCARD notice.
16. Go back to the OPAC page and with the notice deleted you should see the hardcoded content. ie: the page should look exactly the same as it does without this patch.
Comment 4 Andrew Fuerste-Henry 2025-10-09 20:42:19 UTC
Created attachment 187706 [details] [review]
Bug 40659: Close two divs in sample notice
Comment 5 Andrew Fuerste-Henry 2025-10-14 19:42:11 UTC
Created attachment 187894 [details] [review]
Bug 40659: Add new notice VIRTUALCARD

Signed-off-by: Marie Martino <martinom43@morainevalley.edu>
Comment 6 Andrew Fuerste-Henry 2025-10-14 19:42:13 UTC
Created attachment 187895 [details] [review]
Bug 40659: Add ability to customize OPACVirtualCard

To test:

1. Turn on the OPACVirtualCard.
2. Log in to the OPAC as a patron and go look at your Library card ( opac-virtual-card.pl )
3. APPLY PATCH, updatedatabase, restart_all
4. Go again at your patron's card on the OPAC ( opac-virtual-card.pl ), it should look exactly the same.
5. Go to Tools > Notices > VIRTUALCARD
6. You should see the default content for the library card.
7. Play around with different variables from borrower, like [% borrower.firstname%] and branches, like [% branch.branchcode %]
8. You can also move around the barcode by using the placeholder value [% my_barcode %].
9. Put the [% my_barcode %] variable anywhere you want in the notice then go to the sys pref "OPACVirtualCardBarcode".
10. Change to different barcode types and relaod the library card page ( opac-virtual-card.pl ). You should notice the barcode type is being honored.
11. Turn on OPACpatronimages AND patronimages, then upload a patron image for your patron.
12. Use the [% my_image %] variable anywhere you want in the notice to place the patron image.
13. Leaving the [% my_image %] variable as is, turn off OPACpatronimages.
14. Reload the OPAC page, no image should display.
15. Delete the VIRTUALCARD notice.
16. Go back to the OPAC page and with the notice deleted you should see the hardcoded content. ie: the page should look exactly the same as it does without this patch.

Signed-off-by: Marie Martino <martinom43@morainevalley.edu>
Comment 7 Andrew Fuerste-Henry 2025-10-14 19:42:16 UTC
Created attachment 187896 [details] [review]
Bug 40659: Close two divs in sample notice

Signed-off-by: Marie Martino <martinom43@morainevalley.edu>
Comment 8 David Cook 2025-10-14 23:43:01 UTC
(In reply to Andrew Fuerste-Henry from comment #0)
> Libraries would like more flexibility in the virtual card shown to patrons
> via the OPAC. Making that virtual card use a notice template would make is
> highly customizable.

It would also introduce a security vulnerability. 

I was weighing up whether to use "Failed QA" or "In Discussion", but I figured I'd opt for the former.

I think an argument could be made that other Notes & Slips might already have vulnerabilities in them. I haven't reviewed them in depth. 

However, for new code, I think that we want to do better - in any case.

--

On a related note, I think "HTML Customizations" would make more sense than Notes & Slips for this type of change. That might then dovetail in work that Lucas has done for adding templating to show extra MARC fields on the detail page. I can't recall the bug number but I'm sure Lucas will know it. 

(Of course, with the "HTML Customizations" we're in a bit of a bind where you can't use the WYSIWYG, you have to use the text editor, but that would be the same thing as Notes & Slips anyway.)
Comment 9 David Cook 2025-10-15 03:31:33 UTC
(In reply to David Cook from comment #8)
> I can't recall the bug number but I'm sure Lucas will know it. 

It's bug 39860 which I've added as a See Also
Comment 10 Andrew Fuerste-Henry 2025-10-15 13:03:07 UTC
(In reply to David Cook from comment #8)
> It would also introduce a security vulnerability. 
> 
> I was weighing up whether to use "Failed QA" or "In Discussion", but I
> figured I'd opt for the former.
> 
> I think an argument could be made that other Notes & Slips might already
> have vulnerabilities in them. I haven't reviewed them in depth. 
> 
> However, for new code, I think that we want to do better - in any case.

Can you provide some further detail on what "do better" would mean to you in this situation? What is it about the approach on 39860 that meets your needs better than the work here?
Comment 11 David Cook 2025-10-15 22:39:28 UTC
(In reply to Andrew Fuerste-Henry from comment #10)
> (In reply to David Cook from comment #8)
> > However, for new code, I think that we want to do better - in any case.
> 
> Can you provide some further detail on what "do better" would mean to you in
> this situation? What is it about the approach on 39860 that meets your needs
> better than the work here?

Yeah for sure. I can DM you with more in-depth info, but here I'll say that bug 39860 uses the HTML::Scrubber to allow a safe range of HTML to be added. 

I'd add it's not about my needs, but rather the security of Koha as a whole. I'm just quite active in the security space. At some point, I hope to either add some additional coding guidelines or a separate "Safe Coding Guidelines" to complement the existing coding guidelines, which should then make things clearer. Thanks for the question!
Comment 12 Lucas Gass (lukeg) 2025-10-20 12:36:58 UTC
(In reply to David Cook from comment #11)
> (In reply to Andrew Fuerste-Henry from comment #10)
> > (In reply to David Cook from comment #8)
> > > However, for new code, I think that we want to do better - in any case.
> > 
> > Can you provide some further detail on what "do better" would mean to you in
> > this situation? What is it about the approach on 39860 that meets your needs
> > better than the work here?
> 
> Yeah for sure. I can DM you with more in-depth info, but here I'll say that
> bug 39860 uses the HTML::Scrubber to allow a safe range of HTML to be added. 
> 
> I'd add it's not about my needs, but rather the security of Koha as a whole.
> I'm just quite active in the security space. At some point, I hope to either
> add some additional coding guidelines or a separate "Safe Coding Guidelines"
> to complement the existing coding guidelines, which should then make things
> clearer. Thanks for the question!

Is there any reason those details cannot be shared here fro transparency? 

If scrubbing the HTML is the only issue then I think this should be left in Notices/Slips where it can be scrubbed. The reason is simple, notices/slips already has a pattern for using placeholder/substitutions which will be need in this bug. I advocate to leave this in notice/slips for that reason.
Comment 13 David Cook 2025-10-20 22:28:09 UTC
(In reply to Lucas Gass (lukeg) from comment #12)
> Is there any reason those details cannot be shared here fro transparency? 

Typically, we'd talk about it in the Koha Security project. I thought folks might like to keep this bug in Koha, but we could move it to Koha Security for a fuller discussion?
 
> If scrubbing the HTML is the only issue then I think this should be left in
> Notices/Slips where it can be scrubbed. The reason is simple, notices/slips
> already has a pattern for using placeholder/substitutions which will be need
> in this bug. I advocate to leave this in notice/slips for that reason.

That's a compelling point. It does have a better UI for that for sure. 

What about translatability? I was thinking HTML Customizations would be better as people could add in custom labels for each of their different language interfaces?
Comment 14 Lucas Gass (lukeg) 2025-10-20 23:02:03 UTC
> What about translatability? I was thinking HTML Customizations would be
> better as people could add in custom labels for each of their different
> language interfaces?

Notices are translatable. There is a TranslateNotices system preference to allow them to be.
Comment 15 David Cook 2025-10-20 23:15:17 UTC
(In reply to Lucas Gass (lukeg) from comment #14)
> > What about translatability? I was thinking HTML Customizations would be
> > better as people could add in custom labels for each of their different
> > language interfaces?
> 
> Notices are translatable. There is a TranslateNotices system preference to
> allow them to be.

Ahh right. That's good. 

Hmm, conceptually I think it still fits best in HTML Customizations, as it's an HTML Customization. But... I do take your point.

Maybe "Notices and slips" needs a re-brand, and to move out of the "Patrons and circulation" area of "Tools". 

After all, we have the OAI_HARVEST_REPORT and Report templates in "Notices and slips" as well. Huh, SFTP_FAILURE and SFTP_SUCCESS as well. That's interesting. 

Yeah, all right. I think "Notices and slips" is far enough gone now that it's fine to use it for this purpose.

--

So yeah I think if scrubbing is added then it should be all good.
Comment 16 Lucas Gass (lukeg) 2025-10-21 13:12:23 UTC
(In reply to David Cook from comment #15)
> (In reply to Lucas Gass (lukeg) from comment #14)
> > > What about translatability? I was thinking HTML Customizations would be
> > > better as people could add in custom labels for each of their different
> > > language interfaces?
> > 
> > Notices are translatable. There is a TranslateNotices system preference to
> > allow them to be.
> 
> Ahh right. That's good. 
> 
> Hmm, conceptually I think it still fits best in HTML Customizations, as it's
> an HTML Customization. But... I do take your point.
> 
> Maybe "Notices and slips" needs a re-brand, and to move out of the "Patrons
> and circulation" area of "Tools". 
> 
> After all, we have the OAI_HARVEST_REPORT and Report templates in "Notices
> and slips" as well. Huh, SFTP_FAILURE and SFTP_SUCCESS as well. That's
> interesting. 
> 
> Yeah, all right. I think "Notices and slips" is far enough gone now that
> it's fine to use it for this purpose.
> 
> --
> 
> So yeah I think if scrubbing is added then it should be all good.


I did find this already open bug:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36525
Comment 17 Lucas Gass (lukeg) 2025-10-22 13:28:08 UTC
After speaking with Martin I am convinced that this should go in HTML customization, not notice/slips. 

Going forward notice/slips should only be used for things that are considered "communications". All other customization belong in additional contents.
Comment 18 Lucas Gass (lukeg) 2025-10-22 21:54:33 UTC
(In reply to Lucas Gass (lukeg) from comment #17)
> After speaking with Martin I am convinced that this should go in HTML
> customization, not notice/slips. 
> 
> Going forward notice/slips should only be used for things that are
> considered "communications". All other customization belong in additional
> contents.

I changed my mind after reviewing Bug 27334. It is not close to ready for inclusion. It has pretty glaring security issues and doesn't deal with CodeMirrror vs TinyMCE at all. So this needs to stay in Notice/Slips until there is a viable alternative. At which point we'd already have a bunch of things to migrate out of notice/slips.
Comment 19 David Cook 2025-10-23 00:58:10 UTC
Yeah it's a really tough one. Some of the things I've found in bug 11583 also make bug 27334 even harder to implement than one might think.
Comment 20 David Cook 2025-10-23 01:03:31 UTC
(In reply to David Cook from comment #19)
> Yeah it's a really tough one. Some of the things I've found in bug 11583
> also make bug 27334 even harder to implement than one might think.

I mean... in our defence... there are whole apps like Wordpress which focus on this sort of thing and show it's easier said than done.

Actually even in the case of the block editor... I don't know if Wordpress can even do what we're trying to do here.
Comment 21 Lucas Gass (lukeg) 2025-10-23 13:19:57 UTC
Created attachment 188357 [details] [review]
Bug 40659: Add Scrubber profile and scrub the contents of VIRTUALCARD template
Comment 22 Lucas Gass (lukeg) 2025-10-23 13:24:06 UTC
I am making this dependent on Bug 39860 because the 2 scrubber profiles in C4/Scrubber might conflict.


The profile called 'opac' is extremely limited. I am wondering if we need some sort of hint/tip on the VIRTUALCARD template page telling the user what HTML are allowed. If I was working without knowledge of what tags are allowed it would be frustrating.
Comment 23 Lucas Gass (lukeg) 2025-10-23 13:48:26 UTC
Created attachment 188358 [details] [review]
Bug 40659: Unit tests for new Scrubber profile
Comment 24 Lucas Gass (lukeg) 2025-10-23 13:49:26 UTC
Created attachment 188359 [details] [review]
Bug 40659: Unit tests for new Scrubber profile
Comment 25 David Cook 2025-10-24 03:12:47 UTC
(In reply to Lucas Gass (lukeg) from comment #22)
> The profile called 'opac' is extremely limited. I am wondering if we need
> some sort of hint/tip on the VIRTUALCARD template page telling the user what
> HTML are allowed. If I was working without knowledge of what tags are
> allowed it would be frustrating.

That's a really good point.
Comment 26 Katrin Fischer 2025-10-25 11:10:13 UTC
Hm. Looking at this:

When we remove the default template in favor our a notice template, we cause a regression for updating libraries that are not English users. Especially when handling a lot of libraries with multiple languages that can mean a huge extra work on update. Also: Not all libraries use TranslateNotices and many have this turned off.

I feel it would be cleaner to keep the default template, that makes this available for any language installed without putting extra work on the library and have the customization as an optional extra that replaces the default.
Comment 27 Katrin Fischer 2025-10-25 11:11:20 UTC
> I feel it would be cleaner to keep the default template, that makes this
> available for any language installed without putting extra work on the
> library and have the customization as an optional extra that replaces the
> default.

We could think of other solutions, but I am a bit tired of the clean-up we have to do with each update, just because our default is not English and we do provide lots of translations for users in the OPAC.
Comment 28 Lucas Gass (lukeg) 2025-10-27 12:16:49 UTC
(In reply to Katrin Fischer from comment #26)
> Hm. Looking at this:
> 
> When we remove the default template in favor our a notice template, we cause
> a regression for updating libraries that are not English users. Especially
> when handling a lot of libraries with multiple languages that can mean a
> huge extra work on update. Also: Not all libraries use TranslateNotices and
> many have this turned off.
> 
> I feel it would be cleaner to keep the default template, that makes this
> available for any language installed without putting extra work on the
> library and have the customization as an optional extra that replaces the
> default.

So, I should remove the DB update and sample notice? And folks will have to add it themselves?
Comment 29 Katrin Fischer 2025-10-27 21:02:17 UTC
Right now, all you need to do is switch a system preference to make the feature work. If this is added non-English libraries will have to also add translations. This feels backwards for a template that should be provided translated out of the box. 

Could we handle it like OpacMaintenance? We have a default in the template, that can be overwritten if needed with your own text/styling.

Nothing would change for updating libraries, no disruptions, no extra work. But it would still be customizable if needed.

So yes, I suggest removing the default notices and add the standard template back.
Comment 30 David Cook 2025-10-27 22:47:18 UTC
I didn't quite understand the conversation for a little while there, but now I think that I agree with Katrin.

Keeping the status quo would be good. So the idea would be that this feature allows an opt-in way to customize the format? That sounds good to me.

It would probably be useful to provide a sample template somewhere though. Do we put one in the wiki and then link to it from the system preference description? Or maybe we could put a static file or modal somewhere that could be linked to from the system preference description?
Comment 31 Lucas Gass (lukeg) 2025-11-03 14:25:57 UTC
(In reply to David Cook from comment #30)
> I didn't quite understand the conversation for a little while there, but now
> I think that I agree with Katrin.
> 
> Keeping the status quo would be good. So the idea would be that this feature
> allows an opt-in way to customize the format? That sounds good to me.
> 
> It would probably be useful to provide a sample template somewhere though.
> Do we put one in the wiki and then link to it from the system preference
> description? Or maybe we could put a static file or modal somewhere that
> could be linked to from the system preference description?

Makes sense to me. I believe all we have to do is drop the first patch.
Comment 32 Lucas Gass (lukeg) 2025-11-03 14:27:16 UTC
I have obsoleted the first patch, so this should again be ready for testing.
Comment 33 David Cook 2025-11-06 04:32:46 UTC
Hmm could we get a rebase? Looks like "Close two divs in sample notice" is causing a problem.

--

$ git bz apply 40659

Bug 40659 - Allow "My virtual card" format and content to be customizable

187895 - Bug 40659: Add ability to customize OPACVirtualCard
187896 - Bug 40659: Close two divs in sample notice
188357 - Bug 40659: Add Scrubber profile and scrub the contents of VIRTUALCARD template
188359 - Bug 40659: Unit tests for new Scrubber profile

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 40659: Add ability to customize OPACVirtualCard
Applying: Bug 40659: Close two divs in sample notice
error: sha1 information is lacking or useless (installer/data/mysql/en/mandatory/sample_notices.yml).
error: could not build fake ancestor
Patch failed at 0001 Bug 40659: Close two divs in sample notice
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-40659-Close-two-divs-in-sample-notice-ko46zg6c.patch
Comment 34 Lucas Gass (lukeg) 2025-11-06 13:26:15 UTC
(In reply to David Cook from comment #33)
> Hmm could we get a rebase? Looks like "Close two divs in sample notice" is
> causing a problem.

Done!
Comment 35 David Cook 2025-11-07 03:09:42 UTC
It's mostly working but it looks like there might be a problem when using TranslateNotices. I can't get a custom English or Spanish translation to appear. The only thing that works is Default.

Also, there's nothing in Koha to tell you how to customize this. I think it would still be a good idea to have a DB update to insert an empty notice for Patrons > VIRTUALCARD. Then a reference to that in the OPACVirtualCard syspref would be a good idea I think.
Comment 36 David Cook 2025-11-07 03:13:46 UTC
Comment on attachment 188357 [details] [review]
Bug 40659: Add Scrubber profile and scrub the contents of VIRTUALCARD template

Review of attachment 188357 [details] [review]:
-----------------------------------------------------------------

::: C4/Scrubber.pm
@@ +88,4 @@
>              },
>          ],
>      },
> +    opac => {

Might be a good idea to call this something other than "opac" for the time being. Even "opac_virtual_card"  might be good for now.
Comment 37 Lucas Gass (lukeg) 2025-11-17 15:59:07 UTC
Created attachment 189633 [details] [review]
Bug 40659: Change Scrubber profile name to opac_virtual_card