Bug 6740 - can add items at ordering but not remove items
Summary: can add items at ordering but not remove items
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: 3.6
Hardware: All All
: PATCH-Sent (DO NOT USE) critical (vote)
Assignee: Katrin Fischer
QA Contact: Bugs List
URL: /cgi-bin/koha/acqui/neworderempty.pl?...
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-17 13:07 UTC by Nicole C. Engard
Modified: 2019-06-27 09:24 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
screenshot (38.86 KB, image/png)
2011-10-26 22:18 UTC, Katrin Fischer
Details
screenshot of working setup (200.79 KB, image/png)
2011-11-01 04:42 UTC, Paul Poulain
Details
screenshot of firebug output (78.66 KB, image/png)
2011-11-01 09:20 UTC, Katrin Fischer
Details
Bug 6740: can add items at ordering/receiving but not remove (4.04 KB, patch)
2011-12-05 06:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
Signed-off patch (4.14 KB, patch)
2011-12-05 15:36 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2011-08-17 13:07:59 UTC
There used to be a minus sign on the ordering screen to remove your extra items if you clicked the plus too many times.  That needs to come back.
Comment 1 Paul Poulain 2011-10-26 16:02:49 UTC
Nicole, I just tested on a 3.6.0, and the minus sign is here, and works (I agree it's smaller than the +
Also note it appears only when you have the 2nd item created (which is logical)
Comment 2 Katrin Fischer 2011-10-26 22:18:45 UTC
Created attachment 6067 [details]
screenshot

Hi Paul, 
I retested this on 3.6 now and I still can see no minus. I was ordering from a suggestion, see attached screenshot
Comment 3 Paul Poulain 2011-11-01 04:41:26 UTC
I tried again, and it works again...
What I did :
 * adding a line from a suggestion, no ACQ framework
 * adding a line from a suggestion, with ACQ framework
 * tried in both french and english

I attach my screenshot (with the URL)
Comment 4 Paul Poulain 2011-11-01 04:42:30 UTC
Created attachment 6127 [details]
screenshot of working setup

this screenshot shows a small minut on the right of the second +
Comment 5 Katrin Fischer 2011-11-01 08:15:17 UTC
Could it be the MARC flavor?
Comment 6 Paul Poulain 2011-11-01 08:48:56 UTC
I can't imagine a relation with MARC here. 

Here is the code :
            <a style="cursor: pointer; color: grey; font-size: 180%;" onclick="cloneItemBlock('itemblock[% item.itemBlockIndex %]')">+</a>
            <a style="display:none; cursor: pointer; color: grey; font-size: 180%;" onclick="deleteItemBlock('itemblock[% item.itemBlockIndex %]')">-</a>

=> the - is not displayed by default (ie : when only 1 item ordered). It's displayed when clicking +, by the cloneItemBlock sub (in js/additem.js) .
This sub contains :
    CloneButtonMinus.setAttribute('onclick',"deleteItemBlock('" + index + random + "')");
    CloneButtonMinus.setAttribute('style',"display:inline");

without any specific condition.

an idea: is there a javascript error that appears when you click on the + ? (ie: a JS error could stop the js engine, just after duplicating the item)

Other idea: could it be a css problem (ie: do you have a css for staff ?)
Comment 7 Katrin Fischer 2011-11-01 09:13:30 UTC
Started retesting...

1) Testing on master, 'almost' fresh database, no customizations
2) AcqCreateItem is set to 'on order'
3) Started a new basket, added one order from suggestion
4) Clicked + 2 times, a total of 3 item forms showing on the page
5) No minus sign shows

Source code at the end of my third item form looks like this:
           <a style="display:none; cursor: pointer; color: grey; font-size: 180%;" onclick="deleteItemBlock('itemblock')">-</a>

Firebug shows me no javascript errors or warnings. 

I am using an ACQ framework and only working with the English templates.
Comment 8 Katrin Fischer 2011-11-01 09:20:49 UTC
Created attachment 6129 [details]
screenshot of firebug output

I played with the settings for firebug some more and it gave me some warnings.
Comment 9 Paul Poulain 2011-11-01 09:25:16 UTC
(In reply to comment #7)
> Started retesting...
> 
> 1) Testing on master, 'almost' fresh database, no customizations
> 2) AcqCreateItem is set to 'on order'
> 3) Started a new basket, added one order from suggestion
same thing when you add an order from another source ? (empty/existing/z3950/imported record ?)

> Source code at the end of my third item form looks like this:
>            <a style="display:none; cursor: pointer; color: grey; font-size:
> 180%;" onclick="deleteItemBlock('itemblock')">-</a>
This one is OK. The HTML don't show you what javascript can have done.

> Firebug shows me no javascript errors or warnings. 
that's very bad news...
Are you able to track if the cloneItemBlock is called by adding some
alert('I am here');
in some places in the code? It should show a popup everytime you click on the +

this is really strange...
Comment 10 Paul Poulain 2011-11-01 09:26:51 UTC
seen you new comment now... interesting. Please discard my previous comment.
The elem[] is probably related to ACQ framework, i'm investigating (not now, at the conference, but that's a strong candidate). It would not be related to MARC flavor but to framework setup.
Comment 11 Paul Poulain 2011-11-01 10:55:17 UTC
mmm... more and more strange...
Katrin, could you try to un-activte firebug and see if it's does not fix the problem ? I (a long time ago) saw firebug introducing a problem that did not exist without it.
I still can't reproduce your problem, and the "reference to undefined property fbs.breakOnDebugCall" seems suspicious to me...
Comment 12 Katrin Fischer 2011-11-01 11:06:13 UTC
Hi Paul, that's because I activated lots of things to give me at least some
warnings. It does not work with and without firebug. Others like Nicole have
been able to reproduce this problem. Perhaps check on a different installation?
Comment 13 Paul Poulain 2011-11-01 11:14:58 UTC
OK, bywater demo site has the problem as well :

http://intranet.bywatersolutions.com/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=3&basketno=4
(login/pwd: bywater/bywater)
Comment 14 Nicole C. Engard 2011-11-05 19:27:19 UTC
I've seen the issue on my install as well as several other MARC21 libraries - so it is still an issue for me as well.

Nicole
Comment 15 Katrin Fischer 2011-11-16 08:18:00 UTC
Paul, is there an update on this? It's an important feature that is broken right now.
Comment 16 Katrin Fischer 2011-12-05 05:55:00 UTC
Ok, I think I have the reason and a possible fix.

The reason: The function for cloning looks for the first and second <a> element in the <div> block of the item in order to change the parameter to include a unique number and make the minus link show. The problem here is, that you can use an undefined number of plugins within your ACQ framework now, that will create more <a> tags so looking for element number x will no longer work. (Plugins added: http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=e5ff05857e5b99ab19210bc272086f253ea9404b) 

But the good news is, now it all makes sense :) 

Paul couldn't reproduce this, because he had no plugins in his framework.
Comment 17 Katrin Fischer 2011-12-05 06:31:02 UTC Comment hidden (obsolete)
Comment 18 Owen Leonard 2011-12-05 15:36:05 UTC
Created attachment 6581 [details] [review]
Signed-off patch

Tested both test plans and found both to be working properly.
Comment 19 Paul Poulain 2011-12-05 21:43:48 UTC
Wonderfull Katrin ! you probably got it !
I'll QA this patch tomorrow, when waiting for my plane to go back home. It's a little bit late for now, heading to bed.
Comment 20 Paul Poulain 2011-12-06 18:09:27 UTC
QA comment : nothing specific to say. Clean patch, works as announced, no side effect I can see.

patch pushed, please test

cait, this one is worth a special award !