From bugzilla-daemon@bugs.koha-community.org Tue Mar 22 08:31:10 2016
From: bugzilla-daemon@bugs.koha-community.org
To: koha-bugs@lists.koha-community.org
Subject: [Koha-bugs] [Bug 12461] Add patron clubs feature
Date: Tue, 22 Mar 2016 07:31:04 +0000
Message-ID: <bug-12461-70-af7r8pJxFI@https.bugs.koha-community.org/bugzilla3/>
In-Reply-To: <bug-12461-70@https.bugs.koha-community.org/bugzilla3/>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============6391098762303865439=="

--===============6391098762303865439==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

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

--- Comment #31 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
Comment on attachment 49121
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=3D49121
Bug 12461 - Add patron clubs feature

Review of attachment 49121:
 --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=3Dsplinter.html&b=
ug=3D12461&attachment=3D49121)
-----------------------------------------------------------------

Did a first code review - not a lot of findings and mostly minor. Haven't
tested the code yet.

Patch doesn't apply atm - not sure why the modules appear in the general
syspref.pm?

<<<<<<< HEAD
use C4::Log;
=3D=3D=3D=3D=3D=3D=3D
use Koha::Clubs;
use Koha::Club::Enrollments;
>>>>>>> Bug 12461 - Add patron clubs feature

::: Koha/Template/Plugin/AuthorisedValues.pm
@@ +30,4 @@
> =20
>  sub Get {
>      my ( $self, $category, $selected, $opac ) =3D @_;
> +    warn "GET( $self, $category, $selected, $opac )";

Unconditional warn - please remove.

::: koha-tmpl/intranet-tmpl/prog/en/modules/clubs/clubs-add-modify.tt
@@ +32,5 @@
> +<div class=3D"yui-t7">
> +    <div class=3D"yui-main">
> +        [% IF stored %]
> +            <div class=3D"alert">
> +                <p>Your club was [% IF stored =3D=3D 'updated' %] updated =
[% ELSE %] saved [% END %]</p>

Please avoid constructing sentences like that - it's hard to translate, becau=
se
grammar and work order is different in other languages.

::: koha-tmpl/intranet-tmpl/prog/en/modules/clubs/clubs.tt
@@ +23,5 @@
> +        } ));
> +    });
> +
> +    function ConfirmDeleteTemplate( id, name, a ) {
> +        if ( confirm( _("Are you sure you want to delete the club template=
") + name + "?" ) ) {

It would be nicer for translations to use the new Javascript syntax with
placeholders here.

@@ +41,5 @@
> +        }
> +    }
> +
> +    function ConfirmDeleteClub( id, name, a ) {
> +        if ( confirm( _("Are you sure you want to delete the club ") + nam=
e + "?" ) ) {

Same as above - placeholders preferrable!

::: koha-tmpl/intranet-tmpl/prog/en/modules/clubs/templates-add-modify.tt
@@ +9,5 @@
> +//<![CDATA[
> +
> +function CheckForm() {
> +  if ( !$("#club-template-name").val() ) {
> +    alert( _("Name is a required field!")  );

Would it be possible to use the standard markup for required fields insted
here? See administration pages for examples.

@@ +30,5 @@
> +<div class=3D"yui-t7">
> +    <div class=3D"yui-main">
> +        [% IF stored %]
> +            <div class=3D"alert">
> +                <p>Your club template was [% IF stored =3D=3D 'updated' %]=
 updated [% ELSE %] saved [% END %]</p>

Please don't construct sentences (see above).

::: koha-tmpl/opac-tmpl/bootstrap/en/modules/clubs/enroll.tt
@@ +27,5 @@
> +                    </li>
> +                [% END %]
> +
> +                <li>
> +                    <a href=3D"#" class=3D"btn" onclick=3D"addEnrollment()=
; return false;"><i class=3D"icon-plus"></i> Enroll</a>

Switch to FA icon - I saw most of them were already, maybe missed this one?

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

--===============6391098762303865439==--