Bug 9088 - neworderempty should preselect the only active fund for new orders
Summary: neworderempty should preselect the only active fund for new orders
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Marcel de Rooy
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-15 13:08 UTC by Marcel de Rooy
Modified: 2015-06-04 23:33 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
9088 Patch (2.90 KB, patch)
2012-11-15 13:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
9088 Dbrev (2.39 KB, patch)
2012-11-15 13:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
9088 Default values for fund and item type on neworderempty form (2.90 KB, patch)
2013-01-30 15:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
9088: Default values for fund and item type on neworderempty form (dbrev) (2.29 KB, patch)
2013-01-30 15:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9088: neworderempty should preselect the only fund (1.82 KB, patch)
2014-04-03 12:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[SIGNED OFF] Bug 9088: neworderempty should preselect the only fund (2.15 KB, patch)
2014-04-14 04:18 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 9088: Neworderempty should preselect the only active fund (4.09 KB, patch)
2014-04-16 12:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9088: Neworderempty should preselect the only active fund (4.10 KB, patch)
2014-04-16 12:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9088: Neworderempty should preselect the only active fund (4.11 KB, patch)
2014-04-28 09:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9088: Neworderempty should preselect the only active fund (4.46 KB, patch)
2014-05-08 06:40 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9088: Neworderempty should preselect the only active fund (4.67 KB, patch)
2014-05-09 15:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[PASSED QA] Bug 9088: Neworderempty should preselect the only active fund (4.70 KB, patch)
2014-05-16 11:08 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2012-11-15 13:08:10 UTC
Our users really would like to have a default value for fund and item type when entering a new order instead of selecting them over and over again.

The proposed patch needs one syspref containing default codes for such fields.
Comment 1 Marcel de Rooy 2012-11-15 13:29:19 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2012-11-15 13:30:57 UTC Comment hidden (obsolete)
Comment 3 Melia Meggs 2012-11-16 20:13:54 UTC
I applied this patch.  

Before the patch, when doing a new order, I have to select the item type and fund.

After the patch, I set the AcqNewOrderEmptyDefaults system preference to fund=BOOKS.  When I did a new order, I found that the BOOKS fund was correctly filled in by default and can still be changed if necessary using the dropdown menu of funds.

However, I can't get it to work for an item type.  I set AcqCreateItem to create an item when placing an order.  I tried placing itemtype=BOOK in the AcqNewOrderEmptyDefaults system preference, but the y - Koha item type box is still blank by default, which means I have to choose the item type from the dropdown menu.
Comment 4 Marcel de Rooy 2012-11-19 09:07:41 UTC
(In reply to comment #3)
> However, I can't get it to work for an item type.  I set AcqCreateItem to
> create an item when placing an order.  I tried placing itemtype=BOOK in the
> AcqNewOrderEmptyDefaults system preference, but the y - Koha item type box
> is still blank by default, which means I have to choose the item type from
> the dropdown menu.

Thanks for testing. I see what you mean. Note that the itemtype is filled in under Catalog details, but indeed it is not under Item. This behavior is only when AcqCreateItem=placing an order. If you set AcqCreateItem to receiving or cataloging, you will have no problems.

As I see it now, the behavior under "placing an order" is another bug. Without this patch, if you enter an itemtype under Catalog details, it is ignored in the item section below it. This makes the field Itemtype under Catalog details actually useless.

While searching Bugzilla, I also found a related report for "placing an order" with default values. That is bug 8307. Possibly, we can bring the functionality of these two reports together somehow.
Comment 5 Marcel de Rooy 2013-01-30 15:30:12 UTC Comment hidden (obsolete)
Comment 6 Marcel de Rooy 2013-01-30 15:30:26 UTC Comment hidden (obsolete)
Comment 7 Owen Leonard 2013-03-19 00:33:48 UTC
I think I'm ending up with the same questions as Melia. I see that the fund pre-select works, but I don't see how this ties in with default item type. I think what this bug really needs is a thorough test plan, otherwise we're just guessing.
Comment 8 Marcel de Rooy 2014-04-03 12:52:42 UTC Comment hidden (obsolete)
Comment 9 Marcel de Rooy 2014-04-03 12:55:08 UTC
I decided to simplify the approach on this report.
There is no preference anymore. And we ignore itemtype now.
It is just: If there is only one fund, select it rightaway.
Comment 10 Katrin Fischer 2014-04-04 05:54:25 UTC
Just wondering - couldn't you set the default itemtype in the acq framework? I think I have set that up for some libraries in the past.
Comment 11 Marcel de Rooy 2014-04-04 12:27:13 UTC
(In reply to Katrin Fischer from comment #10)
> Just wondering - couldn't you set the default itemtype in the acq framework?
> I think I have set that up for some libraries in the past.

Probably. But this report now focuses on the fund. Thanks.
Comment 12 M. Tompsett 2014-04-14 04:18:56 UTC Comment hidden (obsolete)
Comment 13 Katrin Fischer 2014-04-14 22:48:26 UTC
Hi Marcel, this doesn't seem to work for me. I have older inactive budgets, could this make the difference? The pull down correctly only shows one entry unless you tick the 'show all'.
Comment 14 Marcel de Rooy 2014-04-15 13:57:37 UTC
(In reply to Katrin Fischer from comment #13)
> Hi Marcel, this doesn't seem to work for me. I have older inactive budgets,
> could this make the difference? The pull down correctly only shows one entry
> unless you tick the 'show all'.

Yes, this makes a difference. I can adjust the patch somewhat to account for that.
But looking at it, I wonder: Suppose you have three funds (F1 is active, F2 and F3 are inactive). And an order is saved with F2.
If you click Modify, what should you want neworderempty to do? Select F1, since it is the only active fund now? Or select F2, but mark it as inactive? And if it shows F2, should it also show F3 now right away and preselect Show all?
Comment 15 Katrin Fischer 2014-04-15 20:23:56 UTC
Hi Marcel, 

> Yes, this makes a difference. I can adjust the patch somewhat to account for
> that.
> But looking at it, I wonder: Suppose you have three funds (F1 is active, F2
> and F3 are inactive). And an order is saved with F2.
> If you click Modify, what should you want neworderempty to do? Select F1,
> since it is the only active fund now? Or select F2, but mark it as inactive?
> And if it shows F2, should it also show F3 now right away and preselect Show
> all?

Hm, die we uncover another bug here? It seems one of those days...

I think when modifying an existing order, it should always preselect what's in the database, even if the fund has been marked inactive since or was inactive when selected. If we need to preselect 'show all' in order to make that work, I guess we need to do that.
Comment 16 Marcel de Rooy 2014-04-16 12:14:46 UTC
(In reply to Katrin Fischer from comment #15)
> Hm, die we uncover another bug here? It seems one of those days...
> 
> I think when modifying an existing order, it should always preselect what's
> in the database, even if the fund has been marked inactive since or was
> inactive when selected. If we need to preselect 'show all' in order to make
> that work, I guess we need to do that.

I was thinking in the same direction. New patch forthcoming (need to adjust a few lines more..) No need to preselect Show all btw.
Comment 17 Marcel de Rooy 2014-04-16 12:15:20 UTC Comment hidden (obsolete)
Comment 18 Marcel de Rooy 2014-04-16 12:26:03 UTC Comment hidden (obsolete)
Comment 19 Marcel de Rooy 2014-04-16 12:26:45 UTC
Couldn't resist adjusting indentation in the commit message too :)
Comment 20 M. Tompsett 2014-04-26 16:26:02 UTC
Comment on attachment 27191 [details] [review]
Bug 9088: Neworderempty should preselect the only active fund

Review of attachment 27191 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
@@ +425,5 @@
> +                    [% FOREACH budget_loo IN budget_loop %]
> +                        <option value="[% budget_loo.b_id %]"
> +                        [% IF budget_loo.b_sel %]
> +                            [% active_count = 0 #select no other fund %]
> +                            selected="selected"

Sorry, but this violates HTML1 in the coding guidelines.
http://wiki.koha-community.org/wiki/Coding_Guidelines#HTML1:_Template_Toolkit_markup_inside_HTML
Comment 21 M. Tompsett 2014-04-26 16:26:37 UTC
Sorry, but guidelines should be followed. :(
Comment 22 Marcel de Rooy 2014-04-28 09:36:47 UTC
(In reply to M. Tompsett from comment #21)
> Sorry, but guidelines should be followed. :(

At least in this case.
Have to repeat the option tag now a few times in the if-then-else of the new patch. Slightly less readable. (Also stumbled about the selected attribute; too bad you can't say selected="" when you do not want to select..)
Comment 23 Marcel de Rooy 2014-04-28 09:37:31 UTC Comment hidden (obsolete)
Comment 24 Marcel de Rooy 2014-05-08 06:40:46 UTC Comment hidden (obsolete)
Comment 25 Paola Rossi 2014-05-09 14:48:32 UTC
I test against master 3.15.00.051

I test against all the three options of the AcqCreateItem, :
placing an order
receiving an order
cataloging the record

All is OK.

I pass the patch to "Signed Off" status.
Comment 26 Marcel de Rooy 2014-05-09 15:05:57 UTC Comment hidden (obsolete)
Comment 27 Marcel de Rooy 2014-05-09 15:06:27 UTC
(In reply to Paola Rossi from comment #25)
> I test against master 3.15.00.051
> 
> I test against all the three options of the AcqCreateItem, :
> placing an order
> receiving an order
> cataloging the record
> 
> All is OK.
> 
> I pass the patch to "Signed Off" status.

Thanks, Paola. I added your signoff line to the patch.
Comment 28 Kyle M Hall 2014-05-16 11:08:50 UTC
Created attachment 28291 [details] [review]
[PASSED QA] Bug 9088: Neworderempty should preselect the only active fund

This patch makes the following changes to the template:

[1] If you add an order line, and you have one active fund (and zero or
    more inactive funds), the only active fund is preselected.
[2] If you modify an order line while its fund is inactive, it now shows
    the label (inactive) behind the fund name. (Note that other inactive funds
    may come up when clicking 'Show all' as they did before.)
[3] Corrected some indentation in this template part.

Test plan:

[1] Add an order line while having one active fund. Is it selected?
[2] Add an order line while having two or more active funds. No fund
    should be preselected.
[3] Modify an order line with an active fund. Is it still selected?
[4] Modify an order line with an inactive fund F2 (while having one active
    fund F1; note that this test explicitly wants F1 to be before F2).
    Check if F2 is selected and is labeled inactive.
[5] (Bonus points:) Modify an order line that refers to a deleted fund.
    If you edit this order, the fund combo should say: Select a fund.
    (Note: if you delete a fund, the budget_id in aqorders remains.)

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
I test against master 3.15.00.051
I test against all the three options of the AcqCreateItem:
placing an order
receiving an order
cataloging the record
All is OK.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 29 Galen Charlton 2014-05-25 15:15:45 UTC
Pushed to master.  Thanks, Marcel!