http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9016 --- Comment #88 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to M. de Rooy from comment #87)
First QA comment:
\o/ Thanks Marcel!
* General: Feature looks good (although not complete yet). Seems to be a welcome addition. At this moment in the process, it is not clear what is used (probably only email) and what is not. That makes a push not very obvious?
I tried to explain the complete feature in a previous email to koha-devel: all patches should be reviewed together and pushed together, in order to keep thing consistent (for the review, not the same day :))
* Patch 1 * Functionality: I can imagine that I have filled in multiple transport types but want to activate or deactivate some of them. Would you need some status on each of them?
I am sorry but I don't understand what you mean here.
* POD of GetMessageTransportTypes says: returns a list of hashes?? But it returns an arrayref! (See also Patch 3, yes.) * BTW why not use my $mtts = $dbh->selectcol_arrayref.. instead of the more complex map { } .. in GetMessageTransportTypes?
Yes, will be fixed.
* Just noting that we still have lots of SQL code in tools/letter.pl. Too bad you didn't move it ;) No blocker for me.
Not too much at the same time :)
* Textual: You must specify a title and a content -> Please specify title and content [a content sounds funny to me as non-native speaker] * IMPORTANT: The Insert button does no longer surround the fieldname with << and >>.
Good catch, will be fixed!
When resolving bugs crosses the boundaries of a patch set, it could be hard to perform QA. (I simply cannot qa 30 patches at 8 reports at one time [numbers at random, no offense]..)
I tried to fix issues on the same report, all linked reports do something different. Except if it is a bug on master, I open a new report.
* Patch 2 * Functionality clash? For Overdue notice (phone notice): I can select all transport types. Confusing..
I don't find this notice in sql files. Normally, with this feature, all notices created for a specific transport type, should be moved "into" the "main" notice. For instance, "Overdue notice (phone notice)" should be moved into the "phone" template for the "Overdue" notice.
* From commit: [Currently, only email, sms and print are relevant.] Note that you could hide what is not relevant now?
Maybe someone else would like to implement phone, etc. So I did not restrict the mtts to display on the interface.
* Glancing through tools/overduerules.pl: If you move First, Second, etc. from template to script, you do not translate them anymore?
Yes, they are translatable: var tab_map = { "1" : _("First"), "2" : _("Second"), "3" : _("Third")};
* Patch 4 The dbrev prints: Upgrade done (Bug 9016: Adds the association table overduerules_transport_types) The db rev does a lot more than that (or more important things..) It also adds e.g. message_transport_type to letter which is closer to the 'general theme'. Please change the print message to a more generic one.
Will be fixed. -- You are receiving this mail because: You are watching all bug changes.