Bug 11027 - add a column for basketgroups in late orders table
Summary: add a column for basketgroups in late orders table
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Mathieu Saby
QA Contact: Testopia
URL: 7942
Keywords:
: 11434 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-10-09 17:49 UTC by Mathieu Saby
Modified: 2015-06-04 23:23 UTC (History)
5 users (show)

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


Attachments
PATCH] Bug 11027 : Add a column for basketgroups in late orders table and improve other columns (6.21 KB, patch)
2013-10-10 16:44 UTC, Mathieu Saby
Details | Diff | Splinter Review
[PATCH] Bug 11027 : Add a column for basketgroups in late orders table and improve other columns (4.71 KB, patch)
2013-12-18 21:14 UTC, Mathieu Saby
Details | Diff | Splinter Review
[PATCH 2/2] Bug 11027 - Create a link only if user have required permissions (2.36 KB, patch)
2013-12-22 21:53 UTC, Mathieu Saby
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11027 : Add a column for basketgroups in late orders table and improve other columns (4.73 KB, patch)
2014-01-28 17:35 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11027 : Add a column for basketgroups in late orders table and improve other columns (4.73 KB, patch)
2014-01-28 17:37 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11027 - Create a link only if user have required permissions (2.37 KB, patch)
2014-01-28 17:37 UTC, Mark Tompsett
Details | Diff | Splinter Review
[PATCH] Bug 11027 (QA) : hide link if no basketgroup and use real branch name (2.47 KB, patch)
2014-02-02 22:14 UTC, Mathieu Saby
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11027 (QA) : hide link if no basketgroup and use real branch name (2.49 KB, patch)
2014-02-03 03:37 UTC, Mark Tompsett
Details | Diff | Splinter Review
[PASSED QA] Bug 11027 : Add a column for basketgroups in late orders table and improve other columns (4.94 KB, patch)
2014-02-03 19:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11027 - Create a link only if user have required permissions (2.43 KB, patch)
2014-02-03 19:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11027 (QA) : hide link if no basketgroup and use real branch name (2.55 KB, patch)
2014-02-03 19:46 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mathieu Saby 2013-10-09 17:49:30 UTC
In lateorders.pl it could be usefull to add a new column for basket groups.

M. Saby
Comment 1 Mathieu Saby 2013-10-10 16:44:55 UTC Comment hidden (obsolete)
Comment 2 I'm just a bot 2013-10-30 07:07:25 UTC
Applying: Bug 7298 Export late orders as csv
Using index info to reconstruct a base tree...
M	C4/Acquisition.pm
M	acqui/lateorders.pl
M	koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt
M	serials/lateissues-excel.pl
Falling back to patching base and 3-way merge...
Auto-merging serials/lateissues-export.pl
CONFLICT (content): Merge conflict in serials/lateissues-export.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt
Auto-merging acqui/lateorders.pl
Auto-merging C4/Acquisition.pm
Patch failed at 0001 Bug 7298 Export late orders as csv
The copy of the patch that failed is found in:
   /home/christopher/git/koha/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Comment 3 I'm just a bot 2013-10-30 07:07:39 UTC
This bug depends on bug7298 which is in status Passed QA but the patches for it do not apply cleanly
Comment 4 Mathieu Saby 2013-11-03 18:27:16 UTC
Now it seems to apply cleanly

Mathieu
Comment 5 Mathieu Saby 2013-12-18 21:14:45 UTC Comment hidden (obsolete)
Comment 6 Mathieu Saby 2013-12-18 21:16:30 UTC
This new patch adds a link opening the basketgroup information page (possible now that bug 9807 is pushed. Before it was done, linking to a closed basketgroup was reopening it...)

Mathieu
Comment 7 Mathieu Saby 2013-12-22 21:47:45 UTC
My patch does not take into account the permission for managing groups...
Comment 8 Mathieu Saby 2013-12-22 21:53:53 UTC Comment hidden (obsolete)
Comment 9 Mark Tompsett 2014-01-27 20:43:40 UTC
With bootstrap being the default, could we get the equivalent bootstrap tweaks in this too?
Comment 10 Owen Leonard 2014-01-28 13:00:21 UTC
> With bootstrap being the default, could we get the equivalent bootstrap
> tweaks in this too?

These patches don't touch the OPAC.
Comment 11 Mark Tompsett 2014-01-28 17:35:50 UTC Comment hidden (obsolete)
Comment 12 Mark Tompsett 2014-01-28 17:37:00 UTC Comment hidden (obsolete)
Comment 13 Mark Tompsett 2014-01-28 17:37:11 UTC Comment hidden (obsolete)
Comment 14 Mark Tompsett 2014-01-28 17:39:00 UTC
REVISED TEST PLAN
-----------------
 0) Back up DB if you want to restore it later. Also, apply patch. :)
 1) Log into staff client as superlibrarian
 2) Click Acquisitions
 3) Click the Search button in the Manage orders section
 4) Click + New basket for any one of the vendors
 5) Type a Basket name and click Save
 6) Click a link in the Add order to basket
    -- I used suggestions, since we have 9.
    -- don't forget to click "Add Item"
       and choose a Fund before "Save"
 7) Click Change button beside Branch dropdown
     to set up the branch.
    -- I don't know why this seems necessary, but
       I couldn't get it to work otherwise.
 8) Click Close the basket
 9) Check Attach this basket to a new basket group with the same name
10) Click Yes, close (Y)
11) Click Late orders
    -- the order just made should be listed.
    -- both the Basket and Basket group columns should be links
12) Add a patron with the following permissions:
    - check staff access
    - check acquisition
    - then uncheck group_manage
    - then uncheck order_manage
    Make sure to match the branch to the superlibrarian's
    branch, because that is what the entered late order uses
    as its branch.
13) log out of the staff client
14) log into the staff client with the new user
    -- you'll get an
       "Error: You do not have permission to access
        this page"
15) paste and correct the url:
    http://YOUR_SITE/cgi-bin/koha/acqui/acqui-home.pl
16) Click Late orders
    -- the orders listed will not have links in Basket and
       Basket group columns.
Comment 15 Katrin Fischer 2014-02-02 14:14:08 UTC
Hi Mathieu,

one small thing to fix:
- When an order is not in a basket group, it displays empty parenthesis "()" and
  the link does not work. Please add a check.

I also noticed that we display the branchcode instead of the branch name here - that could be easily fixed now using the TT plugin. If you want to you could add a follow up, but it can also be a separate bug.
Comment 16 Mathieu Saby 2014-02-02 22:14:52 UTC Comment hidden (obsolete)
Comment 17 Mathieu Saby 2014-02-02 22:15:51 UTC
Katrin, I think you should be fine with the last patch.

Mathieu
Comment 18 Mark Tompsett 2014-02-03 03:36:35 UTC
Ran a variation on my test plan, and the branch name shows up, and there are no ()'s in the columns now.
Comment 19 Mark Tompsett 2014-02-03 03:37:34 UTC Comment hidden (obsolete)
Comment 20 Mark Tompsett 2014-02-03 06:45:50 UTC
*** Bug 11434 has been marked as a duplicate of this bug. ***
Comment 21 Katrin Fischer 2014-02-03 19:46:35 UTC
Created attachment 25012 [details] [review]
[PASSED QA] Bug 11027 : Add a column for basketgroups in late orders table and improve other columns

This patch makes 4 changes in late orders page :
- adding a new column for basketgroup, displaying
  the name and number of the basketgroup
- displaying the name of the basket as well as its
  number, with a hyperlink to this basketgroup
- displaying fund in a separate column to make
  sorting easier, and renaming it from "budget" to
  "fund" (the confusion can be found elsewhere in Koha...)
- displaying branch in a separate column to make
  sorting easier (this column could be improved later,
  by getting branch from basketgroup if there is one)

For that, in Aquisition.pm, I made some changes to GetLateOrders:
- 3 new columns in SELECT
- 1 new join

To test :
1. Display the list of late orders of your instance.
2. Check you have 3 new columns for basketgroups, fund and branch
3. In basketgroup column you should have "name of basket group (number)"
4. In basket column you should have "name of basket (number)"
5. Check the links for basketgroups. They should send you to the
   page displaying information for each basketgroup
   (without possibiliy of editing them if they are closed)

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Patches pass QA script and all tests.
Works as described. Also checked sorting and orders which
are not in a basketgroup display correctly.
Comment 22 Katrin Fischer 2014-02-03 19:46:48 UTC
Created attachment 25013 [details] [review]
[PASSED QA] Bug 11027 - Create a link only if user have required permissions

With this followup, a user without order_manage permission won't be able to cick on a basket
and a user without group_manage permission won't be able to click on a basketgroup

Note that the issue for basket is present in current master. So it deserves a separate bugfix (as bug 11027 is an ENH)

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 23 Katrin Fischer 2014-02-03 19:46:59 UTC
Created attachment 25014 [details] [review]
[PASSED QA] Bug 11027 (QA) : hide link if no basketgroup and use real branch name

This patch is a answer to remarks made by QA:
- if there is no basketgroup for an order, the basketgroup
  column in lastorders.pl is now blank (instead of parentheses)
- the name of the branch is used instead of its code in the
  branch column

To test: check late orders, included and not included into basketgroup

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 24 Galen Charlton 2014-04-21 04:40:42 UTC
Pushed to master.  Thanks, Mathieu!
Comment 25 Fridolin Somers 2014-09-10 14:50:06 UTC
I choose to not apply to 3.14.x so I set as resolved.