[Bug 29275] New: issuehistory.pl is very slow when there are many historical issues
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Bug ID: 29275 Summary: issuehistory.pl is very slow when there are many historical issues Change sponsored?: --- Product: Koha Version: 21.05 Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Staff Client Assignee: koha-bugs@lists.koha-community.org Reporter: wizzyrea@gmail.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com Example: We have a library with a biblio that has 39000ish issues in the history, this page cannot be loaded within the plack/apache timeouts, and users can DOS themselves trying to see it. This is admittedly a very extreme case, but even bibs with fewer issues in the history would probably benefit from this page having better performance. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|21.05 |master Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Status|NEW |ASSIGNED CC| |tomascohen@gmail.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |29290 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29290 [Bug 29290] Add routes to fetch checkouts for a given biblio -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 126626 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126626&action=edit Bug 29275: Add $patron_to_html to render patron information in JS Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 126627 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126627&action=edit Bug 29275: Use the API to fetch checkouts information This patch changes issuehistory.pl so it leverages on the API for fetching checkouts information in the template. Workflow changes: This development splits the 'current' checkouts and the 'past' checkouts in two different tables. Terminology could be revisited by native english speakers. To test: 1. Have records with and without current/past checkouts 2. Notice how the Checkouts history tab presents the information on each case. => SUGGESTION: Keep each of them open on separate tabs for comparing 3. Apply this patchset and its dependencies 4. Restart plack, run: $ koha-plack --restart kohadev 5. Follow the same steps you followed for those open tabs, in new ones => SUCCESS: What you see makes sense, information is displayed as it used to, with the different tables now. 6. Try searching on the datatables => SUCCESS: Search works correctly! 7. Revisit the new texts I added => SUGGESTION: If you have better wordings, submit a patch, or just comment on the bug so I do it inline. 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 126628 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126628&action=edit Bug 29275: (follow-up) Remove use of CountSubscriptionFromBiblionumber Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff CC| |jonathan.druart+koha@gmail. | |com, | |kyle@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126626|0 |1 is obsolete| | --- Comment #4 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 126634 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126634&action=edit Bug 29275: Add $patron_to_html to render patron information in JS Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126627|0 |1 is obsolete| | --- Comment #5 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 126635 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126635&action=edit Bug 29275: Use the API to fetch checkouts information This patch changes issuehistory.pl so it leverages on the API for fetching checkouts information in the template. Workflow changes: This development splits the 'current' checkouts and the 'past' checkouts in two different tables. Terminology could be revisited by native english speakers. To test: 1. Have records with and without current/past checkouts 2. Notice how the Checkouts history tab presents the information on each case. => SUGGESTION: Keep each of them open on separate tabs for comparing 3. Apply this patchset and its dependencies 4. Restart plack, run: $ koha-plack --restart kohadev 5. Follow the same steps you followed for those open tabs, in new ones => SUCCESS: What you see makes sense, information is displayed as it used to, with the different tables now. 6. Try searching on the datatables => SUCCESS: Search works correctly! 7. Revisit the new texts I added => SUGGESTION: If you have better wordings, submit a patch, or just comment on the bug so I do it inline. 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126628|0 |1 is obsolete| | --- Comment #6 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 126636 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126636&action=edit Bug 29275: (follow-up) Remove use of CountSubscriptionFromBiblionumber Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #7 from Victor Grousset/tuxayo <victor@tuxayo.net> --- cases tested to get all the strings and behaviors I could thing of - never checked out and no current checkout - never checked out and a current checkout - a past checkout and no current checkout - more than 10 past checkout (for pagination) and a current checkout strings make sense -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #8 from Victor Grousset/tuxayo <victor@tuxayo.net> --- A diff between current_checkouts and old_checkouts functions show that they have a lot of duplication. Is there a way to deduplicate without making something messy? I wonder if there is a way to assign old_checkouts as a copy of current_checkouts and then re assign so it it's properties to add and change stuff. The only difference is the URL param and a render function. Also a call to encodeURIComponent() might be missing in current_checkouts -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |ASSIGNED --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Victor Grousset/tuxayo from comment #8)
A diff between current_checkouts and old_checkouts functions show that they have a lot of duplication. Is there a way to deduplicate without making something messy? I wonder if there is a way to assign old_checkouts as a copy of current_checkouts and then re assign so it it's properties to add and change stuff. The only difference is the URL param and a render function.
Also a call to encodeURIComponent() might be missing in current_checkouts
Thanks! Fixing! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Victor Grousset/tuxayo from comment #8)
A diff between current_checkouts and old_checkouts functions show that they have a lot of duplication. Is there a way to deduplicate without making something messy? I wonder if there is a way to assign old_checkouts as a copy of current_checkouts and then re assign so it it's properties to add and change stuff. The only difference is the URL param and a render function.
I tried, but things get a bit harder to understand and I didn't feel like the trade-off justified a generalization. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I didn't submit the follow-up because the RM mentioned bug 28859, which is expected to be pushed first. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Bug 29275 depends on bug 29290, which changed state. Bug 29290 Summary: Add routes to fetch checkouts for a given biblio https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29290 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #12 from Victor Grousset/tuxayo <victor@tuxayo.net> --- And it has been pushed :D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126634|0 |1 is obsolete| | Attachment #126635|0 |1 is obsolete| | Attachment #126636|0 |1 is obsolete| | --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 127701 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127701&action=edit Bug 29275: Add $patron_to_html to render patron information in JS Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 127702 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127702&action=edit Bug 29275: Use the API to fetch checkouts information This patch changes issuehistory.pl so it leverages on the API for fetching checkouts information in the template. Workflow changes: This development splits the 'current' checkouts and the 'past' checkouts in two different tables. Terminology could be revisited by native english speakers. To test: 1. Have records with and without current/past checkouts 2. Notice how the Checkouts history tab presents the information on each case. => SUGGESTION: Keep each of them open on separate tabs for comparing 3. Apply this patchset and its dependencies 4. Restart plack, run: $ koha-plack --restart kohadev 5. Follow the same steps you followed for those open tabs, in new ones => SUCCESS: What you see makes sense, information is displayed as it used to, with the different tables now. 6. Try searching on the datatables => SUCCESS: Search works correctly! 7. Revisit the new texts I added => SUGGESTION: If you have better wordings, submit a patch, or just comment on the bug so I do it inline. 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 127703 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127703&action=edit Bug 29275: (follow-up) Remove use of CountSubscriptionFromBiblionumber Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Victor Grousset/tuxayo from comment #8)
A diff between current_checkouts and old_checkouts functions show that they have a lot of duplication. Is there a way to deduplicate without making something messy? I wonder if there is a way to assign old_checkouts as a copy of current_checkouts and then re assign so it it's properties to add and change stuff. The only difference is the URL param and a render function.
Not just that. But I made a function to avoid duplication.
Also a call to encodeURIComponent() might be missing in current_checkouts
Solved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|issuehistory.pl is very |Use the API to render |slow when there are many |checkout history for a |historical issues |biblio -- You are receiving this mail because: You are watching all bug changes.
SELECT * FROM columns_settings WHERE tablename='checkoutshistory-table'; => SUCCESS: Koha currently works :-D
SELECT * FROM columns_settings WHERE tablename LIKE '%_checkouts-table'; => SUCCESS: old_checkouts-table and current_checkouts-table have the right setting
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 127704 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127704&action=edit Bug 29275: (follow-up) Split columns settings and handle upgrade This patch splits the table settings as we now have two tables. The atomic update takes care of setting the right values (if present) on the new tables entries. To test: 1. Do not apply this patch 2. Set some values you will remember for the checkoutshistory-table table 3. Run: $ koha-mysql kohadev 4. Apply this patch 5. Run: $ updatedatabase 6. Repeat 3 => SUCCESS: Things have been deleted 7. Run: $ koha-mysql kohadev 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Sorry dude.. I love this improvement.. but I think we have a problem. The new js equivalent to patron-name.inc is awesome and works great.. but it got me looking at what patron-name.inc does and threw me into the rabbit hole looking at how patrons get hidden from other branch staff in certain modes of operation. I can't see any handling, either in the js function or in the API response builder, that would filter out patrons that the logged-in user should not be able to see details for. I'm hopeful that I'm just missing something in the API layer as I think that's where it should sit personally.. we shouldn't expose the data at all if the user doesn't have permission to view it, rather than hide it at the view stage. If that functionality is there, any chance you could point me to the unit tests for it? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |29523 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29523 [Bug 29523] Add a way to prevent embedding objects that should not be allowed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127704|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127701|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127702|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127703|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #19 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 127894 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127894&action=edit Bug 29275: Add $patron_to_html to render patron information in JS Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #20 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 127895 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127895&action=edit Bug 29275: Use the API to fetch checkouts information This patch changes issuehistory.pl so it leverages on the API for fetching checkouts information in the template. Workflow changes: This development splits the 'current' checkouts and the 'past' checkouts in two different tables. Terminology could be revisited by native english speakers. To test: 1. Have records with and without current/past checkouts 2. Notice how the Checkouts history tab presents the information on each case. => SUGGESTION: Keep each of them open on separate tabs for comparing 3. Apply this patchset and its dependencies 4. Restart plack, run: $ koha-plack --restart kohadev 5. Follow the same steps you followed for those open tabs, in new ones => SUCCESS: What you see makes sense, information is displayed as it used to, with the different tables now. 6. Try searching on the datatables => SUCCESS: Search works correctly! 7. Revisit the new texts I added => SUGGESTION: If you have better wordings, submit a patch, or just comment on the bug so I do it inline. 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 127896 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127896&action=edit Bug 29275: (follow-up) Remove use of CountSubscriptionFromBiblionumber Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
SELECT * FROM columns_settings WHERE tablename='checkoutshistory-table'; => SUCCESS: Koha currently works :-D
SELECT * FROM columns_settings WHERE tablename LIKE '%_checkouts-table'; => SUCCESS: old_checkouts-table and current_checkouts-table have the right setting
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #22 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 127897 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127897&action=edit Bug 29275: (follow-up) Split columns settings and handle upgrade This patch splits the table settings as we now have two tables. The atomic update takes care of setting the right values (if present) on the new tables entries. To test: 1. Do not apply this patch 2. Set some values you will remember for the checkoutshistory-table table 3. Run: $ koha-mysql kohadev 4. Apply this patch 5. Run: $ updatedatabase 6. Repeat 3 => SUCCESS: Things have been deleted 7. Run: $ koha-mysql kohadev 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #23 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #18)
Sorry dude.. I love this improvement.. but I think we have a problem. The new js equivalent to patron-name.inc is awesome and works great.. but it got me looking at what patron-name.inc does and threw me into the rabbit hole looking at how patrons get hidden from other branch staff in certain modes of operation.
Good catch! And this is bigger than this dev!
I can't see any handling, either in the js function or in the API response builder, that would filter out patrons that the logged-in user should not be able to see details for. I'm hopeful that I'm just missing something in the API layer as I think that's where it should sit personally.. we shouldn't expose the data at all if the user doesn't have permission to view it, rather than hide it at the view stage. If that functionality is there, any chance you could point me to the unit tests for it?
I proposed a generic solution for checking 'allowed-to-embed' objects on bug 28523. It looks pretty solid to me and I made this bug dependent on it. For the specific case of a 'null' patron on the response (which is now the case, correctly handled in the controller level, not just hidden) I decided to handle it locally (so not in js-patron-format.inc) like this: if ( row.patron != null ) { return $patron_to_html( row.patron, { display_cardnumber: true, url: true } ); } else { return _("A patron from library %s").format(escape_str(row.library.name)); } it felt like there was no need for a generic message, as each scenario could require different wordings. Please test! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Victor Grousset/tuxayo from comment #12)
And it has been pushed :D
Time to test :D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |29575 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29575 [Bug 29575] Add a JS render function equivalent of the patron-title.inc TT include -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127895|0 |1 is obsolete| | --- Comment #25 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 128001 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128001&action=edit Bug 29275: Use the API to fetch checkouts information This patch changes issuehistory.pl so it leverages on the API for fetching checkouts information in the template. Workflow changes: This development splits the 'current' checkouts and the 'past' checkouts in two different tables. Terminology could be revisited by native english speakers. To test: 1. Have records with and without current/past checkouts 2. Notice how the Checkouts history tab presents the information on each case. => SUGGESTION: Keep each of them open on separate tabs for comparing 3. Apply this patchset and its dependencies 4. Restart plack, run: $ koha-plack --restart kohadev 5. Follow the same steps you followed for those open tabs, in new ones => SUCCESS: What you see makes sense, information is displayed as it used to, with the different tables now. 6. Try searching on the datatables => SUCCESS: Search works correctly! 7. Revisit the new texts I added => SUGGESTION: If you have better wordings, submit a patch, or just comment on the bug so I do it inline. 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127896|0 |1 is obsolete| | --- Comment #26 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 128002 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128002&action=edit Bug 29275: (follow-up) Remove use of CountSubscriptionFromBiblionumber Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
SELECT * FROM columns_settings WHERE tablename='checkoutshistory-table'; => SUCCESS: Koha currently works :-D
SELECT * FROM columns_settings WHERE tablename LIKE '%_checkouts-table'; => SUCCESS: old_checkouts-table and current_checkouts-table have the right setting
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127897|0 |1 is obsolete| | --- Comment #27 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 128003 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128003&action=edit Bug 29275: (follow-up) Split columns settings and handle upgrade This patch splits the table settings as we now have two tables. The atomic update takes care of setting the right values (if present) on the new tables entries. To test: 1. Do not apply this patch 2. Set some values you will remember for the checkoutshistory-table table 3. Run: $ koha-mysql kohadev 4. Apply this patch 5. Run: $ updatedatabase 6. Repeat 3 => SUCCESS: Things have been deleted 7. Run: $ koha-mysql kohadev 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127894|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |BLOCKED --- Comment #28 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK, I split out the first patch and moved it to bug 29575 and set it as a dependency. This allows other bugs to utilise this great new function ;) I've added my SO to the patches here and am just finishing up QAing the embed dependency.. will switch to PQA along with that one once I'm done. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #29 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Good idea thanks! :D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Bug 29275 depends on bug 29575, which changed state. Bug 29575 Summary: Add a JS render function equivalent of the patron-title.inc TT include https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29575 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128001|0 |1 is obsolete| | --- Comment #30 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 131333 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131333&action=edit Bug 29275: Use the API to fetch checkouts information This patch changes issuehistory.pl so it leverages on the API for fetching checkouts information in the template. Workflow changes: This development splits the 'current' checkouts and the 'past' checkouts in two different tables. Terminology could be revisited by native english speakers. To test: 1. Have records with and without current/past checkouts 2. Notice how the Checkouts history tab presents the information on each case. => SUGGESTION: Keep each of them open on separate tabs for comparing 3. Apply this patchset and its dependencies 4. Restart plack, run: $ koha-plack --restart kohadev 5. Follow the same steps you followed for those open tabs, in new ones => SUCCESS: What you see makes sense, information is displayed as it used to, with the different tables now. 6. Try searching on the datatables => SUCCESS: Search works correctly! 7. Revisit the new texts I added => SUGGESTION: If you have better wordings, submit a patch, or just comment on the bug so I do it inline. 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128002|0 |1 is obsolete| | --- Comment #31 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 131334 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131334&action=edit Bug 29275: (follow-up) Remove use of CountSubscriptionFromBiblionumber Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
SELECT * FROM columns_settings WHERE tablename='checkoutshistory-table'; => SUCCESS: Koha currently works :-D
SELECT * FROM columns_settings WHERE tablename LIKE '%_checkouts-table'; => SUCCESS: old_checkouts-table and current_checkouts-table have the right setting
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128003|0 |1 is obsolete| | --- Comment #32 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 131335 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131335&action=edit Bug 29275: (follow-up) Split columns settings and handle upgrade This patch splits the table settings as we now have two tables. The atomic update takes care of setting the right values (if present) on the new tables entries. To test: 1. Do not apply this patch 2. Set some values you will remember for the checkoutshistory-table table 3. Run: $ koha-mysql kohadev 4. Apply this patch 5. Run: $ updatedatabase 6. Repeat 3 => SUCCESS: Things have been deleted 7. Run: $ koha-mysql kohadev 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #33 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- A quick rebase, now moving on to testing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #34 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK, the rebase was trivial enough, but I think we need some more work in context of bug 29523.. and there's some weird things happening in the call to kohaTable.. for instance I'm not seeing the embeds get added to the query here in my testing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=23838 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #35 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK, this needs another rebase and there's a bit more to it this time so I think I need to defer to you Tomas. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #36 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Eager to see this one move forward :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #37 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- + if ( row.patron != null ) { + return $patron_to_html( row.patron, { display_cardnumber: true, url: true } ); + } + else { + return _("A patron from library %s").format(escape_str(row.library.name)); } This is not correct, is it? row.library.name should be row.patron.library.name, but we don't have it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #38 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #37)
+ if ( row.patron != null ) { + return $patron_to_html( row.patron, { display_cardnumber: true, url: true } ); + } + else { + return _("A patron from library %s").format(escape_str(row.library.name)); }
This is not correct, is it? row.library.name should be row.patron.library.name, but we don't have it.
+Strings -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #39 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Tomás Cohen Arazi from comment #38)
(In reply to Jonathan Druart from comment #37)
+ if ( row.patron != null ) { + return $patron_to_html( row.patron, { display_cardnumber: true, url: true } ); + } + else { + return _("A patron from library %s").format(escape_str(row.library.name)); }
This is not correct, is it? row.library.name should be row.patron.library.name, but we don't have it.
+Strings
lol yes if you want, but that was not my point :D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #40 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #37)
+ if ( row.patron != null ) { + return $patron_to_html( row.patron, { display_cardnumber: true, url: true } ); + } + else { + return _("A patron from library %s").format(escape_str(row.library.name)); }
This is not correct, is it? row.library.name should be row.patron.library.name, but we don't have it.
It took me a minute, but I think I see what you're saying. "row.library.name" would be the name of the library where the checkout occurred - not the name of the library the patron is from. While it's possible that these two values could be the same, it's also possible that they could be different. Wouldn't it make more sense to just change the text to something like: "Patron details suppressed", or "Unknown Patron", or even ""? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #41 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #40)
Wouldn't it make more sense to just change the text to something like: "Patron details suppressed", or "Unknown Patron", or even ""?
"Patron details currently unavailable" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #42 from David Cook <dcook@prosentient.com.au> --- But I could also be misunderstanding how this is supposed to work... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #43 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to David Cook from comment #40)
Wouldn't it make more sense to just change the text to something like: "Patron details suppressed", or "Unknown Patron", or even ""?
We could, but we remove information. IIRC it was part of the specs (bug 18403). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Failed QA --- Comment #44 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Gave a go at rebasing but can't arbitrate on a conflict about a change of HTML header level. (both in this patch and main/master) Anyway, this doesn't seem to be needing a rebase right now but rather settling on what to do about comment 37 (continued in Comment 40 and Comment 43) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #45 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Still love to see this one move forward :S -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |ASSIGNED --- Comment #46 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Gonna do some QA work on bug 33568, and will align this patchset with what Jonathan has done there! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Bug 29275 depends on bug 29523, which changed state. Bug 29523 Summary: Add a way to prevent embedding objects that should not be allowed https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29523 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |33568 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33568 [Bug 33568] Use the REST API to display items on the biblio detail view -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=15219 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #47 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- One to strive for early in 24.11.x cycle Tomas? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #48 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #47)
One to strive for early in 24.11.x cycle Tomas?
Totally. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #49 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Shall we put this one on the roadmap? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |roadmap_24_11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #50 from Jonathan Druart <jonathan.druart@gmail.com> --- ping? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Bug 29275 depends on bug 33568, which changed state. Bug 33568 Summary: Use the REST API to display items on the staff biblio detail view https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33568 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #51 from Fridolin Somers <fridolin.somers@biblibre.com> --- This would be great, we just had a crash from a huge circ history -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29275 --- Comment #52 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #50)
ping?
I totally forgot about this. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org