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
Created attachment 176170 [details] [review] 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.
Created attachment 177146 [details] [review] 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 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.
(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
Created attachment 177691 [details] [review] 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>
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
Pushed for 25.05! Well done everyone, thank you!
(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 :)
(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 .....
Nice work everyone! Pushed to 24.11.x for 24.11.03