Bug 8782

Summary: Add a feature to close a subscription
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: SerialsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Paul Poulain <paul.poulain>
Severity: new feature    
Priority: P5 - low CC: colin.campbell, dcook, jcamins, katrin.fischer, kyle, nengard, paul.poulain
Version: unspecified   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9323
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9391
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 5357    
Bug Blocks:    
Attachments: Bug 8782: Close a subscription
Bug 8782: Followup: add some minor modifications
Bug 8782: Close a subscription
Bug 8782: Followup: add some minor modifications
[SIGNED-OFF] Bug 8782: Close a subscription
[SIGNED-OFF] Bug 8782: Followup: add some minor modifications
Bug 8782 - Close a subscription - Followup - Fix updatedatabase.pl
Bug 8782 - Close a subscription - Followup - Fix updatedatabase.pl
Bug 8782: Close a subscription

Description Jonathan Druart 2012-09-18 12:10:29 UTC
In the serial module, we would like to close a subscription.
Comment 1 Jonathan Druart 2012-09-18 12:42:51 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2012-09-18 13:05:34 UTC
Hi Jonathan,

thank you for providing the description. I was wondering about the new status "stopped". I was thinking that closig a subscription would happen on subscription level. In which circumstances would I mark an issue as "stopped"? Or did I misunderstand the new status?
Comment 3 Jonathan Druart 2012-09-18 13:38:43 UTC
(In reply to comment #2)
> Hi Jonathan,
> 
> thank you for providing the description. I was wondering about the new
> status "stopped". I was thinking that closig a subscription would happen on
> subscription level. In which circumstances would I mark an issue as
> "stopped"? Or did I misunderstand the new status?

An issue could not be marked as 'stopped'. This status is not writable/settable. The only way to have a status=8 (for serials.status) is to close the subscription and to have a serial expected. If the subscription is closed, the serial cannot be expected.

I don't know if I'm clear :)
Comment 4 David Cook 2012-09-26 21:33:49 UTC
I'm glad to see this enhancement! This issue came up with a client back when I first started using Koha, so it's nice to see a fix for what was one of my first issues!

The patch seems rather large though, so I'm not completely sure how to even start testing to see if it breaks anything. Knowing Jonathan's work, I'm sure that the patch does what it describes (yay), but what's the best way of testing if it breaks something else? Unit tests? Are the following tests enough?

prove t/ 
prove t/db_dependent
prove xt/
prove xt/author

Thoughts?

I'd like to start testing and signing off on larger patches, but I want to make sure that I'm doing a thorough enough job.
Comment 5 Jonathan Druart 2012-10-16 07:45:55 UTC
(In reply to comment #4)
> I'd like to start testing and signing off on larger patches, but I want to
> make sure that I'm doing a thorough enough job.

Hi David!
The patch is large but there is no complex code. There are a lot of template modifications and a new status for serials.
I think if
1/ you agree with this new status
2/ you don't find any regression in the serials module pages
3/ the feature works as expected
4/ you don't have any question

You can sign off this patch :)
Comment 6 Jonathan Druart 2012-10-17 08:27:59 UTC Comment hidden (obsolete)
Comment 7 David Cook 2012-10-17 17:01:36 UTC
Hi Jonathan:

I just tried applying your patch against master with git bz, but it looks like it needs a rebase:

Applying: Bug 8782: Close a subscription
error: patch failed: installer/data/mysql/updatedatabase.pl:5795
error: installer/data/mysql/updatedatabase.pl: patch does not apply
error: patch failed: koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-collection.tt:189
error: koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-collection.tt: patch does not apply
error: patch failed: koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt:33
error: koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt: patch does not apply
Patch failed at 0001 Bug 8782: Close a subscription

Looking forward to testing this one out though!

(In reply to comment #5)
> (In reply to comment #4)
> > I'd like to start testing and signing off on larger patches, but I want to
> > make sure that I'm doing a thorough enough job.
> 
> Hi David!
> The patch is large but there is no complex code. There are a lot of template
> modifications and a new status for serials.
> I think if
> 1/ you agree with this new status
> 2/ you don't find any regression in the serials module pages
> 3/ the feature works as expected
> 4/ you don't have any question
> 
> You can sign off this patch :)
Comment 8 Jonathan Druart 2012-10-18 13:49:48 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2012-10-18 13:49:56 UTC Comment hidden (obsolete)
Comment 10 Kyle M Hall 2012-10-19 11:58:54 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall 2012-10-19 11:59:11 UTC Comment hidden (obsolete)
Comment 12 Kyle M Hall 2012-10-19 11:59:23 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2012-10-19 12:01:00 UTC
Everything looks good except for updatedatabase. If get the following error:

Undefined subroutine &main::column_exists called at installer/data/mysql/updatedatabase.pl line 6016.

I've added a patch to remove this code. Even without it, should this query be executed it should fail gracefully.
Comment 14 Jonathan Druart 2012-10-19 12:05:18 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2012-10-19 12:06:16 UTC
(In reply to comment #13)
> I've added a patch to remove this code. Even without it, should this query
> be executed it should fail gracefully.

Thank you Kyle for your test and your followup !
Comment 16 Paul Poulain 2012-11-28 17:04:34 UTC
QA comment:
 * passes koha-qa.pl
 * quite a large patch, but most of it is related to adding [% IF closed %] in templates and reindenting accordingly
 * nothing spotted that would justify a failed QA

passed QA
Comment 17 Paul Poulain 2012-11-28 17:06:13 UTC
Created attachment 13741 [details] [review]
Bug 8782: Close a subscription

If a subscription is no longer enough published (or we are not waiting
for a new periodical) we are allowed to close it.
If a subscription is closed, we are not able to receive or generate a
new serial.

On the serial module, we can now
- close a subscriptionn
- reopen a closed subscription

On serial search 2 tabs is displayed (opened and closed subscriptions).

This patch adds:
- a new field subscription.closed in DB
- a new status for serials (8 = stopped)

Test plan:
- search subscriptions
- close a subscription and check that you cannot receive or generate a
  new serial
- launch another search and check that the closed serial is into the "closed"
  tab.
- You are allowed to reopen a subscription on the subscription detail
  page and on the subscription result page. A javascript alert ask you
  if are certain to do this operation.
- Check the serial status "stopped" everywhere the status is
  displayed (catalogue/detail.pl, serials/claims.pl,
  serials/serial-issues-full.pl, serials/serials-collection.pl,
  serials/serials-edit.pl, serials/serials-recieve.pl,
  serials/subscription-detail.pl and opac-full-serial-issues.pl)
- The report statistics does not include the closed subscriptions if you
  don't check the "Include expired subscriptions" checkbox.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 8782: Followup: add some minor modifications

- Show 'closed' information in biblio detail page
- Add a column in serials report table
- Search subscriptions on title words instead of string
- Prevent serials editing when subscription is closed
- Don't change status of "disabled" serials

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Bug 8782 - Close a subscription - Followup - Fix updatedatabase.pl

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Comment 18 Jared Camins-Esakov 2012-11-30 13:14:56 UTC
This patch has been pushed to master.