https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17047
Jonathan Druart <jonathan.dru...@bugs.koha-community.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|Passed QA |Failed QA
--- Comment #162 from Jonathan Druart <jonathan.dru...@bugs.koha-community.org>
---
Quick code review:
1/ Please squash patches when possible to ease readability
2/ Some strings are not translatable (at least "Subscription found on Mana
Knowledge Base")
3/ Please fix indentation
4/ Remove use of onclick attribute
5/
- [% INCLUDE 'serials-toolbar.inc' %]
+ [% INCLUDE 'serials-toolbar.inc' mana_id = mana_id %]
Not sure it is needed
6/ + 'mana_success' => $input->param('mana_success'),
CGI->param raises warning called in list context (there are other occurrences)
7/ subroutines added in serials/subscription-add.pl smell
8/ serials/subscription-add.pl
+ my $mana_id;
+ if ( $query->param('mana_id') ne "" ) {
+ $mana_id = $query->param('mana_id');
+ Koha::SharedContent::manaIncrementRequest("subscription",$mana_id,
"nbofusers");
+ }
+ else {
+ $mana_id = undef;
+ }
=> the else block is useless
9/ Too many "use" statements in serials/subscription-detail.pl
10/ script names in svc/mana are not consistent
11/
+my $templatename;
+$templatename = "mana/mana-".$input->param( "resource" )."-search-result.tt";
That smells too. There are only 2 files, list them (same for other
occurrences).
12/ Test coverage for Koha::SharedContent is nonexistent
13/ Why tests have been removed from
t/db_dependent/Serials/GetFictiveIssueNumber.t
14/ No reference of mana_config from debian/templates/koha-conf-site.xml.in
--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://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/
