[Bug 25008] New: Koha::RecordProcessor->options doesn't refresh the filters
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Bug ID: 25008 Summary: Koha::RecordProcessor->options doesn't refresh the filters Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org On testing something in opac-showmarc.pl I noticed it wasn't applying the ViewPolicy filter! And it seems the way the record processor is used is to blame. I plan to submit a fix for opac-showmarc.pl but thought it was worth filing a bug against Koha::RecordProcessor. -- 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=25008 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 102019 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102019&action=edit Bug 25008: Regression tests This patch highlights a behaviour of Koha::RecordProcessor that is unexpected: if you change the original options using ->options, the loaded filters don't pick the change. That's because the filter objects are loaded on ->new, and they are never updated. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/RecordProcessor.t => FAIL: Test prove ->options doesn't update the filters! -- 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=25008 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org, | |mtompset@hotmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Problematic uses: $ git grep 'processor\->options' opac/opac-basket.pl: $record_processor->options({ opac/opac-downloadcart.pl: $record_processor->options({ opac/opac-downloadshelf.pl: $record_processor->options({ opac/opac-shelves.pl: $record_processor->options({ -- 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=25008 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- The easiest for now seems to make it RO and modify the callers, what do you think? -- 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=25008 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #3)
The easiest for now seems to make it RO and modify the callers, what do you think?
I agree. I filled this bug so I don't forget about the problem. We want to be able to update the options when processing lists of records (e.g. lists, carts, etc) so we don't generate new objects only to change (say) the items. In those, we should cache the objects on a per-framework basis. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 102082 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102082&action=edit Bug 25008: Tests for ->options -- 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=25008 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 102083 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102083&action=edit Bug 25008: Overload Koha::RecordProcessor->options to update filters This patch overloads the 'options' accessor generated by Class:Accessor. It does so the passed options are used to refresh the loaded filters. Tests are added for this overloaded method as well. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/RecordProcessor.t => SUCCESS: Tests pass! 3. Sign off :-D -- 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=25008 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Status|NEW |Needs Signoff -- 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=25008 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102019|0 |1 is obsolete| | Attachment #102082|0 |1 is obsolete| | Attachment #102083|0 |1 is obsolete| | --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 102086 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102086&action=edit Bug 25008: Regression tests This patch highlights a behaviour of Koha::RecordProcessor that is unexpected: if you change the original options using ->options, the loaded filters don't pick the change. That's because the filter objects are loaded on ->new, and they are never updated. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/RecordProcessor.t => FAIL: Test prove ->options doesn't update the filters! Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 102087 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102087&action=edit Bug 25008: Tests for ->options Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 102088 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102088&action=edit Bug 25008: Overload Koha::RecordProcessor->options to update filters This patch overloads the 'options' accessor generated by Class:Accessor. It does so the passed options are used to refresh the loaded filters. Tests are added for this overloaded method as well. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/RecordProcessor.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25019 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- The patch and description are very technical - should we expect that this fixes the display issues on the names pages? And should this not be verified in testing? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I tested this by hiding 100$a and 245$c via the framework settings: Problematic = information still shows - Result list - Cart - normal and more detailed view - MARC download from cart/list - MARC download from detail page (245$c kept showing?) Couldn't test the emails sent form list and shelf. Probably an entirely different thing to discuss: - Use for a suggestoin (from detail page) - author is in the form - Place hold (probably using other methods of pulling the information) Good = information doesn't show - Detail page, normal view: hides - Detail page, plain and MARC (plain fixed by patch) - ISBD view - RIS export - ISBD export - Bibtex export - Dublin Core export - MARCXML export - List view (fixed by patch) I am willing to PQA on ths, but could you take a list at the problematic ones first? Result list especially. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Katrin Fischer from comment #11)
I tested this by hiding 100$a and 245$c via the framework settings:
Problematic = information still shows - Result list - Cart - normal and more detailed view - MARC download from cart/list - MARC download from detail page (245$c kept showing?) Couldn't test the emails sent form list and shelf.
Probably an entirely different thing to discuss: - Use for a suggestoin (from detail page) - author is in the form - Place hold (probably using other methods of pulling the information)
Good = information doesn't show - Detail page, normal view: hides - Detail page, plain and MARC (plain fixed by patch) - ISBD view - RIS export - ISBD export - Bibtex export - Dublin Core export - MARCXML export - List view (fixed by patch)
I am willing to PQA on ths, but could you take a list at the problematic ones first? Result list especially.
Katrina, I was waiting for you to show up :-) Please file bugs for the problematic ones and sign them to me. I will happily deal with them. They are not related to this bug directly, they don't use RecordProcessor. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
I am willing to PQA on ths, but could you take a list at the problematic ones first? Result list especially.
Katrina, I was waiting for you to show up :-) Please file bugs for the problematic ones and sign them to me. I will happily deal with them. They are not related to this bug directly, they don't use RecordProcessor.
Should have read 'take a look'(not list). It looks like the bugs mailing list is out of order at the moment, which means I might be missing things. Please email me directly if you need me to show up somewhere else :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102086|0 |1 is obsolete| | --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 102806 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102806&action=edit Bug 25008: Regression tests This patch highlights a behaviour of Koha::RecordProcessor that is unexpected: if you change the original options using ->options, the loaded filters don't pick the change. That's because the filter objects are loaded on ->new, and they are never updated. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/RecordProcessor.t => FAIL: Test prove ->options doesn't update the filters! Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102087|0 |1 is obsolete| | --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 102807 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102807&action=edit Bug 25008: Tests for ->options Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102088|0 |1 is obsolete| | --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 102808 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102808&action=edit Bug 25008: Overload Koha::RecordProcessor->options to update filters This patch overloads the 'options' accessor generated by Class:Accessor. It does so the passed options are used to refresh the loaded filters. Tests are added for this overloaded method as well. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/RecordProcessor.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25124 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25125 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25126 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Tomás Cohen Arazi from comment #12)
(In reply to Katrin Fischer from comment #11)> Katrina, I was waiting for you to show up :-) Please file bugs for the problematic ones and sign them to me. I will happily deal with them. They are not related to this bug directly, they don't use RecordProcessor.
Done: bug 25124, bug 25125, bug 25126 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |katrin.fischer@bsz-bw.de |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|20.05.00 |20.05.00, 19.11.06 released in| | CC| |joy@bywatersolutions.com --- Comment #19 from Joy Nelson <joy@bywatersolutions.com> --- qa tests pass but i do get an error on pod (see below) I did backport to 19.11.x for 19.11.06 FAIL Koha/RecordProcessor.pm OK critic OK forbidden patterns OK git manipulation FAIL pod *** ERROR: Spurious text after =cut in file Koha/RecordProcessor.pm *** ERROR: Spurious text after =cut in file Koha/RecordProcessor.pm *** ERROR: Spurious text after =cut in file Koha/RecordProcessor.pm *** ERROR: Spurious text after =cut in file Koha/RecordProcessor.pm OK pod coverage OK spelling OK valid -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 --- Comment #20 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Katrina wouldn't have allowed me to submit something like that. I guess it is because of the rebase. I can go fix it with a follow-up patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 --- Comment #21 from Joy Nelson <joy@bywatersolutions.com> --- Entirely likely my fault! I would it if you would take a look at it. I thought I had rebased it well and when I went back to check nothing looked wrong to me. Thanks! joy (In reply to Tomás Cohen Arazi from comment #20)
Katrina wouldn't have allowed me to submit something like that. I guess it is because of the rebase. I can go fix it with a follow-up patch.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 --- Comment #22 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Joy Nelson from comment #21)
Entirely likely my fault! I would it if you would take a look at it. I thought I had rebased it well and when I went back to check nothing looked wrong to me.
Thanks! joy
(In reply to Tomás Cohen Arazi from comment #20)
Katrina wouldn't have allowed me to submit something like that. I guess it is because of the rebase. I can go fix it with a follow-up patch.
It is a POD error so no rush. I will fix it first thing in the morning with Javier -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25008 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|20.05.00, 19.11.06 |20.05.00, 19.11.06, released in| |19.05.11 Status|Pushed to master |Pushed to stable CC| |lucas@bywatersolutions.com --- Comment #23 from Lucas Gass <lucas@bywatersolutions.com> --- Same pod error for me but I backported regardless. Backported to 19.05.x for 19.05.11 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org