http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5339
Katrin Fischer <katrin.fisc...@bsz-bw.de> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|Needs Signoff |Failed QA
CC| |katrin.fisc...@bsz-bw.de
See Also| |http://bugs.koha-community.
| |org/bugzilla3/show_bug.cgi?
| |id=6504
--- Comment #7 from Katrin Fischer <katrin.fisc...@bsz-bw.de> 2012-01-22
20:15:29 UTC ---
Hi Julian,
I looked at the code and did some tests. These are my findings:
1) I would be nice if you could add documentation for the new tables in
kohastructure.sql. I wonder if it would be better to name the table aqinvoices.
This would make it more consistent with the other table names.
2) Remove MySQLisms
+ `closedate` date default NULL,
Coding guidelines: don't use ` in table or column names (this is a mySQLism)
3) Only a note: Patch moves invoice information from aqorders into the new
table invoices. I think testing this with a larger database would be good - my
test database is only very small and has only a few invoices.
4) Perhaps it would be good to check that all invoicenumbers have been moved
correctly before dropping the column. The datatype in invoices is smaller than
before: mediumtext vs. varchar(80).
+ $dbh->do("
+ ALTER TABLE aqorders
+ DROP COLUMN booksellerinvoicenumber
+ ");
5) 1st patch includes kohaversion, but file should not be in the patch.
6) Patch adds new subs to C4. It would be nice if you could provide unit tests
for those.
+ &GetInvoices
+ &GetInvoice
+ &GetInvoiceDetails
+ &AddInvoice
+ &ModInvoice
+ &CloseInvoice
+ &ReopenInvoice
7) From looking at the code I wondered about date formatting, but couldn't
check the display. Perhaps you want to make use of the new TT filter for dates.
http://wiki.koha-community.org/wiki/Coding_Guidelines#Displaying_dates
7) Both patches don't apply any longer.
Can you please fix them and resubmit? Thx!
--
Configure bugmail:
http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.
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/
