Created attachment 8240 [details] histenddate box legend May be I missed something but even if the subscription history displays the following legend below the histenddate box: (if empty, subscription is still active) the new subscription enddate is systematically copied as histenddate during the renew process as we can see in the ReNewSubscription function (line 1332 from Serial.pm) : $query = qq| UPDATE subscriptionhistory SET histenddate=? WHERE subscriptionid=? |; $sth = $dbh->prepare($query); $sth->execute( $enddate, $subscriptionid );
Hello Lyon3 We will soon renew subscriptions for next year. Can you tell me if it is still an issue? Mathieu
(In reply to mathieu saby from comment #1) > Hello Lyon3 > We will soon renew subscriptions for next year. Can you tell me if it is > still an issue? > > Mathieu yes it is on version 3.10.07 ,no changes. Olivier
Olivier, what do you suggest?
The Renewal of a subscription fills the subscriptionhistory.enddate field. This field is not filled when you create a subscription. It is supposed to keep track of the subscription end for ended subscription. The renewal should follow the same workflow as the creation.
Created attachment 45529 [details] [review] Bug 7728: fix subs history end date consistency Without this patch, there is an inconsistency in subscriptions: * a new subscription has no history end date set (as expected) * if you renew it, a date is set. This patch removes the setting of the history end date during renewal Test plan: * create a subscription, go to subscription-detail.pl page, tab "Summary". No History end date is set * renew the subscription => a date has been set * apply the patch * create another subscription * renew it => no history end date has been set * sign-off
Please provide tests to highlight the changes ;)
I created a subscription, and in 'subscription-detail.pl' page, tab "Summary". No History end date was set I renewed the subscription => a date had been set I applied the patch I created another subscription I renewed it => no history end date had been set I signed-off
Patch tested with a sandbox, by beroud <isabelle.beroud@univ-lyon3.fr>
Created attachment 45776 [details] [review] Bug 7728: fix subs history end date consistency Without this patch, there is an inconsistency in subscriptions: * a new subscription has no history end date set (as expected) * if you renew it, a date is set. This patch removes the setting of the history end date during renewal Test plan: * create a subscription, go to subscription-detail.pl page, tab "Summary". No History end date is set * renew the subscription => a date has been set * apply the patch * create another subscription * renew it => no history end date has been set * sign-off Signed-off-by: beroud <isabelle.beroud@univ-lyon3.fr>
Failing QA this patch, I have to write unit tests before it can be QAed. Once I've added unit tests, I'll switch back to "signed-off" (if someone else want to write UT, feel free !)
Created attachment 47303 [details] [review] BZ 7728 adding unit test test plan: * run without the patch, the test will fail * run with the patch, the test will pass
Comment on attachment 47303 [details] [review] BZ 7728 adding unit test Review of attachment 47303 [details] [review]: ----------------------------------------------------------------- 1/ Should not we update the histenddate if it has been set? 2/ Could we imagine fixing existing DB entries? ::: t/db_dependent/Serials/ReNewSubscription.t @@ +40,5 @@ > + > +#my $mContext = new Test::MockModule('C4::Context'); > +#$mContext->mock( 'userenv', sub { > +# return { branch => $library->{branchcode} }; > +#}); Useless commented lines, please remove. @@ +94,5 @@ > +# Actual testing starts here! > + > +# Renew the subscription and check that enddate has not been set > +ReNewSubscription($subscription->{subscriptionid},'',"2016-01-01",'','',12,''); > +my @history = Koha::Subscription::Histories->search( {subscriptionid => $subscription->{subscriptionid} } ); use Koha::Subscription::Histories->find( $subscription->{subscriptionid} ) when you already have the primary key.
Created attachment 58813 [details] [review] Bug 7728: fix subs history end date consistency Without this patch, there is an inconsistency in subscriptions: * a new subscription has no history end date set (as expected) * if you renew it, a date is set. This patch removes the setting of the history end date during renewal Test plan: * create a subscription, go to subscription-detail.pl page, tab "Summary". No History end date is set * renew the subscription => a date has been set * apply the patch * create another subscription * renew it => no history end date has been set * sign-off Signed-off-by: beroud <isabelle.beroud@univ-lyon3.fr>
Created attachment 58814 [details] [review] BZ 7728 adding unit test test plan: * run without the patch, the test will fail * run with the patch, the test will pass
Created attachment 58815 [details] [review] Bug 7728: QA fixes - Remove useless comments - Use Koha::Objects::find instead of Koha::Objects::search
(In reply to Jonathan Druart from comment #12) > 1/ Should not we update the histenddate if it has been set? I don't know what the expected behaviour is here. Any serials expert around ? > 2/ Could we imagine fixing existing DB entries? We could imagine, but is it possible to know if an histenddate has been set by a renewal or by hand ?
Created attachment 61212 [details] [review] Bug 7728: Fix subs history end date consistency Without this patch, there is an inconsistency in subscriptions: * a new subscription has no history end date set (as expected) * if you renew it, a date is set. This patch removes the setting of the history end date during renewal Test plan: * create a subscription, go to subscription-detail.pl page, tab "Summary". No History end date is set * renew the subscription => a date has been set * apply the patch * create another subscription * renew it => no history end date has been set * sign-off Signed-off-by: beroud <isabelle.beroud@univ-lyon3.fr> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 61213 [details] [review] Bug 7728: Adding unit test ReNewSubscription.t Test plan: * run without the patch, the test will fail * run with the patch, the test will pass Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 61214 [details] [review] Bug 7728: QA fixes - Remove useless comments - Use Koha::Objects::find instead of Koha::Objects::search Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 61215 [details] [review] Bug 7728: [QA Follow-up] Fix POD whitespace Resolves complaints from qa tools. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Two patches from Paul again; come, write a few more :)
Pushed to master for 17.05, thanks Paul, Marcel, Julian!
These patches have been pushed to 16.11.x and will be in 16.11.07. .
Pushed to 3.22.x for 3.22.20
Pushed to 16.05.x, for 16.05.12 release