Bug 20953 - Discharge can be requested several times on OPAC
Summary: Discharge can be requested several times on OPAC
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin SOMERS
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 19990 (view as bug list)
Depends on: 20951
Blocks:
  Show dependency treegraph
 
Reported: 2018-06-15 12:39 UTC by Fridolin SOMERS
Modified: 2018-07-30 12:35 UTC (History)
5 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:


Attachments
Bug 20953: Prevent several discharge requests on OPAC (4.63 KB, patch)
2018-06-18 10:29 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 20953: Prevent several discharge requests on OPAC (4.69 KB, patch)
2018-06-29 19:52 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 20953: Prevent several discharge requests on OPAC (4.74 KB, patch)
2018-07-04 20:23 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 Fridolin SOMERS 2018-06-15 12:39:23 UTC
On OPAC, a user can requested a discharge even if one is already pending.
This generates several pending discharges in staff interface that can not be deleted.

This is because request operation leads to page 'opac-discharge.pl?op=request' and user can refresh this page performing a new request.

Perl code must check that operation is allowed.
Comment 1 Fridolin SOMERS 2018-06-15 12:40:14 UTC
Can not be tested without Bug 20951
Comment 2 Fridolin SOMERS 2018-06-18 10:29:25 UTC
Created attachment 76117 [details] [review]
Bug 20953: Prevent several discharge requests on OPAC

On OPAC, a user can requested a discharge even if one is already pending.
This generates several pending discharges in staff interface that can not be deleted.

This is because request operation leads to page 'opac-discharge.pl?op=request' and user can refresh this page performing a new request.

Perl code must check that operation is allowed.

Patch reoganised the code so that the following FIXME is obsolete :
  'FIXME looks like $available is not needed'
Patch also replaces 'op' arg test to also check undef : input->param("op") // ''

Test plan :
1) Set system preference 'useDischarge' to 'Allow'
2) Choose a patron without checkouts nor fines nor restrictions
3) Log at OPAC and go to patron page /cgi-bin/koha/opac-user.pl
4) Click on 'ask for a discharge' tab
   => You see /cgi-bin/koha/opac-discharge.pl
      with text 'What is a discharge? ...'
5) Click on 'Ask for a discharge' link
   => You see /cgi-bin/koha/opac-discharge.pl?op=request
      with text 'Your discharge request has been sent ...'
6) In a new browser tab/page, go to intranet on /cgi-bin/koha/members/discharges.pl
   => You see one discharge requets for the patron
7) Come back to OPAC and refresh page
   => You see /cgi-bin/koha/opac-discharge.pl
   with text 'Your discharge will be available on this page within a few days.'
8) Come back to intranet and refresh /cgi-bin/koha/opac-discharge.pl
   => There is still one requets for the patron
9) Come back to OPAC and enter URL /cgi-bin/koha/opac-discharge.pl?op=get
   => You see /cgi-bin/koha/opac-discharge.pl
      with text 'Your discharge will be available on this page within a few days.'
10) Come back to intranet and refresh /cgi-bin/koha/opac-discharge.pl
   => There is still one requets for the patron
11) Click on 'allow' on patron discharge request
12) Come back to OPAC and refresh /cgi-bin/koha/opac-discharge.pl
    => You see link 'Get your discharge'
13) enter URL /cgi-bin/koha/opac-discharge.pl?op=request
    => You see same page and no new discharge requets is created
14) Come back to intranet on patron details page
15) Remove the discharge restriction
16) Come back to OPAC and refresh /cgi-bin/koha/opac-discharge.pl
    => You see text 'What is a discharge?...'
17) enter URL /cgi-bin/koha/opac-discharge.pl?op=get
    => You see same page and no new discharge requets is created
Comment 3 Koha Team Lyon 3 2018-06-18 15:41:45 UTC
Hello,
Tested on a sandbobx (biblibre sandbox n°1)
At step n°11, there's an error when clicking on "allow" :

Software error:
Undefined subroutine &Koha::Patron::Debarments::AddDebarment called at /home/koha/src/Koha/Patron/Discharge.pm line 85.
For help, please send mail to the webmaster ([no address given]), giving this error message and the time and date of the error.
Comment 4 Fridolin SOMERS 2018-06-19 05:23:13 UTC
(In reply to Koha Team Lyon 3 from comment #3)
> Hello,
> Tested on a sandbobx (biblibre sandbox n°1)
> At step n°11, there's an error when clicking on "allow" :
> 
> Software error:
> Undefined subroutine &Koha::Patron::Debarments::AddDebarment called at
> /home/koha/src/Koha/Patron/Discharge.pm line 85.
> For help, please send mail to the webmaster ([no address given]), giving
> this error message and the time and date of the error.

You need to apply also Bug 20951 to be able to run test plan
Comment 5 Charles Farmer 2018-06-29 19:48:30 UTC
Two things:

* This bug's dependency is now 'pushed to stable'. The patch in this bug can now be applied alone.
* Oddly, there seems to be an issue with generate_as_pdf at the end of the test plan. This is what I get on my system: Undefined subroutine &C4::Letters::GetPreparedLetter called at /.../Koha/Patron/Discharge.pm line 113

Since the issue I mentionned is unrelated to the problem you're trying to solve, I'll open another bug for it.
Comment 6 Charles Farmer 2018-06-29 19:52:57 UTC
Created attachment 76586 [details] [review]
Bug 20953: Prevent several discharge requests on OPAC

On OPAC, a user can requested a discharge even if one is already pending.
This generates several pending discharges in staff interface that can not be deleted.

This is because request operation leads to page 'opac-discharge.pl?op=request' and user can refresh this page performing a new request.

Perl code must check that operation is allowed.

Patch reoganised the code so that the following FIXME is obsolete :
  'FIXME looks like $available is not needed'
Patch also replaces 'op' arg test to also check undef : input->param("op") // ''

Test plan :
1) Set system preference 'useDischarge' to 'Allow'
2) Choose a patron without checkouts nor fines nor restrictions
3) Log at OPAC and go to patron page /cgi-bin/koha/opac-user.pl
4) Click on 'ask for a discharge' tab
   => You see /cgi-bin/koha/opac-discharge.pl
      with text 'What is a discharge? ...'
5) Click on 'Ask for a discharge' link
   => You see /cgi-bin/koha/opac-discharge.pl?op=request
      with text 'Your discharge request has been sent ...'
6) In a new browser tab/page, go to intranet on /cgi-bin/koha/members/discharges.pl
   => You see one discharge requets for the patron
7) Come back to OPAC and refresh page
   => You see /cgi-bin/koha/opac-discharge.pl
   with text 'Your discharge will be available on this page within a few days.'
8) Come back to intranet and refresh /cgi-bin/koha/opac-discharge.pl
   => There is still one requets for the patron
9) Come back to OPAC and enter URL /cgi-bin/koha/opac-discharge.pl?op=get
   => You see /cgi-bin/koha/opac-discharge.pl
      with text 'Your discharge will be available on this page within a few days.'
10) Come back to intranet and refresh /cgi-bin/koha/opac-discharge.pl
   => There is still one requets for the patron
11) Click on 'allow' on patron discharge request
12) Come back to OPAC and refresh /cgi-bin/koha/opac-discharge.pl
    => You see link 'Get your discharge'
13) enter URL /cgi-bin/koha/opac-discharge.pl?op=request
    => You see same page and no new discharge requets is created
14) Come back to intranet on patron details page
15) Remove the discharge restriction
16) Come back to OPAC and refresh /cgi-bin/koha/opac-discharge.pl
    => You see text 'What is a discharge?...'
17) enter URL /cgi-bin/koha/opac-discharge.pl?op=get
    => You see same page and no new discharge requets is created

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Comment 7 Katrin Fischer 2018-07-04 20:23:24 UTC
Created attachment 76699 [details] [review]
Bug 20953: Prevent several discharge requests on OPAC

On OPAC, a user can requested a discharge even if one is already pending.
This generates several pending discharges in staff interface that can not be deleted.

This is because request operation leads to page 'opac-discharge.pl?op=request' and user can refresh this page performing a new request.

Perl code must check that operation is allowed.

Patch reoganised the code so that the following FIXME is obsolete :
  'FIXME looks like $available is not needed'
Patch also replaces 'op' arg test to also check undef : input->param("op") // ''

Test plan :
1) Set system preference 'useDischarge' to 'Allow'
2) Choose a patron without checkouts nor fines nor restrictions
3) Log at OPAC and go to patron page /cgi-bin/koha/opac-user.pl
4) Click on 'ask for a discharge' tab
   => You see /cgi-bin/koha/opac-discharge.pl
      with text 'What is a discharge? ...'
5) Click on 'Ask for a discharge' link
   => You see /cgi-bin/koha/opac-discharge.pl?op=request
      with text 'Your discharge request has been sent ...'
6) In a new browser tab/page, go to intranet on /cgi-bin/koha/members/discharges.pl
   => You see one discharge requets for the patron
7) Come back to OPAC and refresh page
   => You see /cgi-bin/koha/opac-discharge.pl
   with text 'Your discharge will be available on this page within a few days.'
8) Come back to intranet and refresh /cgi-bin/koha/opac-discharge.pl
   => There is still one requets for the patron
9) Come back to OPAC and enter URL /cgi-bin/koha/opac-discharge.pl?op=get
   => You see /cgi-bin/koha/opac-discharge.pl
      with text 'Your discharge will be available on this page within a few days.'
10) Come back to intranet and refresh /cgi-bin/koha/opac-discharge.pl
   => There is still one requets for the patron
11) Click on 'allow' on patron discharge request
12) Come back to OPAC and refresh /cgi-bin/koha/opac-discharge.pl
    => You see link 'Get your discharge'
13) enter URL /cgi-bin/koha/opac-discharge.pl?op=request
    => You see same page and no new discharge requets is created
14) Come back to intranet on patron details page
15) Remove the discharge restriction
16) Come back to OPAC and refresh /cgi-bin/koha/opac-discharge.pl
    => You see text 'What is a discharge?...'
17) enter URL /cgi-bin/koha/opac-discharge.pl?op=get
    => You see same page and no new discharge requets is created

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Nick Clemens 2018-07-06 10:09:31 UTC
Can we attempt to deal with the discharges that may have been created and cannot be removed by staff? Can you provide an update statement Frido?
Comment 9 Fridolin SOMERS 2018-07-09 12:14:54 UTC
(In reply to Nick Clemens from comment #8)
> Can we attempt to deal with the discharges that may have been created and
> cannot be removed by staff? Can you provide an update statement Frido?

Seems hard, I'd say just add a warning text.
Comment 10 Katrin Fischer 2018-07-11 05:50:41 UTC
*** Bug 19990 has been marked as a duplicate of this bug. ***
Comment 11 Nick Clemens 2018-07-13 11:25:47 UTC
Awesome work all!

Pushed to master for 18.11
Comment 12 Martin Renvoize 2018-07-15 19:02:00 UTC
Pushed to 18.05.x for 18.05.02
Comment 13 Fridolin SOMERS 2018-07-30 12:35:04 UTC
Pushed to 17.11.x for 17.11.09