[Koha-bugs] [Bug 7178] Improve order item creation

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7178

Paul Poulain <paul.poulain at biblibre.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Passed QA                   |Pushed to Master
                 CC|                            |paul.poulain at biblibre.com

--- Comment #29 from Paul Poulain <paul.poulain at biblibre.com> ---
(In reply to comment #28)
> Larger patch. Code looks good to me, apart from some remarks below. Has got
> a lot of attention in testing providing some more grounds to push it.
> 
> check_uniqueness.pl: I do not really object to this code, but this script
> contains SQL code and injects CGI params into SQL (now guarded fine by grep,
> but what happens later?). Would it be better to call a function in a module
> and have your code inspect the CGI params somewhat more explicitly in terms
> of maintainability? I am not blocking this, leave it up to RM ;)
> 
> About the javascript errors: I am still quite sure that this code produces a
> few js errors. (Already when I open form New order from empty record). But I
> do not have the time to debug it. If you change part A, part B could start
> generating errors now. Programming is fun ;) It does not prevent me from
> entering an order. So we could find it later. No reason to block a lot of
> work either.
> 
> Finally, the discussion on quantity zero and the extra click. Personally I
> would not be satisfied with leaving it that way. But perhaps we can still
> work on it when it reaches master.
> 
> So updating status to Passed QA and give Paul a chance to judge it ...

Well... with great power comes great responsibilities...

I should mark failed QA for the following reason:
#1 SQL inside a .pl, that is forbidden
#2 Marcel has some js errors that others can't reproduce (including me), those
errors don't prevent the feature to work
#3 the SHOW COLUMN is now to be avoided (see
http://wiki.koha-community.org/wiki/PostgreSQL)
(#4 the "0/1" thing is secondary, it's an improvement)

BUT:
* This patch contains an interesting feature, that works from a user point of
view
* we've almost reached feature freeze, and I think it's worth adding it.

So = patch pushed, but Julian, please provide a follow-up for #1 and #2

Julian: for the record: in french, the : has a space before and after. But in
english, it has a space just after.
So i've added a tiny follow-up to remove a space (this follow-up also fixes
some ucfirst-ed words that should not be)

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