Bug 7435 - An inactive fund is selected a default in neworderempty.pl
Summary: An inactive fund is selected a default in neworderempty.pl
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) minor (vote)
Assignee: Adrien SAURAT
QA Contact: Ian Walls
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-11 09:36 UTC by Adrien SAURAT
Modified: 2013-12-05 19:59 UTC (History)
3 users (show)

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


Attachments
proposed patch (1.59 KB, patch)
2012-01-11 15:31 UTC, Adrien SAURAT
Details | Diff | Splinter Review
Bug 7435: corrects Fund selectbox in addneworder (1.65 KB, patch)
2012-01-13 06:50 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Adrien SAURAT 2012-01-11 09:36:01 UTC
When making a new order in the neworderempty page ( Home › Acquisitions › *budget* › Basket X › New order ), in some cases the "Fund" select box display a fund which should be hidden.


Here is an excerpt from the HTML code of a problematic case:

<select id="budget_id" onchange="fetchSortDropbox(this.form)" size="1" name="budget_id">
 <option value="1" class="b_inactive" style="display : none;">fond test A1</option> 
 <option value="2" class="b_inactive" style="display : none;">fond test A2</option> 
 <option value="3" class="b_inactive" style="display : none;">test fond B1</option> 
 <option value="4" class="b_inactive" style="display : none;">test fond A0 à rendre inactif</option> 
 <option value="5">poste 3a</option>
 <option value="6">poste 3b</option>
 <option value="7">poste budgetaire #2-A</option>
</select>

In this case, "fond test A1" should be hidden, but it's actually displayed as the default choice. The CSS hiding is not functionning properly.
Comment 1 Adrien SAURAT 2012-01-11 15:13:59 UTC
Test plan :

1) Create different budgets, at least 2, both being Activated.

2) In each budget, create at least one Fund.

3) When you select a vendor and order a new book, you see all your funds in the "Fund" select box.

4) Now edit the 1st budget (containing the funds which appear on top in the select box) and deactivate it.

5) Go back to the book ordering, the Fund select box still displays the 1st fund of the 1st budget, even though it's supposed to be not active and thus hidden.
Comment 2 Adrien SAURAT 2012-01-11 15:31:29 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2012-01-13 06:50:42 UTC
Created attachment 7125 [details] [review]
Bug 7435: corrects Fund selectbox in addneworder

Prevent disabled funds to appear in the Fund
select box.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 4 Fred P 2012-01-16 17:04:40 UTC
Displaying the inactive funds in red interferes with the display:none setting. The problem can be solved by option-grouping the selection list. A secondary problem also exists that the Show All checkbox can display the full list, but not un-display it. The javascript is buggy and prevents re-hiding the inactive items. Here is an effective patch:

-----------------------------------------------------------
File: neworderempty.pl
Near Line 263:
-----------------------------------------------------------
my $budget_loop = [];
my $inactive_budget_loop = [];
my $budgets = GetBudgetHierarchy(q{},$borrower->{branchcode},$borrower->{borrowernumber});
foreach my $r (@{$budgets}) {
   if (!defined $r->{budget_amount} || $r->{budget_amount} == 0) {
       next;
   }
if($r->{budget_period_active}){
   push @{$budget_loop}, {
       b_id  => $r->{budget_id},
       b_txt => $r->{budget_name},
       b_active => $r->{budget_period_active},
       b_sel => ( $r->{budget_id} == $budget_id ) ? 1 : 0,
   };

}else{
   push @{$inactive_budget_loop}, {
       b_id  => $r->{budget_id},
       b_txt => $r->{budget_name},
       b_active => $r->{budget_period_active},
       b_sel => ( $r->{budget_id} == $budget_id ) ? 1 : 0,
   };
}
}


if ($close) {
   $budget_id      =  $data->{'budget_id'};
   $budget_name    =   $budget->{'budget_name'};

}
-----------------------------------------------------------
Near Line 392:
-----------------------------------------------------------

   budget_loop      => $budget_loop,
   inactive_budget_loop => $inactive_budget_loop,
   isbn             => $data->{'isbn'},

-----------------------------------------------------------
THAT FIXES THE JAVASCRIPT for the Checkbox "Show All"
Next we create the Option Groups in the Select Drop-Down:
-----------------------------------------------------------
File: neworderempty.tt
Near Line 95:
-----------------------------------------------------------

       $('#showallbudgets').click(function() {
           if ( $('#inactive_funds .b_inactive').is(":visible") )
           {
   $('#inactive_funds').attr("disabled","disabled");
           $('#inactive_funds .b_inactive').hide();

           }
           else {
   $('#inactive_funds').attr("disabled","");
           $('#inactive_funds .b_inactive').show();

           }
       });

-----------------------------------------------------------
Near Line 333:
-----------------------------------------------------------

               <label for="budget_id">Fund: </label>
               <select id="budget_id" onchange="fetchSortDropbox(this.form)" size="1" name="budget_id">
<optgroup label="active funds">
[% IF !(budget_id) %]<option value='0' >Select Fund</option> [% END %]
               [% FOREACH budget_loo IN budget_loop %]

                   [% IF ( budget_loo.b_sel ) %]
                       <option value="[% budget_loo.b_id %]" selected="selected">[% budget_loo.b_txt %]</option>
                   [% ELSE %]
                       [% IF ( budget_loo.b_active ) %]<option value="[% budget_loo.b_id %]">[% budget_loo.b_txt %]</option>
                       [% ELSE %]<option value="[% budget_loo.b_id %]" class="b_inactive" style="display : none; color:red;" >[% budget_loo.b_txt %]</option>  

                       [% END %]
                   [% END %]
               [% END %]
</optgroup>

<optgroup label="inactive funds" id="inactive_funds" disabled="disabled">
               [% FOREACH inactive_budget_loo IN inactive_budget_loop %]

                   [% IF ( inactive_budget_loo.b_sel ) %]
                       <option value="[% inactive_budget_loo.b_id %]" selected="selected">[% inactive_budget_loo.b_txt %]</option>
                   [% ELSE %]
                       [% IF ( inactive_budget_loo.b_active ) %]<option value="[% inactive_budget_loo.b_id %]">[% inactive_budget_loo.b_txt %]</option>
                       [% ELSE %]<option value="[% inactive_budget_loo.b_id %]" class="b_inactive" style="display : none;" >[% inactive_budget_loo.b_txt %]</option>  

                       [% END %]
                   [% END %]
               [% END %]
</optgroup>
               </select>
----------------------------------------------------------------
On my system, Chrome displays only two lines of the drop-down, 
both before and after the fix, so I use Firefox for testing this.
Comment 5 Fred P 2012-01-16 17:09:44 UTC
Because you might still want to edit an invoice from 2011, even though the fund is inactive.
Comment 6 Adrien SAURAT 2012-01-17 08:27:14 UTC
The inactive funds are not supposed to appear in red! Did they do so when you tested this?
I just made red the "Fund:" label before the selectbox.

When I test my patch the "Show All" checkbox works correctly (when showing all and hiding inactive funds again). That's for Firefox 9.

Testing with Chromium (linux version of Chrome), showing all works but then, unchecking the box to hide again won't do anything, but it was the same before the patch I sent.


I agree that it would be a good occasion to repair a browser-specific code, but does your patch work with Chromium? What's with this "Chrome displays only two lines of the drop-down" problem? Sounds strange... Is the koha javascript guilty or is it a bug of this browser?
Comment 7 Fred P 2012-01-17 14:40:21 UTC
Excuse me for jumping in without fully testing your patch in Firefox. Chrome does have some glitchy behavior, but it's what I test on first. Your patch does work well for Firefox. Thank you for your work.

I mentioned the red text only because of the original post. Our staff asked for the inactive funds to be red to distinguish them from the active funds and red text interferes with the css code, "display:none" used to hide the inactive funds options. Your use of the label "required" class works well.

We modified the javascript such that it does rehide the inactive items in Chrome, so feel free to use that if the patch gets future modifications. 

I think the other consideration is that we are using Koha 3.5, and not up-to-date 3.6, so some changes have already been made prior to our suggestion. I don't have access to the up-to-date javascript in neworderempty.tt, so perhaps the issues with rehiding are already solved. Apologies for not testing your script before commenting - we were very excited that we got it working for our Tech Services.
Comment 8 Paul Poulain 2012-01-17 17:22:41 UTC
QA comment: good test plan, small patch, passed QA
Comment 9 Jared Camins-Esakov 2012-05-23 12:54:03 UTC
This fix was included in the 3.6.x branch prior to 3.6.4.