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

Marcel de Rooy <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Patch complexity|---                         |Medium patch
             Status|Signed Off                  |Passed QA

--- Comment #47 from Marcel de Rooy <[email protected]> ---
QA Comment (by patch number):

0002
Delete CSV Profile "Test01?"
Yes, delete
No, do not Delete
I would suggest to change this into Yes and No. This looks weird. 

0005
If you provide a wrong csv id to export_records.pl, it will generate this
error:
Can't call method "encoding" on an undefined value at
/usr/share/koha/masterclone/C4/Record.pm line 481.
This is more or less outside the scope of this report.
As a side note, I think it is kind of strange to pass record id's to a script.
I would have expected the profile name here.
At least the validity of this csv argument could be tested somewhere.
If I choose the format csv, I think it is wrong to output to a file called
koha.mrc. (Yes, I did not add the filename parameter. Should be obvious here.) 

0006
my $profile = Koha::CsvProfiles->find($id);
This is actually the line that I refer to in my former comment. 
What if this find does not have any results? You cannot assume that you will
find a record here. You should test.
Not a blocker, but please provide a follow-up.

Passed QA

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
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/

Reply via email to