Bug 11459 - [Signed-Off] OrderPdfFormat pref description could be more verbose
Summary: [Signed-Off] OrderPdfFormat pref description could be more verbose
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: mathieu saby
QA Contact: Testopia
URL:
Keywords:
Depends on: 10746
Blocks:
  Show dependency treegraph
 
Reported: 2013-12-29 23:01 UTC by mathieu saby
Modified: 2015-06-04 23:30 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 11459: Make OrderPdfFormat syspref description more verbose (1.46 KB, patch)
2014-01-26 22:57 UTC, mathieu saby
Details | Diff | Splinter Review
[PATCH] Bug 11459: Make OrderPdfFormat syspref description more verbose (1.47 KB, patch)
2014-01-28 07:28 UTC, mathieu saby
Details | Diff | Splinter Review
Bug 11459: Make OrderPdfFormat syspref description more verbose (1.60 KB, patch)
2014-02-17 18:29 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 11459: Make OrderPdfFormat syspref description more verbose (1.69 KB, patch)
2014-02-23 22:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
[COUNTER PATCH] Bug 11459: use drop-down to set OrderPdfFormat syspref (1.55 KB, patch)
2014-02-26 16:50 UTC, Galen Charlton
Details | Diff | Splinter Review
[Signed-off] Bug 11459: use drop-down to set OrderPdfFormat syspref (1.73 KB, patch)
2014-02-27 21:41 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 11459: use drop-down to set OrderPdfFormat syspref (1.89 KB, patch)
2014-03-12 17:23 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description mathieu saby 2013-12-29 23:01:29 UTC
The description is:
" 	Use when printing basket groups. "

The list of possible templates should be precised.


M. Saby
Comment 1 mathieu saby 2014-01-26 22:46:23 UTC
There are only 3 valid values for OrderPdfFormat :
pdfformat::layout2pages
pdfformat::layout3pages
in french:pdfformat::layout3pagesfr


We should improve syspref description, and the manual.

M. Saby
Comment 2 mathieu saby 2014-01-26 22:57:44 UTC Comment hidden (obsolete)
Comment 3 mathieu saby 2014-01-28 07:28:12 UTC Comment hidden (obsolete)
Comment 4 Marc Véron 2014-02-17 18:29:12 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2014-02-23 22:52:43 UTC Comment hidden (obsolete)
Comment 6 Galen Charlton 2014-02-26 16:50:17 UTC Comment hidden (obsolete)
Comment 7 Galen Charlton 2014-02-26 16:51:29 UTC
The system preferences editor lets one specify a set of choice for a syspref.  A drop-down makes more sense than asking the user to copy and paste their chosen value from the system preference, and also makes it less error-prone.

Consequently, I've attached a counter-patch.
Comment 8 Marc Véron 2014-02-27 19:45:54 UTC
11828 will introduce a German translation. 

IHO, Mathieus patch would be more flexible for new translations.

Marc
Comment 9 Galen Charlton 2014-02-27 20:27:28 UTC
(In reply to Marc Véron from comment #8)
> 11828 will introduce a German translation. 
> 
> IHO, Mathieus patch would be more flexible for new translations.

Can you say why you think that?  In both cases, the list of options is subject to exactly the same translation tools, but my patch makes the display both more concise and less likely for a user to make an error that would break basketgroup printing.
Comment 10 Marc Véron 2014-02-27 21:13:34 UTC
I agree that the drop down list would be more user friendly. 

What I wanted to say is that without the drop down list it would be easier to add other templates by just adding the files.

And imagine that each and every of the Koha languages would have it's own template(s). That would give a very long drop down list.

BTW I tested an non-existing template (pdfformat::layout2pagesxx). It simply leads to an error message while printing: "Invalid PDF Format set - Please go to the systempreferences and set a valid pdfformat"
Comment 11 Katrin Fischer 2014-02-27 21:16:18 UTC
Adding the files is not enough - for security reasons the valid pdf formats are currently hardcoded into basketgroup.pl. So touching the code is always needed in some way to add another template currently.
Comment 12 Galen Charlton 2014-02-27 21:30:29 UTC
(In reply to Marc Véron from comment #10)
> I agree that the drop down list would be more user friendly. 

OK

> What I wanted to say is that without the drop down list it would be easier
> to add other templates by just adding the files.

Katrin has addressed this point.

> And imagine that each and every of the Koha languages would have it's own
> template(s). That would give a very long drop down list.

I am very dubious whether that is a real possibility, considering that it's been more more than four years since the feature was originally introduced, and only now is somebody adding support for a third language.

In fact, I really hope that we have a better templating system in place long before somebody contemplates adding a fourth language, as requiring a user to change Perl code and binary files to customize their printed purchase orders is, to put it mildly, less than ideal.

> BTW I tested an non-existing template (pdfformat::layout2pagesxx). It simply
> leads to an error message while printing: "Invalid PDF Format set - Please
> go to the systempreferences and set a valid pdfformat"

That still counts as broken.  Better to constrain the user to options that are guaranteed to work.
Comment 13 Marc Véron 2014-02-27 21:33:40 UTC
I was not aware of the hard coding in basketgroup.pl

So let's go ahead - Sign off follows.
Comment 14 mathieu saby 2014-02-27 21:34:07 UTC
Hello

I wrote something for allowing customized basketgroup.
It failed:
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10135

By the way, the current system (using perl for the pdf, and not a html template) is a nightmare. 

This bug was only to make the description of the syspref a bit more understandable for the average librarian ;-)

Mathieu
Comment 15 mathieu saby 2014-02-27 21:35:44 UTC
(in my library we made our own template - this was before a french template was available - so, we had to write a local patch for allowing koha to use a non-standard template)

Mathieu
Comment 16 Marc Véron 2014-02-27 21:41:24 UTC Comment hidden (obsolete)
Comment 17 Galen Charlton 2014-02-28 15:27:42 UTC
Sounds like we have an agreement, so I'm setting this to signed-off.
Comment 18 Marc Véron 2014-03-01 09:21:08 UTC
Thanks, Galen. I did not set it to "Signed off" because it was "In Discussion". 

Marc
Comment 19 Katrin Fischer 2014-03-12 17:23:22 UTC
Created attachment 26205 [details] [review]
[PASSED QA] Bug 11459: use drop-down to set OrderPdfFormat syspref

This patch converts the OrderPdfFormat to a choice.

To test:

[1] Look at OrderPdfFormat in the system preferences editor. Verify
    that there is a drop-down giving a choice among the three PDF
    basketgroup printing formats.
[2] Change the OrderPdfFormat setting and print a basketgroup.  Verify
    that the chosen template is used.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>

I followed the test plan. Patch behaves as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Improves usability of the OrderPdfFormat system preference.
Works as described, only changes YAML file.
Comment 20 Galen Charlton 2014-03-12 18:04:48 UTC
Pushed to master.
Comment 21 Jonathan Druart 2014-04-11 10:42:23 UTC
This change is quite confusing if you have a customized pdf, the selected value is the first one of the dropdown list. Which is different from the value in the DB.
Comment 22 Jonathan Druart 2014-04-11 10:45:29 UTC
I should have read the discussion before raising something already discussed.
Comment 23 Fridolin SOMERS 2014-06-04 10:06:04 UTC
Pushed to 3.14.x, will be in 3.14.08
Comment 24 Kyle M Hall 2014-06-06 13:59:51 UTC
Pushed to 3.12.x will be in 3.12.13