[Koha-bugs] [Bug 8215] Add Course Reserves
bugzilla-daemon at bugs.koha-community.org
bugzilla-daemon at bugs.koha-community.org
Fri Sep 14 12:08:11 CEST 2012
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8215
Paul Poulain <paul.poulain at biblibre.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|Passed QA |Failed QA
CC| |paul.poulain at biblibre.com
--- Comment #52 from Paul Poulain <paul.poulain at biblibre.com> ---
QA comments:
To begin, I'm really sorry to fail QA this large patch, with a great feature,
but I think it's needed
* I don't like at all the introduction of
+sub GetItemTypesFlat {
+ my $dbh = C4::Context->dbh;
+ my $query = "SELECT * FROM itemtypes ORDER BY description";
+ my $sth = $dbh->prepare($query);
+ $sth->execute;
+
+ return $sth->fetchall_arrayref({});
+}
That's another way to access itemtype. We need consistency, not more and more
various way to access the same information
That would have been a wonderful time to introduce Koha::Service::Itemtype.pm !
Can you find a way to avoid introducing this function ?
* The introduction of Koha/CourseReserves.pm is wrong:
- it must follow the structure we've defined at the hackfest and updated after
some tests I made (see mail on koha-devel, june 25th, Re: [Koha-devel]
http://wiki.koha-community.org/wiki/Koha_Namespace_RFC, moving ahead
- it must be OOP, as everything that enters Koha::
- parameters must be passed by hashes (this one is OK)
I let you 2 options here :
* you rewrite CourseReserves.pm in Koha:: namespace to respect the rule
* you move it to C4/ as it's an 'old style' stuff
(I bet you'll follow the 2nd patch, that is *much* easier, but that would have
been a great occasion to start filling Koha:: :\ )
* mod_course.pl says:
+#script to modify reserves/requests
+#written 2/1/00 by chris at katipo.oc.nz
+#last update 27/1/2000 by chris at katipo.co.nz
+# Copyright 2000-2002 Katipo Communications
are you sure this code is 12 years old ? ;-)
* There are no foreign keys, we can have some (not sure this list is complete):
- instructors.borrowernumbers => borrowers.borrowernumber
- instructors.course_id => courses.course_id
- course_items.itemnumber => items.itemnumber
- course_items.holdingbranch => branches.branchcode
- course_reserves.course_id => courses.course_id
Not failing QA for that but:
* scripts names sounds a little bit long/duplicated :
course_reserves/course-details.pl => could be course/detail, don't you think ?
- do we really need course_reserve as directory ?
- do we need to repeas course in course-detail.pl
* the course_reserve table has an index with a probable typo in the name:
+ UNIQUE KEY `psuedo_key` (`course_id`,`ci_id`), => PSEUDO, not PSUEDO ?
QUESTIONS:
* what are the changes to
.../prog/en/modules/members/mancredit.tt | 2 +-
.../prog/en/modules/members/maninvoice.tt | 2 +-
.../prog/en/modules/members/memberentrygen.tt | 2 +-
done for ? Why is it in this patch ?
PS : if you decide to go to Koha::Service::Itemtype.pm, don't do it with
DBIx::Class, just with standard SQL. Introducing DBIx::Class is a huge thing,
that must be done separately
--
You are receiving this mail because:
You are watching all bug changes.
More information about the Koha-bugs
mailing list