[Koha-bugs] [Bug 22544] Move C4:NewsChannels to Koha namespace

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

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

--- Comment #97 from Josef Moravec <josef.moravec at gmail.com> ---
(In reply to Katrin Fischer from comment #95)
> Hi Josef, 
> 
> trying to dive into this one here :) Could you please look at the comments
> before I continue testing?
> 
> 1) QA Tools show one fail:
> 
>  FAIL	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt
>    FAIL	  valid_template
> 		The type and lang parameters combination is not valid

I'll fix it

> 
> 2) search_for_display
> 
> Trying to understand the code, I stumbled a bit on the POD for this one:
> 
> +=head3 search_for_display
> +You can limit the results by type(lang) and library by optional params
> 
> I am not sure I follow you here (type(lang)?) - maybe you changed the
> signature of the method after this was written?
> 

I will polish the pod in follow-up patch

> +
> +library_id should be valid branchcode of defined library
> +
> +type is one of this:
> +- slip - for ISSUESLIP notice
> +- koha - for intranet
> 
> I think "koha" is not good for intranet, I think useing "intranet" or
> "staff" instead, would be better.

I will do that on (or after) bug 27451, 'koha' string is saved in BD, so it
makes sense to change it in DB not only in UI (in it would be easier)

> 
> +- opac - for online catalogue
> +- OpacNavRight - Right column in the online catalogue
> +- OpacLoginInstructions
> +- OpacMainUserBlock
> +- OpacCustomSearch
> +- opacheader
> +- opaccredits
> 
> +lang is language code - it is used only when type is opac or OpacNavRight
> 
> For future enh: I think once we have an "all languages" selection we should
> enable lang for the staff interface as well (I filed bug 27459)

Absolutely agree

> 
> I guess the other prefs hadn't been migrated yet when this comment was
> written.

True, I'll fix pod

> 
> 3) koha-news.tt
> 
> +                                    Librarian interface (koha)
> 
> I'd like to see the (koha) removed here again - both the OPAC and staff
> interface are Koha. My suggestion would be to change "Librarian interface"
> to "Staff interface" as this is the standard terminology we have agreed to.

Will do after/on bug 27451

> 
> 4) html_helpers.inc
> 
> I like the use of the patron-title.inc here - maybe we should use it in
> koha-news.tt too? 
> I am not sure about the hide_patron_infos_if_needed=1. I think we should
> only apply the NewsAuthorDisplay here, the difference being that these are
> staff users, not library patrons.
> 
> -                            <td>[% opac_new.author_title | html %] [%
> opac_new.author_firstname | html %] [% opac_new.author_surname | html %]</td>
> +                            <td>[% IF ( opac_new.author) %][% INCLUDE
> 'patron-title.inc' patron=opac_new.author hide_patron_infos_if_needed=1 %][%
> END %]</td>

Does make sense, I'll change it

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