[Bug 38838] New: optgroup construct needs cleaning in the reports module
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38838 Bug ID: 38838 Summary: optgroup construct needs cleaning in the reports module Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@gmail.com Reporter: jonathan.druart@gmail.com QA Contact: testopia@bugs.koha-community.org reports/dictionary.tt and reports/guided_reports_start.tt display a list of the columns available using optgroup to group the columns per table. The template code is quite ugly because of the perl structure choose in C4::Reports::Guided->get_columns -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38838 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |38714 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38714 [Bug 38714] [OMNIBUS] Adjust templates for prettier -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38838 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38838 --- Comment #1 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 176170 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176170&action=edit Bug 38838: Clean C4::Reports::Guided->get_columns reports/dictionary.tt and reports/guided_reports_start.tt display a list of the columns available using optgroup to group the columns per table. The template code is quite ugly because of the perl structure choose in C4::Reports::Guided->get_columns This patch removes $cgi and the 'first' variable that were not used. Now we have an usual structure with the table names as keys and column info in an arrayref. Test plan: Create a report and a new dictionary definition. At step 3 you should see the list of the columns. Note that they are now identical. The list when you create a dictionary now show the description of the columns. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38838 David Nind <david@davidnind.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=38838 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176170|0 |1 is obsolete| | --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 177146 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177146&action=edit Bug 38838: Clean C4::Reports::Guided->get_columns reports/dictionary.tt and reports/guided_reports_start.tt display a list of the columns available using optgroup to group the columns per table. The template code is quite ugly because of the perl structure choose in C4::Reports::Guided->get_columns This patch removes $cgi and the 'first' variable that were not used. Now we have an usual structure with the table names as keys and column info in an arrayref. Test plan: Create a report and a new dictionary definition. At step 3 you should see the list of the columns. Note that they are now identical. The list when you create a dictionary now show the description of the columns. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38838 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This enhancement updates release notes| |what is shown when | |selecting the columns when | |creating a new dictionary | |definition in reports. It | |now shows "Field | |description / | |tablename.fieldname", | |instead of just the "Field | |description" - the same as | |for creating reports. | |Example, 'Publication date | |/ | |biblioitems.publicationyear | |' (previously it just | |showed 'Publication date'). CC| |david@davidnind.com --- Comment #3 from David Nind <david@davidnind.com> --- Signed off and release note added. I'm not sure about changing the formatting of the table group title from bold to bold and italic/emphasis. I think I prefer just the bold. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38838 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |roadmap_25_05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38838 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #4 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (In reply to David Nind from comment #3)
Signed off and release note added.
Thanks :D
I'm not sure about changing the formatting of the table group title from bold to bold and italic/emphasis. I think I prefer just the bold.
What do you mean? I don't see a difference on that. main report creation: https://lutim.lagout.org/YxS2Ztt7/EziG1QO4.png vs with patch report creation: https://lutim.lagout.org/6nlRmlJu/eqTMYXF1.png main Dictionary new definition: https://lutim.lagout.org/0cb7U9uw/CSwG8gwH.png vs with patch Dictionary new definition: https://lutim.lagout.org/2dBXIb2J/gWG8zI6O.png -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38838 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177146|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 177691 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177691&action=edit Bug 38838: Clean C4::Reports::Guided->get_columns reports/dictionary.tt and reports/guided_reports_start.tt display a list of the columns available using optgroup to group the columns per table. The template code is quite ugly because of the perl structure choose in C4::Reports::Guided->get_columns This patch removes $cgi and the 'first' variable that were not used. Now we have an usual structure with the table names as keys and column info in an arrayref. Test plan: Create a report and a new dictionary definition. At step 3 you should see the list of the columns. Note that they are now identical. The list when you create a dictionary now show the description of the columns. Signed-off-by: David Nind <david@davidnind.com> 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=38838 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com Status|Signed Off |Passed QA --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Also looks OK to me.. in the name of progress and I'm going to PQA.. we can revisit later if there does prove to be resistance.. but the code is much cleaner now. Passing -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38838 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |25.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=38838 --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 25.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38838 --- Comment #8 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (In reply to Martin Renvoize (ashimema) from comment #6)
Also looks OK to me.. in the name of progress and I'm going to PQA.. we can revisit later if there does prove to be resistance.. but the code is much cleaner now.
Thanks for review. As far as I looked at the code changes in the previous days, I hadn't find anything suspicious yet. So let's go with this :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38838 --- Comment #9 from David Nind <david@davidnind.com> --- (In reply to Victor Grousset/tuxayo from comment #4)
(In reply to David Nind from comment #3)
Signed off and release note added.
Thanks :D
I'm not sure about changing the formatting of the table group title from bold to bold and italic/emphasis. I think I prefer just the bold.
What do you mean? I don't see a difference on that. main report creation: https://lutim.lagout.org/YxS2Ztt7/EziG1QO4.png vs with patch report creation: https://lutim.lagout.org/6nlRmlJu/eqTMYXF1.png
main Dictionary new definition: https://lutim.lagout.org/0cb7U9uw/CSwG8gwH.png vs with patch Dictionary new definition: https://lutim.lagout.org/2dBXIb2J/gWG8zI6O.png
Not sure anymore. In an updated main, the field group heading is just bold. Example using circulation: biblioitems <-- This is now just bold, when I tested it was bold and italics Biblio item number (internal) / biblioitems.biblioitemnumber ..... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38838 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |paul.derscheid@lmscloud.de Version(s)|25.05.00 |25.05.00,24.11.03 released in| | Status|Pushed to main |Pushed to stable --- Comment #10 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Nice work everyone! Pushed to 24.11.x for 24.11.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38838 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wainuiwitikapark@catalyst.n | |et.nz Status|Pushed to stable |Needs documenting --- Comment #11 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11.x as it wasn't backported to 24.05.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org