[Koha-bugs] [Bug 9016] Multi transport types for notices

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9016

--- Comment #87 from M. de Rooy <m.de.rooy at rijksmuseum.nl> ---
First QA comment:

  * 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?

  * 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? 
  * t/db_dependent/Letters.t passed
    Just noting that the value 'feed' is not added in the test. Seems no
problem..
  * 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?
  * 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.
  * 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 >>. 
    This line (among others) got killed: myQuery.value = chaineSql.substring(0,
startPos) +'<<'+ chaineAj+'>>' + chaineSql.substring(endPos, chaineSql.length);
    Note that this does not build up confidence in the test stage. (Did you
perhaps repair this on another report already?) 
    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]..)

  * Patch 2
  * Test t/db_dependent/Overdues.t passed
  * Functionality clash? For Overdue notice (phone notice): I can select all
transport types. Confusing..
  * From commit: [Currently, only email, sms and print are relevant.] Note that
you could hide what is not relevant now?
  * Glancing through tools/overduerules.pl: If you move First, Second, etc.
from template to script, you do not translate them anymore?
    Note that I would not disapprove the idea of more flexibility there..

  * 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. 
  * Other patches: no specific comments at this time.

Changing status for now.

-- 
You are receiving this mail because:
You are watching all bug changes.