Bug 11287 - Import borrowers doesn't take into account the new restriction system
Summary: Import borrowers doesn't take into account the new restriction system
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on: 2720
Blocks:
  Show dependency treegraph
 
Reported: 2013-11-22 14:15 UTC by Kyle M Hall
Modified: 2014-12-07 20:07 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 11287 - Import borrowers doesn't take into account the new debarment system (3.62 KB, patch)
2013-11-22 14:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11287 - Import borrowers doesn't take into account the new debarment system (4.13 KB, patch)
2013-11-22 14:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11287 - Import borrowers doesn't take into account the new debarment system (4.22 KB, patch)
2013-12-22 21:09 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 11287 - Import borrowers doesn't take into account the new debarment system (4.29 KB, patch)
2013-12-23 15:44 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2013-11-22 14:15:17 UTC
The import patrons tool will add restrictions in the previous style ( directly to the borrowers table ). This will actually work, but will result in the restriction being un-removable, and will be overwritten by any new restrictions.
Comment 1 Kyle M Hall 2013-11-22 14:17:59 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2013-11-22 14:20:50 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2013-12-22 21:09:39 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2013-12-23 15:44:07 UTC
Created attachment 23788 [details] [review]
Bug 11287 - Import borrowers doesn't take into account the new debarment system

The import patrons tool will add restrictions in the previous style (
directly to the borrowers table ). This will actually work, but
will result in the restriction being un-removable, and will be
overwritten by any new restrictions.

Test Plan:
1) Apply this patch
2) Import new patrons with debarrments, note the debarment is created
   correctly
3) Import existing patrons with overwrite enabled, note that any new
   debarrment is added, and any existing debarrment is ignored

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Works as expected, and fixes the POD too

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 5 Galen Charlton 2013-12-26 15:50:29 UTC
This probably won't affect my decision to push the patch, but one thing that occurs to me: even with the patch, there is overall a regression from 3.12.  Specifically, prior to the new restriction system, a patron file could be used both to set and remove debarments.

One example where one might want to remove debarments via patron import: imagine an academic library that receives patron updates frequently.  If the patron owes money to the university, the bursar's office might want to be able to set a debarment to suspend the patron's library privileges.  Once they settle their account with the bursar, the next patron import could lift the restriction.

General question: are there Koha users out there who actually do this?
Comment 6 Katrin Fischer 2013-12-26 16:05:06 UTC
If you can't unset/delete a restriction on import with the new system we will be hurt by this. We have one library where we receive a complete file of all students for importing. Prior to the import we set a restriction for all students, that is then lifted for all students in the file. Students that have left/not paid for the next semester will stay restricted.
Comment 7 Galen Charlton 2013-12-26 16:17:56 UTC
Thinking aloud, I wonder of adding a borrower_debarments.type value of "PATRON_IMPORT" that could be set and removed by the patron importer is the way to go.
Comment 8 Katrin Fischer 2013-12-26 16:27:26 UTC
I think this could work. 
Another idea: could we use the description in combination with the debarment date? 
Set restriction: 'some description' '2014-02-28'
Unset restriction: 'some description' ''
Comment 9 Katrin Fischer 2013-12-26 16:31:53 UTC
Hm I think the advantage of Galen's idea would be that the existing format would not have to be changed.
Comment 10 Galen Charlton 2014-05-05 22:37:44 UTC
Doesn't solve the whole problem, but since it's better than the status quo, I've pushed it.  Thanks, Kyle!
Comment 11 Katrin Fischer 2014-05-18 10:45:21 UTC
Should I file a new bug for the remaining regression? Being able to lift debarments via the patron import is important for some workflows.