Bug 39739 - OPAC's datatables.js out of sync with staff's
Summary: OPAC's datatables.js out of sync with staff's
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks: 39600
  Show dependency treegraph
 
Reported: 2025-04-24 12:56 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2025-05-16 15:26 UTC (History)
5 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
RM NOTE: REVERTED
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 39739: Sync OPAC's datatables.js with the staff interface's version (50.22 KB, patch)
2025-04-24 13:22 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 39739: Sync OPAC's datatables.js with the staff interface's version (50.28 KB, patch)
2025-04-25 11:34 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2025-04-24 12:56:27 UTC
When helping adapt bug 38340 to our coding guidelines and general style, I found the need to rewrite a manually crafted datatable using kohaTable. What I found is that key pieces (like x-koha-request-id handling) are missing.

I propose we port the intranet version into the OPAC. With some caveats.
Comment 1 Tomás Cohen Arazi (tcohen) 2025-04-24 13:22:26 UTC
Created attachment 181444 [details] [review]
Bug 39739: Sync OPAC's datatables.js with the staff interface's version

This patch overwrites the OPAC version of `datatables,js` with the one
for the staff interface.

A block code adding links to the columns configuration page is removed
as it doesn't apply to the OPAC and produces errors in the console for
missing variable definitions that would be inherited from the caller
context (looking for `CAN_user_parameters_manage_column_config`).

To test:
1. Apply this patch
2. Find templates using `kohaTable`:
   $ cd koha-tmpl/opac-tmpl
   $ git grep kohaTable
3. Play in the UI with those pages
=> SUCCESS: Tables work, nothing's broken
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Martin Renvoize (ashimema) 2025-04-25 11:34:20 UTC
Created attachment 181506 [details] [review]
Bug 39739: Sync OPAC's datatables.js with the staff interface's version

This patch overwrites the OPAC version of `datatables,js` with the one
for the staff interface.

A block code adding links to the columns configuration page is removed
as it doesn't apply to the OPAC and produces errors in the console for
missing variable definitions that would be inherited from the caller
context (looking for `CAN_user_parameters_manage_column_config`).

To test:
1. Apply this patch
2. Find templates using `kohaTable`:
   $ cd koha-tmpl/opac-tmpl
   $ git grep kohaTable
3. Play in the UI with those pages
=> SUCCESS: Tables work, nothing's broken
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 3 Martin Renvoize (ashimema) 2025-04-25 11:35:46 UTC
Simple resync.. agree with the singular removed function that's a staff only requirement and I've tested various tables in the OPAC.

Happy it's all working.. QA script reports some new console.logs but I don't think there's anything to worry about here.

Passing QA

(I do think there's some tidying up we can do down the line to both the staff and opac versions of this.. feels like we're duplicating some code when I looked)
Comment 4 Katrin Fischer 2025-04-25 16:36:30 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 5 Jonathan Druart 2025-04-28 08:36:22 UTC
This does not make sense. The tables and features *are* different on the two interfaces, and it has always been like that.

This should be revert IMO.
Comment 6 Katrin Fischer 2025-04-28 08:41:22 UTC
Some of it was needed for bug 39600 I believe. I know we don't offer some of the features in the OPAC like the actual column settings, but we do have export and some others. Do we have negative side effects if we keep it? Having them in synch might be easier to maintain, even if some things are not used.
Comment 7 Jonathan Druart 2025-04-28 08:42:31 UTC
x-koha-request-id didn't exist in columns_settings.inc or datatables.js prior to bug 26553.
Comment 8 Katrin Fischer 2025-04-28 08:43:21 UTC
(In reply to Jonathan Druart from comment #7)
> x-koha-request-id didn't exist in columns_settings.inc or datatables.js
> prior to bug 26553.

Sorry, can you explain a bit more?
Comment 9 Jonathan Druart 2025-04-28 08:45:51 UTC
(In reply to Katrin Fischer from comment #6)
> Some of it was needed for bug 39600 I believe. I know we don't offer some of
> the features in the OPAC like the actual column settings, but we do have
> export and some others. Do we have negative side effects if we keep it?
> Having them in synch might be easier to maintain, even if some things are
> not used.

Yes, it's bad and wrong to have code that is not used: state, shareable link, colvis, etc.

Always the UI is broken (see the export button)

The dom was different on purpose, as well as other attributes.
Comment 10 Jonathan Druart 2025-04-28 08:47:50 UTC
(In reply to Katrin Fischer from comment #8)
> (In reply to Jonathan Druart from comment #7)
> > x-koha-request-id didn't exist in columns_settings.inc or datatables.js
> > prior to bug 26553.
> 
> Sorry, can you explain a bit more?

In comment 0, Tomas want to have x-koha-request-id back at the OPAC. But I didn't find any traces of this before 26553. So my guess is that it never worked, and we should only add the code dealing with this header.
Comment 11 Katrin Fischer 2025-04-28 08:48:13 UTC
I think it will conflict if I revert with the other bigger feature. Waiting for Tomas to see how we can move forward here.
Comment 12 Katrin Fischer 2025-04-28 09:31:36 UTC
(In reply to Katrin Fischer from comment #11)
> I think it will conflict if I revert with the other bigger feature. Waiting
> for Tomas to see how we can move forward here.

The patch reverts cleanly from main. I am not pushing this yet, waiting for feedback from Tomas.
Comment 13 Tomás Cohen Arazi (tcohen) 2025-04-28 11:35:58 UTC
(In reply to Katrin Fischer from comment #12)
> (In reply to Katrin Fischer from comment #11)
> > I think it will conflict if I revert with the other bigger feature. Waiting
> > for Tomas to see how we can move forward here.
> 
> The patch reverts cleanly from main. I am not pushing this yet, waiting for
> feedback from Tomas.

For reverting it, you will need to also revert ad4a9777a2306ab8bf6f85e94aff2507695b5122 and also need a follow-up I can prepare.

If Joubu insists I can go that route.
Comment 14 Tomás Cohen Arazi (tcohen) 2025-04-28 11:56:57 UTC
(In reply to Jonathan Druart from comment #10)
> (In reply to Katrin Fischer from comment #8)
> > (In reply to Jonathan Druart from comment #7)
> > > x-koha-request-id didn't exist in columns_settings.inc or datatables.js
> > > prior to bug 26553.
> > 
> > Sorry, can you explain a bit more?
> 
> In comment 0, Tomas want to have x-koha-request-id back at the OPAC. But I
> didn't find any traces of this before 26553. So my guess is that it never
> worked, and we should only add the code dealing with this header.

There's never been an API driven datatable in the OPAC it seems.
Comment 15 Katrin Fischer 2025-04-28 12:02:23 UTC
We are taking a step back here, to move forward again. 
Reverted this patch, which will break bug 39600 for the moment, but hope we can fix that quickly.
Comment 16 Katrin Fischer 2025-04-28 12:07:25 UTC
... not reverted yet (sorry for the back and forth)
Comment 17 Tomás Cohen Arazi (tcohen) 2025-04-28 12:51:24 UTC
I've submitted a patch for 39600 that would *only* introduce server side rendering in the OPAC's datatables, and could pair with reverting this one.
Comment 18 Tomás Cohen Arazi (tcohen) 2025-04-28 13:11:25 UTC
This should be reverted.
Comment 19 Katrin Fischer 2025-04-28 14:29:51 UTC
(In reply to Tomás Cohen Arazi (tcohen) from comment #18)
> This should be reverted.
Comment 20 Katrin Fischer 2025-04-28 14:31:02 UTC
Reverted.