Bug 6842 - Branch transfer limits broken
Summary: Branch transfer limits broken
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: 3.6
Hardware: All All
: P2 major (vote)
Assignee: Paul Poulain
QA Contact: Bugs List
URL: /admin/branch_transfer_limits.pl?bran...
Whiteboard:
Keywords:
Depends on: 6836
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-02 20:12 UTC by Katrin Fischer
Modified: 2013-12-05 19:57 UTC (History)
4 users (show)

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


Attachments
Proposed fix (2.87 KB, patch)
2011-09-15 14:25 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Fix for Bug 6842 - Branch transfer limits broken (3.09 KB, patch)
2011-09-20 06:34 UTC, Katrin Fischer
Details | Diff | Splinter Review
patch proposed. (2.54 KB, patch)
2011-12-22 13:44 UTC, laurenthdl
Details | Diff | Splinter Review
followup patch : assigns a problem of deleted information when editing an other branch (1.71 KB, patch)
2011-12-22 16:15 UTC, laurenthdl
Details | Diff | Splinter Review
Alternate fix which doesn't require Datatables plugin (3.17 KB, patch)
2011-12-28 16:47 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6842 Alternate fix, Branch transfer limits broken (3.35 KB, patch)
2011-12-29 20:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] followup patch : assigns a problem of deleted information when editing an other branch (1.81 KB, patch)
2012-02-07 15:31 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 6842 Alternate fix, Branch transfer limits broken (3.39 KB, patch)
2012-02-07 16:13 UTC, Ian Walls
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2011-09-02 20:12:42 UTC
1) Select of library broken

When selecting a library and clicking 'choose' the page reloads but the pull down jumps back to the first branch in the list

2) Can't save changes

If you change the checkboxed for the selected library and click 'save' nothing happens. Page does not reload, there is no message displayed and changes are not saved.
Comment 1 Owen Leonard 2011-09-15 14:25:16 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2011-09-20 06:34:41 UTC Comment hidden (obsolete)
Comment 3 Ian Walls 2011-09-20 11:52:05 UTC
Mainly a template change, with one additional param in the Perl.  Easy to read, correct HTML nesting, marking as Passed QA
Comment 4 Chris Cormack 2011-09-20 21:41:20 UTC
Pushed please test
Comment 5 Katrin Fischer 2011-12-18 20:07:29 UTC
I tested this on current master and can't save changes to branch transfer limists. Clicking on 'save' gives no reaction.
Comment 6 laurenthdl 2011-12-22 13:44:48 UTC Comment hidden (obsolete)
Comment 7 laurenthdl 2011-12-22 16:15:33 UTC Comment hidden (obsolete)
Comment 8 Owen Leonard 2011-12-28 16:47:36 UTC Comment hidden (obsolete)
Comment 9 Katrin Fischer 2011-12-29 20:43:26 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2011-12-29 20:50:59 UTC
I had no problems deleting branch transfer rules, so I am not sure how to test the second patch attached to this bug. Moving this to signed off for now.
Comment 11 Paul Poulain 2012-02-01 21:50:32 UTC
Katrin: about the 2nd patch, here is the test case:
* select a branch
* unselect some checkboxes to prevent transfer
* save
* change your branch
* unselect some checkboxes and save
* go back to the 1st branch
=> everything is checked again !
This is because the SQL delete everything and not only the branch rules you're saving !
I've tested the patch, please test it too, so I can QA & push both patches

I'll obsolet the 1st patch
Comment 12 Paul Poulain 2012-02-01 21:50:51 UTC
Comment on attachment 6923 [details] [review]
patch proposed.

obsoleted by owen patch
Comment 13 Owen Leonard 2012-02-07 15:31:36 UTC
Created attachment 7477 [details] [review]
[SIGNED-OFF] followup patch : assigns a problem of deleted information when editing an other branch

I see now why the follow-up patch is required. If you make a change to a one branch the correct changes are saved. If you make a change to another branch after that the changes to the first branch are lost. The patch fixes that problem.
Comment 14 Ian Walls 2012-02-07 16:11:33 UTC
Initial patch:  template only change, replacing <form> with <p>, and updating the necessary CSS.  Unfortunately, it does not apply to current master.  Rebased patch forthcoming.

Followup patch:  alters the subroutine DeleteBranchTransferLimits() to take a branchcode param.  Subsequently alters ALL usages of this subroutine in Koha (only one) to make use of this new param.  Marking Passed QA.
Comment 15 Ian Walls 2012-02-07 16:13:54 UTC
Created attachment 7484 [details] [review]
Bug 6842 Alternate fix, Branch transfer limits broken

The patch which moved the table pager markup into an
include undid the fix which was previously pushed for
this problem. Re-implementing the fix in the pager
include is a simpler fix which doesn't depend on
the Datatables plugin.

The markup change in table-pager.inc fixes the problem
in branch transfer limits and does not affect functionality
on other pages.

The changes in this patch to branch_transfer_limits.tt and
staff-global.css are for cosmetic reasons only.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Saving changes works now correctly. I tried:
- uncheck all for a branch
- check all for a branch
- making changes for different itemtypes on different tabs
Comment 16 Ian Walls 2012-02-07 16:15:16 UTC
CSS conflict from local cover images development.  Rebased now, marking both patches here as Passed QA.
Comment 17 Jared Camins-Esakov 2012-05-23 13:00:09 UTC
This fix was included in the 3.6.x branch prior to 3.6.4.