https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33260

--- Comment #196 from Martin Renvoize (ashimema) 
<[email protected]> ---
(In reply to Jonathan Druart from comment #169)
> Very quick code review and testing:
> 1. Koha::Notice::Message->patron
> +    return unless $patron_rs;
> This is not tested

Fixed in yet another follow-up

> 2. Koha::Notice::Message->template
> +    return unless $template_rs;

Also fixed in yet another follow-up

> 3. QA script failures 
>  FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt
>     FAIL   tt_valid                              lines 75
>  FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/tools/notices.tt
>     FAIL   spelling                                                         
> 
>                  noice ==> noise, nice, notice              

Also fixed in a follow-up

> 4.
>   <th class="nosort">&nbsp;</th>
> 
> +                "aoColumnDefs": [
> +                    { 'bSortable': false, 'aTargets': [ 'nosort' ] }
> +                ],
> 
> You should use .NoSort on the th and don't need the aoColumnDefs.

Also fixed in a follow-up

> 5. members/notices.pl?borrowernumber=5
> Without entry we should not show the table but "There are no notices for
> this patron" instead.

This is already the case

> 6. "Pending" notices have their checkbox disabled, we should display a
> onmouseover tooltip.

Fixed in a follow-up

> 7. if you print a notice that was "failed", it's then "sent" but the
> "delivery note" failure is still displayed on the template. Does it make
> sense?

I raise this earlier in the thread of discussion and agree but I also don't
really know what the answer is... I think we aught to handle print and other
transports independently.. if it's print and it's printed yes, see it to sent..
if it's anything else perhaps add 'printed' to either the status or the
delivery notes

> 8. Is it expected to print using "email" mtt, shouldn't we use the "print"
> template instead?

Also raised above in discussion.. the objective I think was to allow printing
existing notices already queued.. it's hard to 'convert to print' as we don't
have the original objects data to hand after a message has been queued.

> 9. 2 different outputs:
>  * Using "Print" from the actions column
> http://localhost:8081/cgi-bin/koha/tools/print_notice.pl?message_ids=73
> Using CHECKIN, all on one line
> "The following items have been checked out: E Street shuffle Thank you for
> visiting Centerville."
>  * Using "Print" from the modal
> http://localhost:8081/cgi-bin/koha/members/notices.pl?borrowernumber=5
> "Checkouts
> The following items have been checked out:
> E Street shuffle
> Thank you for visiting Centerville."

I asked about this above.. they use different printing methods.. the modal
preview applies CSS and then you print the modal content.. the print from the
actions hits a print controller on the server to load a whole new page.

> Also the name of the pdf in the second example is "notices.pl.pdf"

I'd missed this

> 10. "Resend" - Shouldn't we actually enqueue another message in order to
> keep track of what have been failed/sent?

Resend predates all this work.. I don't know the answer but I think that should
be dealt with in a distinct bug.. personally I think this should have been two
bugs at least already.. one for display and a second for print handling.. the
recent additions to notices and template handling with CSS and things have
really complicated and confused matters for end users in my opinion and I'm
just trying to work through and clarify those in the bug tree beyond this bug.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to