Summary: | OPAC's datatables.js out of sync with staff's | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Component: | Templates | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Status: | Pushed to main --- | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
Severity: | normal | ||
Priority: | P5 - low | CC: | jonathan.druart, katrin.fischer, martin.renvoize, oleonard, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
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: | |
Bug Depends on: | |||
Bug Blocks: | 39600 | ||
Attachments: |
Bug 39739: Sync OPAC's datatables.js with the staff interface's version
Bug 39739: Sync OPAC's datatables.js with the staff interface's version |
Description
Tomás Cohen Arazi (tcohen)
2025-04-24 12:56:27 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> 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> 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) Pushed for 25.05! Well done everyone, thank you! 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. 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. x-koha-request-id didn't exist in columns_settings.inc or datatables.js prior to bug 26553. (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 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. (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. I think it will conflict if I revert with the other bigger feature. Waiting for Tomas to see how we can move forward here. (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. (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. (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. 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. ... not reverted yet (sorry for the back and forth) 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. This should be reverted. (In reply to Tomás Cohen Arazi (tcohen) from comment #18) > This should be reverted. Reverted. |