Bug 30070 - Performance issues with edifactmsgs when you have a large number of messages
Summary: Performance issues with edifactmsgs when you have a large number of messages
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-02-10 13:23 UTC by Martin Renvoize
Modified: 2024-02-27 22:18 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement converts the EDIFACT messages display table to an API driven asynchronous datatable greatly improving the performance of that page when large numbers of messages exist.
Version(s) released in:
24.05.00


Attachments
Bug 30070: Add Koha Objects for EDI Message Files (3.27 KB, patch)
2023-11-10 15:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add fetch endpoint for EDI Files (8.09 KB, patch)
2023-11-10 15:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Update edifactmsgs to use the API (7.68 KB, patch)
2023-11-10 15:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Return unless the related object exists (1.01 KB, patch)
2023-11-13 16:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30070: Add Koha Objects for EDI Message Files (3.27 KB, patch)
2023-11-14 09:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add fetch endpoint for EDI Files (8.26 KB, patch)
2023-11-14 09:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Update edifactmsgs to use the API (7.68 KB, patch)
2023-11-14 09:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Return unless the related object exists (1.01 KB, patch)
2023-11-14 09:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add Koha Objects for EDI Message Files (3.34 KB, patch)
2023-11-14 09:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add fetch endpoint for EDI Files (8.26 KB, patch)
2023-11-14 09:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Update edifactmsgs to use the API (7.68 KB, patch)
2023-11-14 09:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Return unless the related object exists (1.45 KB, patch)
2023-11-14 09:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add Koha Objects for EDI Message Files (3.34 KB, patch)
2023-11-14 10:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add fetch endpoint for EDI Files (8.43 KB, patch)
2023-11-14 10:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Update edifactmsgs to use the API (7.68 KB, patch)
2023-11-14 10:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Return unless the related object exists (1.45 KB, patch)
2023-11-14 10:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add unit tests (6.68 KB, patch)
2023-11-14 10:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add koha_obejct_class(s) to EdifactMessage (761 bytes, patch)
2023-11-14 11:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Fix javascript errors (3.38 KB, patch)
2023-11-14 13:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30070: Add Koha Objects for EDI Message Files (3.34 KB, patch)
2023-11-14 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add fetch endpoint for EDI Files (8.43 KB, patch)
2023-11-14 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Update edifactmsgs to use the API (7.68 KB, patch)
2023-11-14 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Return unless the related object exists (1.45 KB, patch)
2023-11-14 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add unit tests (6.68 KB, patch)
2023-11-14 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add koha_obejct_class(s) to EdifactMessage (761 bytes, patch)
2023-11-14 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Fix javascript errors (3.38 KB, patch)
2023-11-14 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Further javascript fixes (2.93 KB, patch)
2023-11-14 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Filter out deleted files on the API (2.13 KB, patch)
2023-11-14 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Improve view message modal display (1.49 KB, patch)
2023-11-14 15:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Remove 'Configure' option from datatable (1.13 KB, patch)
2023-11-14 15:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add Koha Objects for EDI Message Files (3.34 KB, patch)
2023-11-14 15:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add fetch endpoint for EDI Files (8.43 KB, patch)
2023-11-14 15:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Update edifactmsgs to use the API (7.68 KB, patch)
2023-11-14 15:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Return unless the related object exists (1.45 KB, patch)
2023-11-14 15:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add unit tests (6.68 KB, patch)
2023-11-14 15:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add koha_obejct_class(s) to EdifactMessage (761 bytes, patch)
2023-11-14 15:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Fix javascript errors (3.38 KB, patch)
2023-11-14 15:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Further javascript fixes (2.93 KB, patch)
2023-11-14 15:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Filter out deleted files on the API (3.92 KB, patch)
2023-11-14 15:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Improve view message modal display (1.49 KB, patch)
2023-11-14 15:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Remove 'Configure' option from datatable (1.13 KB, patch)
2023-11-14 15:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add Koha Objects for EDI Message Files (3.40 KB, patch)
2023-11-14 17:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30070: Add fetch endpoint for EDI Files (8.48 KB, patch)
2023-11-14 17:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30070: Update edifactmsgs to use the API (7.73 KB, patch)
2023-11-14 17:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30070: (follow-up) Return unless the related object exists (1.51 KB, patch)
2023-11-14 17:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30070: Add unit tests (6.73 KB, patch)
2023-11-14 17:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30070: Add koha_obejct_class(s) to EdifactMessage (816 bytes, patch)
2023-11-14 17:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30070: (follow-up) Fix javascript errors (3.44 KB, patch)
2023-11-14 17:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30070: (follow-up) Further javascript fixes (2.99 KB, patch)
2023-11-14 17:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30070: (follow-up) Filter out deleted files on the API (3.97 KB, patch)
2023-11-14 17:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30070: Improve view message modal display (1.55 KB, patch)
2023-11-14 17:02 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30070: Remove 'Configure' option from datatable (1.19 KB, patch)
2023-11-14 17:02 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30070: (QA follow-up) Make qa script happy (1.38 KB, patch)
2023-11-14 17:02 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30070: Add Koha Objects for EDI Message Files (3.45 KB, patch)
2023-11-15 09:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add fetch endpoint for EDI Files (8.53 KB, patch)
2023-11-15 09:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Update edifactmsgs to use the API (7.79 KB, patch)
2023-11-15 09:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Return unless the related object exists (1.56 KB, patch)
2023-11-15 09:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add unit tests (6.79 KB, patch)
2023-11-15 09:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add koha_obejct_class(s) to EdifactMessage (870 bytes, patch)
2023-11-15 09:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Fix javascript errors (3.49 KB, patch)
2023-11-15 09:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Further javascript fixes (3.04 KB, patch)
2023-11-15 09:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Filter out deleted files on the API (4.03 KB, patch)
2023-11-15 09:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Improve view message modal display (1.60 KB, patch)
2023-11-15 09:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Remove 'Configure' option from datatable (1.24 KB, patch)
2023-11-15 09:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (QA follow-up) Make qa script happy (1.43 KB, patch)
2023-11-15 09:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Ensure strings are translatable (5.52 KB, patch)
2023-11-15 09:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add Koha Objects for EDI Message Files (3.34 KB, patch)
2023-11-16 09:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add fetch endpoint for EDI Files (8.43 KB, patch)
2023-11-16 09:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Update edifactmsgs to use the API (7.68 KB, patch)
2023-11-16 09:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Return unless the related object exists (1.45 KB, patch)
2023-11-16 09:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add unit tests (6.68 KB, patch)
2023-11-16 09:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add koha_obejct_class(s) to EdifactMessage (761 bytes, patch)
2023-11-16 09:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Fix javascript errors (3.38 KB, patch)
2023-11-16 09:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Further javascript fixes (2.93 KB, patch)
2023-11-16 09:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Filter out deleted files on the API (3.92 KB, patch)
2023-11-16 09:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Improve view message modal display (1.49 KB, patch)
2023-11-16 09:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Remove 'Configure' option from datatable (1.13 KB, patch)
2023-11-16 09:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add Koha Objects for EDI Message Files (3.34 KB, patch)
2023-12-28 10:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add fetch endpoint for EDI Files (8.43 KB, patch)
2023-12-28 10:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Update edifactmsgs to use the API (7.68 KB, patch)
2023-12-28 10:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Return unless the related object exists (1.45 KB, patch)
2023-12-28 10:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add unit tests (6.68 KB, patch)
2023-12-28 10:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Add koha_obejct_class(s) to EdifactMessage (761 bytes, patch)
2023-12-28 10:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Fix javascript errors (3.38 KB, patch)
2023-12-28 10:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Further javascript fixes (2.93 KB, patch)
2023-12-28 10:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Filter out deleted files on the API (3.92 KB, patch)
2023-12-28 10:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Improve view message modal display (1.49 KB, patch)
2023-12-28 10:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: Remove 'Configure' option from datatable (1.13 KB, patch)
2023-12-28 10:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (QA follow-up) Make qa script happy (1.43 KB, patch)
2023-12-28 10:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30070: (follow-up) Ensure strings are translatable (5.52 KB, patch)
2023-12-28 10:46 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2022-02-10 13:23:02 UTC
The edifact messages table can grow to a very large size over time, and the page to display them has issues with this growth as it always loads all messages.

We should switch this to an ajax driven datatable.
Comment 1 Martin Renvoize 2023-11-10 15:36:48 UTC
Created attachment 158808 [details] [review]
Bug 30070: Add Koha Objects for EDI Message Files

This patch adds the Koha Object based classes assocaited with the
edifact_message table. This table actually contains the raw message
files which may well each contain multiple edifact messages.
Comment 2 Martin Renvoize 2023-11-10 15:36:51 UTC
Created attachment 158809 [details] [review]
Bug 30070: Add fetch endpoint for EDI Files

This patch adds a simple EDI Files listing REST endpoint
Comment 3 Martin Renvoize 2023-11-10 15:36:53 UTC
Created attachment 158810 [details] [review]
Bug 30070: Update edifactmsgs to use the API
Comment 4 Martin Renvoize 2023-11-10 15:39:06 UTC
Adding Tomas and Kyle to CC here.

* I'd like your opinions on the locations of the API endpoint and mappings Tomas.
* Kyle added as a bit of an EDI guru in the hopes you might test it for me as most people shy away from EDI for testing.
Comment 5 Ray Delahunty 2023-11-13 10:17:11 UTC
Our test server has 10,400 lines in the table and it is taking about 40 seconds to display the page. I don't know how we could set up a sandbox with lots of messages, so perhaps we could see how the change works on our test server. (Our higher specification production server has 11,100 lines but it still takes about 35 seconds to draw the page).
Comment 6 Kyle M Hall 2023-11-13 15:57:39 UTC
I'm testing this is some production data and I'm getting 500 errors. I think it's because Koha::Edifact::File presumes every file will have a vendor and basket, but that's not necessarily true. I've patched those two methods but then I get the error:

omething went wrong when loading the table.
500: Internal Server Error. 
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.
Properties not allowed: basket, deleted, vendor.

Can you try testing with an edi message that has a null basket number?
Comment 7 Kyle M Hall 2023-11-13 16:00:25 UTC
Created attachment 158887 [details] [review]
Bug 30070: (follow-up) Return unless the related object exists
Comment 8 Martin Renvoize 2023-11-13 19:02:59 UTC
Thanks for testing Kyle, good call on the relation change. I'll continue here later tonight, be great to see this little enhancement in.

Ray, I'll work through the issues Kyle highlighted and then look at it getting it into your test system.
Comment 9 Martin Renvoize 2023-11-13 19:05:59 UTC
Ha, I just missed the final addition of the relations in the spec file. I'm out at the minute, but just adding vendor, deleted and basket keys to the edi_file spec should get this going.
Comment 10 Martin Renvoize 2023-11-14 09:19:17 UTC
Created attachment 158903 [details] [review]
Bug 30070: Add Koha Objects for EDI Message Files

This patch adds the Koha Object based classes assocaited with the
edifact_message table. This table actually contains the raw message
files which may well each contain multiple edifact messages.
Comment 11 Martin Renvoize 2023-11-14 09:19:20 UTC
Created attachment 158904 [details] [review]
Bug 30070: Add fetch endpoint for EDI Files

This patch adds a simple EDI Files listing REST endpoint
Comment 12 Martin Renvoize 2023-11-14 09:19:22 UTC
Created attachment 158905 [details] [review]
Bug 30070: Update edifactmsgs to use the API
Comment 13 Martin Renvoize 2023-11-14 09:19:25 UTC
Created attachment 158906 [details] [review]
Bug 30070: (follow-up) Return unless the related object exists
Comment 14 Martin Renvoize 2023-11-14 09:25:06 UTC
Created attachment 158907 [details] [review]
Bug 30070: Add Koha Objects for EDI Message Files

This patch adds the Koha Object based classes assocaited with the
edifact_message table. This table actually contains the raw message
files which may well each contain multiple edifact messages.
Comment 15 Martin Renvoize 2023-11-14 09:25:08 UTC
Created attachment 158908 [details] [review]
Bug 30070: Add fetch endpoint for EDI Files

This patch adds a simple EDI Files listing REST endpoint
Comment 16 Martin Renvoize 2023-11-14 09:25:11 UTC
Created attachment 158909 [details] [review]
Bug 30070: Update edifactmsgs to use the API
Comment 17 Martin Renvoize 2023-11-14 09:25:14 UTC
Created attachment 158910 [details] [review]
Bug 30070: (follow-up) Return unless the related object exists
Comment 18 Martin Renvoize 2023-11-14 10:34:30 UTC
Created attachment 158911 [details] [review]
Bug 30070: Add Koha Objects for EDI Message Files

This patch adds the Koha Object based classes assocaited with the
edifact_message table. This table actually contains the raw message
files which may well each contain multiple edifact messages.
Comment 19 Martin Renvoize 2023-11-14 10:34:33 UTC
Created attachment 158912 [details] [review]
Bug 30070: Add fetch endpoint for EDI Files

This patch adds a simple EDI Files listing REST endpoint
Comment 20 Martin Renvoize 2023-11-14 10:34:36 UTC
Created attachment 158913 [details] [review]
Bug 30070: Update edifactmsgs to use the API
Comment 21 Martin Renvoize 2023-11-14 10:34:39 UTC
Created attachment 158914 [details] [review]
Bug 30070: (follow-up) Return unless the related object exists
Comment 22 Martin Renvoize 2023-11-14 10:34:42 UTC
Created attachment 158915 [details] [review]
Bug 30070: Add unit tests
Comment 23 Martin Renvoize 2023-11-14 11:42:40 UTC
Created attachment 158918 [details] [review]
Bug 30070: Add koha_obejct_class(s) to EdifactMessage
Comment 24 Kyle M Hall 2023-11-14 13:57:03 UTC
Created attachment 158920 [details] [review]
Bug 30070: (follow-up) Fix javascript errors
Comment 25 Kyle M Hall 2023-11-14 14:01:14 UTC
I fixed some more js issues but there are still a few outstanding issues:

* The table is showing all edifact messages including deleted messages. I image some kind of search paramter needs to be added in Koha::REST::V1::Acquisitions::Edifact::Files::list

* I get the following error on some but not all datatable pages: DataTables warning: table id=edi_msgs - Requested unknown parameter '4' for row 12, column 4. For more information about this error, please see http://datatables.net/tn/4 datatables_23.0600079.js:514:13

* The bookseller id is missing from the vendor links in the "Vendor" column
Comment 26 Martin Renvoize 2023-11-14 15:23:05 UTC
Created attachment 158922 [details] [review]
Bug 30070: Add Koha Objects for EDI Message Files

This patch adds the Koha Object based classes assocaited with the
edifact_message table. This table actually contains the raw message
files which may well each contain multiple edifact messages.
Comment 27 Martin Renvoize 2023-11-14 15:23:08 UTC
Created attachment 158923 [details] [review]
Bug 30070: Add fetch endpoint for EDI Files

This patch adds a simple EDI Files listing REST endpoint
Comment 28 Martin Renvoize 2023-11-14 15:23:12 UTC
Created attachment 158924 [details] [review]
Bug 30070: Update edifactmsgs to use the API
Comment 29 Martin Renvoize 2023-11-14 15:23:15 UTC
Created attachment 158925 [details] [review]
Bug 30070: (follow-up) Return unless the related object exists
Comment 30 Martin Renvoize 2023-11-14 15:23:18 UTC
Created attachment 158926 [details] [review]
Bug 30070: Add unit tests
Comment 31 Martin Renvoize 2023-11-14 15:23:21 UTC
Created attachment 158927 [details] [review]
Bug 30070: Add koha_obejct_class(s) to EdifactMessage
Comment 32 Martin Renvoize 2023-11-14 15:23:24 UTC
Created attachment 158928 [details] [review]
Bug 30070: (follow-up) Fix javascript errors
Comment 33 Martin Renvoize 2023-11-14 15:23:27 UTC
Created attachment 158929 [details] [review]
Bug 30070: (follow-up) Further javascript fixes

This fixes an issue with the datatables when a message file is found to
be missing the linked vendor or basket details.  This removes the
datatables error: DataTables warning: table id=edi_msgs - Requested
unknown parameter '4' for row 12, column 4.
Comment 34 Martin Renvoize 2023-11-14 15:23:30 UTC
Created attachment 158930 [details] [review]
Bug 30070: (follow-up) Filter out deleted files on the API

EDI uses a 'deleted' flag in the table to denote deleted files, we
should filter those out of the API response.
Comment 35 Martin Renvoize 2023-11-14 15:23:33 UTC
Created attachment 158931 [details] [review]
Bug 30070: Improve view message modal display

The display had some old specific styling added, we can just use
modal-lg from the bootstrap library now to make the display more
consistent with other modals.
Comment 36 Martin Renvoize 2023-11-14 15:30:46 UTC
Created attachment 158932 [details] [review]
Bug 30070: Remove 'Configure' option from datatable

This passes 0 for table_settings effectively removing the 'Configure'
option from the table. We haven't set up the background logic for table
settings for this table and I don't believe there's a real use case for
that here at the moment.
Comment 37 Martin Renvoize 2023-11-14 15:43:02 UTC
Created attachment 158933 [details] [review]
Bug 30070: Add Koha Objects for EDI Message Files

This patch adds the Koha Object based classes assocaited with the
edifact_message table. This table actually contains the raw message
files which may well each contain multiple edifact messages.
Comment 38 Martin Renvoize 2023-11-14 15:43:05 UTC
Created attachment 158934 [details] [review]
Bug 30070: Add fetch endpoint for EDI Files

This patch adds a simple EDI Files listing REST endpoint
Comment 39 Martin Renvoize 2023-11-14 15:43:08 UTC
Created attachment 158935 [details] [review]
Bug 30070: Update edifactmsgs to use the API
Comment 40 Martin Renvoize 2023-11-14 15:43:11 UTC
Created attachment 158936 [details] [review]
Bug 30070: (follow-up) Return unless the related object exists
Comment 41 Martin Renvoize 2023-11-14 15:43:14 UTC
Created attachment 158937 [details] [review]
Bug 30070: Add unit tests
Comment 42 Martin Renvoize 2023-11-14 15:43:17 UTC
Created attachment 158938 [details] [review]
Bug 30070: Add koha_obejct_class(s) to EdifactMessage
Comment 43 Martin Renvoize 2023-11-14 15:43:20 UTC
Created attachment 158939 [details] [review]
Bug 30070: (follow-up) Fix javascript errors
Comment 44 Martin Renvoize 2023-11-14 15:43:23 UTC
Created attachment 158940 [details] [review]
Bug 30070: (follow-up) Further javascript fixes

This fixes an issue with the datatables when a message file is found to
be missing the linked vendor or basket details.  This removes the
datatables error: DataTables warning: table id=edi_msgs - Requested
unknown parameter '4' for row 12, column 4.
Comment 45 Martin Renvoize 2023-11-14 15:43:27 UTC
Created attachment 158941 [details] [review]
Bug 30070: (follow-up) Filter out deleted files on the API

EDI uses a 'deleted' flag in the table to denote deleted files, we
should filter those out of the API response.
Comment 46 Martin Renvoize 2023-11-14 15:43:30 UTC
Created attachment 158942 [details] [review]
Bug 30070: Improve view message modal display

The display had some old specific styling added, we can just use
modal-lg from the bootstrap library now to make the display more
consistent with other modals.
Comment 47 Martin Renvoize 2023-11-14 15:43:33 UTC
Created attachment 158943 [details] [review]
Bug 30070: Remove 'Configure' option from datatable

This passes 0 for table_settings effectively removing the 'Configure'
option from the table. We haven't set up the background logic for table
settings for this table and I don't believe there's a real use case for
that here at the moment.
Comment 48 Kyle M Hall 2023-11-14 17:01:34 UTC
Created attachment 158944 [details] [review]
Bug 30070: Add Koha Objects for EDI Message Files

This patch adds the Koha Object based classes assocaited with the
edifact_message table. This table actually contains the raw message
files which may well each contain multiple edifact messages.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 49 Kyle M Hall 2023-11-14 17:01:41 UTC
Created attachment 158945 [details] [review]
Bug 30070: Add fetch endpoint for EDI Files

This patch adds a simple EDI Files listing REST endpoint

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 50 Kyle M Hall 2023-11-14 17:01:44 UTC
Created attachment 158946 [details] [review]
Bug 30070: Update edifactmsgs to use the API

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 51 Kyle M Hall 2023-11-14 17:01:46 UTC
Created attachment 158947 [details] [review]
Bug 30070: (follow-up) Return unless the related object exists

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 52 Kyle M Hall 2023-11-14 17:01:49 UTC
Created attachment 158948 [details] [review]
Bug 30070: Add unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 53 Kyle M Hall 2023-11-14 17:01:51 UTC
Created attachment 158949 [details] [review]
Bug 30070: Add koha_obejct_class(s) to EdifactMessage

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 54 Kyle M Hall 2023-11-14 17:01:54 UTC
Created attachment 158950 [details] [review]
Bug 30070: (follow-up) Fix javascript errors

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 55 Kyle M Hall 2023-11-14 17:01:56 UTC
Created attachment 158951 [details] [review]
Bug 30070: (follow-up) Further javascript fixes

This fixes an issue with the datatables when a message file is found to
be missing the linked vendor or basket details.  This removes the
datatables error: DataTables warning: table id=edi_msgs - Requested
unknown parameter '4' for row 12, column 4.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 56 Kyle M Hall 2023-11-14 17:01:59 UTC
Created attachment 158952 [details] [review]
Bug 30070: (follow-up) Filter out deleted files on the API

EDI uses a 'deleted' flag in the table to denote deleted files, we
should filter those out of the API response.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 57 Kyle M Hall 2023-11-14 17:02:02 UTC
Created attachment 158953 [details] [review]
Bug 30070: Improve view message modal display

The display had some old specific styling added, we can just use
modal-lg from the bootstrap library now to make the display more
consistent with other modals.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 58 Kyle M Hall 2023-11-14 17:02:04 UTC
Created attachment 158954 [details] [review]
Bug 30070: Remove 'Configure' option from datatable

This passes 0 for table_settings effectively removing the 'Configure'
option from the table. We haven't set up the background logic for table
settings for this table and I don't believe there's a real use case for
that here at the moment.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 59 Kyle M Hall 2023-11-14 17:02:07 UTC
Created attachment 158955 [details] [review]
Bug 30070: (QA follow-up) Make qa script happy

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 60 Martin Renvoize 2023-11-15 08:05:49 UTC
This is now up and running on your test server Ray. Let me know if you spot any issues or if you're happy I can add your signoff here and we can upgrade Kyles to a PQA stamp and get this pushed upstream :)
Comment 61 Ray Delahunty 2023-11-15 09:12:47 UTC
Had my stopwatch ready to see how much of a reduction from 35-40 seconds I would see, but the EDIFACT messages page appeared almost instantaneously. I can't see any problems- delete action works and navigation to the message and back to other pages is just fine.
Comment 62 Martin Renvoize 2023-11-15 09:16:33 UTC
Excellent, thanks for testing Ray :)

I'll go ahead and add your signoff line for you.
Comment 63 Martin Renvoize 2023-11-15 09:20:22 UTC
Created attachment 158963 [details] [review]
Bug 30070: Add Koha Objects for EDI Message Files

This patch adds the Koha Object based classes assocaited with the
edifact_message table. This table actually contains the raw message
files which may well each contain multiple edifact messages.

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 64 Martin Renvoize 2023-11-15 09:20:25 UTC
Created attachment 158964 [details] [review]
Bug 30070: Add fetch endpoint for EDI Files

This patch adds a simple EDI Files listing REST endpoint

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 65 Martin Renvoize 2023-11-15 09:20:28 UTC
Created attachment 158965 [details] [review]
Bug 30070: Update edifactmsgs to use the API

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 66 Martin Renvoize 2023-11-15 09:20:31 UTC
Created attachment 158966 [details] [review]
Bug 30070: (follow-up) Return unless the related object exists

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 67 Martin Renvoize 2023-11-15 09:20:34 UTC
Created attachment 158967 [details] [review]
Bug 30070: Add unit tests

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 68 Martin Renvoize 2023-11-15 09:20:37 UTC
Created attachment 158968 [details] [review]
Bug 30070: Add koha_obejct_class(s) to EdifactMessage

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 69 Martin Renvoize 2023-11-15 09:20:40 UTC
Created attachment 158969 [details] [review]
Bug 30070: (follow-up) Fix javascript errors

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 70 Martin Renvoize 2023-11-15 09:20:43 UTC
Created attachment 158970 [details] [review]
Bug 30070: (follow-up) Further javascript fixes

This fixes an issue with the datatables when a message file is found to
be missing the linked vendor or basket details.  This removes the
datatables error: DataTables warning: table id=edi_msgs - Requested
unknown parameter '4' for row 12, column 4.

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 71 Martin Renvoize 2023-11-15 09:20:46 UTC
Created attachment 158971 [details] [review]
Bug 30070: (follow-up) Filter out deleted files on the API

EDI uses a 'deleted' flag in the table to denote deleted files, we
should filter those out of the API response.

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 72 Martin Renvoize 2023-11-15 09:20:49 UTC
Created attachment 158972 [details] [review]
Bug 30070: Improve view message modal display

The display had some old specific styling added, we can just use
modal-lg from the bootstrap library now to make the display more
consistent with other modals.

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 73 Martin Renvoize 2023-11-15 09:20:52 UTC
Created attachment 158973 [details] [review]
Bug 30070: Remove 'Configure' option from datatable

This passes 0 for table_settings effectively removing the 'Configure'
option from the table. We haven't set up the background logic for table
settings for this table and I don't believe there's a real use case for
that here at the moment.

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 74 Martin Renvoize 2023-11-15 09:20:55 UTC
Created attachment 158974 [details] [review]
Bug 30070: (QA follow-up) Make qa script happy

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 75 Martin Renvoize 2023-11-15 09:28:43 UTC
Created attachment 158975 [details] [review]
Bug 30070: (follow-up) Ensure strings are translatable

Some strings moved from inline in templates to embedded in JS so we need
to wrap them in _() now.

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 76 Ray Delahunty 2023-11-16 07:59:26 UTC
My apologies, but one thing I should have noticed in testing was the unexpected order of results delivered. Prior to the patch I believe the sort was on column 2 (Transferred), with newest at the top, but now it appears to be on column 1 (Type). Certainly the ordering for us goes INVOICE > ORDERS  > ORDRSP > QUOTE. Within those I don't know what further ordering is based on. Default sorting based on Transferred (newest at the top) would be the preferred option I think- it certainly is mine.
Comment 77 Martin Renvoize 2023-11-16 09:43:07 UTC
Created attachment 159026 [details] [review]
Bug 30070: Add Koha Objects for EDI Message Files

This patch adds the Koha Object based classes assocaited with the
edifact_message table. This table actually contains the raw message
files which may well each contain multiple edifact messages.
Comment 78 Martin Renvoize 2023-11-16 09:43:11 UTC
Created attachment 159027 [details] [review]
Bug 30070: Add fetch endpoint for EDI Files

This patch adds a simple EDI Files listing REST endpoint
Comment 79 Martin Renvoize 2023-11-16 09:43:15 UTC
Created attachment 159028 [details] [review]
Bug 30070: Update edifactmsgs to use the API
Comment 80 Martin Renvoize 2023-11-16 09:43:18 UTC
Created attachment 159029 [details] [review]
Bug 30070: (follow-up) Return unless the related object exists
Comment 81 Martin Renvoize 2023-11-16 09:43:22 UTC
Created attachment 159030 [details] [review]
Bug 30070: Add unit tests
Comment 82 Martin Renvoize 2023-11-16 09:43:25 UTC
Created attachment 159031 [details] [review]
Bug 30070: Add koha_obejct_class(s) to EdifactMessage
Comment 83 Martin Renvoize 2023-11-16 09:43:29 UTC
Created attachment 159032 [details] [review]
Bug 30070: (follow-up) Fix javascript errors
Comment 84 Martin Renvoize 2023-11-16 09:43:32 UTC
Created attachment 159033 [details] [review]
Bug 30070: (follow-up) Further javascript fixes

This fixes an issue with the datatables when a message file is found to
be missing the linked vendor or basket details.  This removes the
datatables error: DataTables warning: table id=edi_msgs - Requested
unknown parameter '4' for row 12, column 4.
Comment 85 Martin Renvoize 2023-11-16 09:43:35 UTC
Created attachment 159034 [details] [review]
Bug 30070: (follow-up) Filter out deleted files on the API

EDI uses a 'deleted' flag in the table to denote deleted files, we
should filter those out of the API response.
Comment 86 Martin Renvoize 2023-11-16 09:43:38 UTC
Created attachment 159035 [details] [review]
Bug 30070: Improve view message modal display

The display had some old specific styling added, we can just use
modal-lg from the bootstrap library now to make the display more
consistent with other modals.
Comment 87 Martin Renvoize 2023-11-16 09:43:41 UTC
Created attachment 159036 [details] [review]
Bug 30070: Remove 'Configure' option from datatable

This passes 0 for table_settings effectively removing the 'Configure'
option from the table. We haven't set up the background logic for table
settings for this table and I don't believe there's a real use case for
that here at the moment.
Comment 88 Katrin Fischer 2023-12-28 10:31:38 UTC
Sorry, this no longer applies for me, but could it be we are missing patches? It starts right with a QA follow-up. Can you please check and fix?

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 30070: (QA follow-up) Make qa script happy
error: sha1 information is lacking or useless (Koha/REST/V1/Acquisitions/Vendor/Edifact/Files.pm).
error: could not build fake ancestor
Patch failed at 0001 Bug 30070: (QA follow-up) Make qa script happy
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-30070-QA-follow-up-Make-qa-script-happy-1cvqbapq.patch
Comment 89 Martin Renvoize 2023-12-28 10:45:55 UTC
Created attachment 160336 [details] [review]
Bug 30070: Add Koha Objects for EDI Message Files

This patch adds the Koha Object based classes assocaited with the
edifact_message table. This table actually contains the raw message
files which may well each contain multiple edifact messages.
Comment 90 Martin Renvoize 2023-12-28 10:45:59 UTC
Created attachment 160337 [details] [review]
Bug 30070: Add fetch endpoint for EDI Files

This patch adds a simple EDI Files listing REST endpoint
Comment 91 Martin Renvoize 2023-12-28 10:46:03 UTC
Created attachment 160338 [details] [review]
Bug 30070: Update edifactmsgs to use the API
Comment 92 Martin Renvoize 2023-12-28 10:46:07 UTC
Created attachment 160339 [details] [review]
Bug 30070: (follow-up) Return unless the related object exists
Comment 93 Martin Renvoize 2023-12-28 10:46:10 UTC
Created attachment 160340 [details] [review]
Bug 30070: Add unit tests
Comment 94 Martin Renvoize 2023-12-28 10:46:13 UTC
Created attachment 160341 [details] [review]
Bug 30070: Add koha_obejct_class(s) to EdifactMessage
Comment 95 Martin Renvoize 2023-12-28 10:46:16 UTC
Created attachment 160342 [details] [review]
Bug 30070: (follow-up) Fix javascript errors
Comment 96 Martin Renvoize 2023-12-28 10:46:20 UTC
Created attachment 160343 [details] [review]
Bug 30070: (follow-up) Further javascript fixes

This fixes an issue with the datatables when a message file is found to
be missing the linked vendor or basket details.  This removes the
datatables error: DataTables warning: table id=edi_msgs - Requested
unknown parameter '4' for row 12, column 4.
Comment 97 Martin Renvoize 2023-12-28 10:46:23 UTC
Created attachment 160344 [details] [review]
Bug 30070: (follow-up) Filter out deleted files on the API

EDI uses a 'deleted' flag in the table to denote deleted files, we
should filter those out of the API response.
Comment 98 Martin Renvoize 2023-12-28 10:46:26 UTC
Created attachment 160345 [details] [review]
Bug 30070: Improve view message modal display

The display had some old specific styling added, we can just use
modal-lg from the bootstrap library now to make the display more
consistent with other modals.
Comment 99 Martin Renvoize 2023-12-28 10:46:29 UTC
Created attachment 160346 [details] [review]
Bug 30070: Remove 'Configure' option from datatable

This passes 0 for table_settings effectively removing the 'Configure'
option from the table. We haven't set up the background logic for table
settings for this table and I don't believe there's a real use case for
that here at the moment.
Comment 100 Martin Renvoize 2023-12-28 10:46:32 UTC
Created attachment 160347 [details] [review]
Bug 30070: (QA follow-up) Make qa script happy

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 101 Martin Renvoize 2023-12-28 10:46:35 UTC
Created attachment 160348 [details] [review]
Bug 30070: (follow-up) Ensure strings are translatable

Some strings moved from inline in templates to embedded in JS so we need
to wrap them in _() now.

Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 102 Katrin Fischer 2024-01-16 14:45:51 UTC
*ping* Still waiting for a rebase.
Comment 103 Kyle M Hall 2024-01-16 15:16:52 UTC
(In reply to Katrin Fischer from comment #102)
> *ping* Still waiting for a rebase.

The patch set is applying to current master cleanly for me! Are you getting conflicts?
Comment 104 Katrin Fischer 2024-01-16 15:42:02 UTC
It applies, not sure what went wrong here, testing now.
Comment 105 Nick Clemens (kidclamp) 2024-01-17 19:55:46 UTC
This appears to be pushed to master?
Comment 106 Katrin Fischer 2024-01-17 22:40:02 UTC
I think I messed the bugzilla step or typoed "yes" again - I'll check in the morning.
Comment 107 Fridolin Somers 2024-01-31 12:26:03 UTC
Can this be backported to 23.11.x ?
Comment 108 Martin Renvoize 2024-01-31 13:44:18 UTC
We have it in our local 23.11 branch without causing any issues.. it's a performance enhancement, but I'd be happy to see it.
Comment 109 Ray Delahunty 2024-01-31 14:00:37 UTC
We have had this running on 22.11.11 since November (see my Comment 76). Some further fixes (QA-related?) appear to have been added since mid-November 2023 and I am not sure if those have been applied to us on 21.11.11 but the backporting at 11/2023 worked fine then and other than the sorting issue I missed in testing (!), the performance improvement is major. We have 12,000 lines and the data apppears instantaneously rather than taking (I recall) 2 minutes or so.
Comment 110 Martin Renvoize 2024-01-31 14:17:09 UTC
Further fixes were backported to your 21.11 Ray ;)

For the rest of the PTFS customers, it's only ported back to 22.11.. In both cases, the backport was fairly trivial but for community I think I'd only recommend porting one version back here.
Comment 111 Ray Delahunty 2024-01-31 14:24:48 UTC
Thanks Martin- nice to know we have had the additional patches applied to our system. But annoyingly I made a typo in my last comment. When I said "I am not sure if those have been applied to us on 21.11.11 [...] I should have said "I am not sure if those have been applied to us on 22.11.11 [...]". We have been on 22.11 since July 2023. Apologies.
Comment 112 Katrin Fischer 2024-01-31 21:46:37 UTC
Pushed for 24.05!

Well done everyone, thank you!