Bug 14368

I get the part about having discharge history. But why would an user be 'discharged' several times? Is that not counter intuitive? Could you elaborate with an use-case where there could be multiple "discharge"?

Academic libraries in India particularly require discharge management feature. A multiple discharge of the same patron will create confusion and major patron management headaches (fees, fines, access card issuance etc).

Setting this to IN DISCUSSION

Please mention some case where this kind of feature is needed and it could not be manage by temporarily block feature?

Signed off based on discussions referenced at http://irc.koha-community.org/koha/2015-06-16#i_1688844

The indentation should be 4 spaces, please resubmit.

Submitted with 4 spaces identation.

It makes sense to have more than one discharge in a patron's lifetime (requested when ending studies, and then if he leaves the institution as a professor...).

I tested the feature and it showed more information about the discharge, but I doesn't seem to be working as expected as only the first discharge is being saved...

Matts, a few notes on this patch.

- I like the idea, I think we should've had this for the 3.20 release already.
- There's a bug on the unit test you introduce for lifting the discharge, because it should look like:

    ok( ! Koha::Borrower::Discharge::is_discharged({ borrowernumber => $borrowernumber }),
        'The patron is not discharged after the restriction has been lifted' );

And you will notice it fails :/

- There's a problem with Koha::Borrower::Discharge::discharge (not introduced by your patch, but affects the beaviour): it overwrites the already defined discharges... so no history! Heh.
- For fixing the previous, you will need to add a column to address the fact that the discharge is active/non active. So you can have more than one discharges for a borrowernumber, and always check only one is active at a time.

I'm attaching a followup with those changes on the tests, It'd be great if you could take a look and maybe talk tomorrow on IRC about this to have this ready ASAP.

(In reply to Tomás Cohen Arazi from comment #12)

> - There's a bug on the unit test you introduce for lifting the discharge,
> because it should look like:
> 
>     ok( ! Koha::Borrower::Discharge::is_discharged({ borrowernumber =>
> $borrowernumber }),
>         'The patron is not discharged after the restriction has been lifted'
> );
> 
> And you will notice it fails :/

Indeed. Misuse of DelDebarment. Fixed in the follow-up.

> 
> - There's a problem with Koha::Borrower::Discharge::discharge (not
> introduced by your patch, but affects the beaviour): it overwrites the
> already defined discharges... so no history! Heh.

Hmm, the sql patch introducing discharge_id in the discharges table and the following part of the patch should prevent this behavior, shouldn't it ?


-    my $discharge = $rs->search({ borrower => $borrowernumber });
+    my $discharge = $rs->search({ borrower => $borrowernumber }, { order_by => { -desc => 'needed' }, rows => 1 });

It does not work anymore.
When I try to allow the discharge (/members/discharges.pl?op=allow&borrowernumber=XXXX), I get
DBIx::Class::ResultSet::_rs_update_delete(): Unable to perform complex resultset update() without an identifying set of columns on source 'Discharge' at /home/koha/src/Koha/Borrower/Discharge.pm line 96

Jonathan, did you run misc/devel/update_dbix_class_files.pl , as stated in the commit message, to update the DBIx Discharge schema ?

(In reply to Matthias Meusburger from comment #18)
> Jonathan, did you run misc/devel/update_dbix_class_files.pl , as stated in
> the commit message, to update the DBIx Discharge schema ?

Oops, no!

Created attachment 42670 [details] [review]
Bug 14368: Add discharges history.

Currently, once a user is discharged, lifting the discharge debarment does
not allow the user to ask for a discharge again.

This patch adds the ability for a user to be discharged several times and to
keep an history of the discharges.

Test plan:

 - As a user, ask for a discharge.
 - As a librarian, allow the discharge.
 - Check that the requested and validated dates appear on the discharge panel of the borrower in the admin interface.
 - Lift the discharge debarment of the user.
 - Check that you can repeat these steps and that the new discharges are added in the discharge panel of the borrower in the admin interface.

Please note that you will need to run misc/devel/update_dbix_class_files.pl after the SQL update.
Adding a primary key to the discharges table was mandatory for DBIX::Class to be able to query with order_by.

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Created attachment 42671 [details] [review]
Bug 14368 [Follow-up]: Fix tests.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Created attachment 42672 [details] [review]
Bug 14368 [Follow-up]: Add filter on borrower when displaying discharges history on borrower's page.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Created attachment 42673 [details] [review]
Bug 14368: Add some missing tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Hum, something does not work correctly IMO.
Let say the given workflow:
- A patron asks for a discharge at the OPAC
- The patron comes at the librarian's office.
- On the patron page, there is no track of this demand, so the librarian goes on the discharge page (members/discharge.pl) and generates the discharge
- The discharge does not appear in the discharge list but still exist in the pending discharge requests.

There is something which is not consistent here, it should be fixed.
Failing QA (sorry!)

The last patch fixes the bug you described, Jonathan.

Created attachment 42723 [details] [review]
Bug 14368 [Follow-up]: Fix workflow.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

(In reply to Matthias Meusburger from comment #26)
> The last patch fixes the bug you described, Jonathan.

Thanks, everything sounds good now!

Created attachment 42930 [details] [review]
Bug 14368: (RM followup) DBIx update

Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Patches pushed to master.

Good job, Matthias!