Description
Koha Team University Lyon 3
2016-11-17 16:53:32 UTC
The current irregularity feature works with saving internal fictive issue numbers in a text field irregularity. If you edited/shifted some dates in serial collection, you might be surprised what happens. You cannot view the saved irregularities in serial collection (I only see e.g. 2 issues on the Planning tab); they are only visible when editing the subscription and looking at the prediction pattern again. Bug 18365 and friends made it easier to handle discrepancies in the schedule. An occasional irregularity could be resolved that way. Would it be useful to define recurring irregularities in terms of the serial frequency chosen? If it is month, we could allow the user to select one or more months to skip? Same for weeks or days. (Year seems less interesting.) Doing this would replace selection of irregulars in the prediction pattern. If so, how should we handle 'special' frequencies like 1 per 3 months or 3 per month btw? Skip all in e.g. July or refine with skip second in July etc.? It seems to me that we should make the irregularity info more visible to the user too on Serial collection form. Any feedback is appreciated. I feel my expectation for this was always that it will be recurring. So say I have a serial prediction pattern for 1/month and I say there is never an issue in August (vacation?), I'd want this to stick. As I understood the feature, it would allow to have patterns that are not 'regular' in some odd ways. Like 50 issues a year, mostly weekly, where the time between issues is not always the same, but some are just skipped. Not sure if we could reach out to serial librarians on the mailing list maybe? (In reply to Katrin Fischer from comment #2) > Not sure if we could reach out to serial librarians on the mailing list > maybe? Sent to general ml. We are looking for feedback on this one - adding some names to cc in hope of more comments! Thread: https://lists.katipo.co.nz/pipermail/koha/2017-June/048380.html Sadly no comments so far. So it's Lyon 3 and me so far considering this a bug, maybe enough? (In reply to Katrin Fischer from comment #5) > Thread: https://lists.katipo.co.nz/pipermail/koha/2017-June/048380.html > > Sadly no comments so far. So it's Lyon 3 and me so far considering this a > bug, maybe enough? We can wait a little bit more :) Good things take time.. Hope to address this later, having some more feedback. Hello Marcel, I think it's a pity not to try to find a solution if we are several libraries that miss this feature. Furthermore, it existed in the previous versions of Koha and for us, it was clearly a regression when it disappeared. If the solution proposed isn't correct, it will be Failed QA.... If you don't mind, I will change this bug title to the former one to give a chance to this feature to be back in koha. Sonia Hi Sonia, it's good information that this is a regression to the older serials module. I agree that we need to bring it back. Created attachment 68259 [details] [review] Bug 17656 - Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for monthly or daily serials ( 1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly subscription Here is a proposal for keeping irregularities on subscription after renewal. This patch save days of week or month in a new column (permanent_irregularity). So it works only for daily and monthly subscriptions. I think that it doesn't make sense to do this for weekly... you opinion? Created attachment 68261 [details] [review] Bug 17656 - Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for monthly or daily serials ( 1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly subscription Hi Alex When checking if Wednesday and Thursday are still checked I have found that the test prediction pattern div is no longer being displayed. The errors I am getting in my intranet-error.log are: 0.0.2.2:34032] AH01215: [Fri Oct 20 00:20:18 2017] showpredictionpattern.pl: Argument "month" isn't numeric in numeric eq (==) at /home/vagrant/kohaclone/serials/showpredictionpattern.pl line 116., referer: http://localhost:8081/cgi-bin/koha/serials/subscription-add.pl?op=modify&subscriptionid=4 [Fri Oct 20 00:20:18.672718 2017] [cgi:error] [pid 7358] [client 10.0.2.2:34032] AH01215: [Fri Oct 20 00:20:18 2017] showpredictionpattern.pl: Argument "day" isn't numeric in numeric eq (==) at /home/vagrant/kohaclone/serials/showpredictionpattern.pl line 116., referer: http://localhost:8081/cgi-bin/koha/serials/subscription-add.pl?op=modify&subscriptionid=4 [Fri Oct 20 00:20:18.678244 2017] [cgi:error] [pid 7358] [client 10.0.2.2:34032] AH01215: [Fri Oct 20 00:20:18 2017] showpredictionpattern.pl: The method permanent_irregularity is not covered by tests!, referer: http://localhost:8081/cgi-bin/koha/serials/subscription-add.pl?op=modify&subscriptionid=4 Your t/db_dependent/Serials/SubscriptionIrregularity.pm test also fails because there is no test for permanent_irregularity function Hello Alex, Good thing for dayly and monthly publication. As for weekly issues, don't know if it's realistic but what about basing calculation on week numbering (iso 8601) ? Some review (not so unusual cases) announce clearly their "not issued weeks". Example the New Yorker publishes 5 issues that runs along 2 weeks and they are cited in the publication information block. Olivier Crouzet (In reply to Alex Buckley from comment #12) > Hi Alex > > When checking if Wednesday and Thursday are still checked I have found that > the test prediction pattern div is no longer being displayed. > > The errors I am getting in my intranet-error.log are: > > 0.0.2.2:34032] AH01215: [Fri Oct 20 00:20:18 2017] showpredictionpattern.pl: > Argument "month" isn't numeric in numeric eq (==) at > /home/vagrant/kohaclone/serials/showpredictionpattern.pl line 116., referer: > http://localhost:8081/cgi-bin/koha/serials/subscription-add. > pl?op=modify&subscriptionid=4 > [Fri Oct 20 00:20:18.672718 2017] [cgi:error] [pid 7358] [client > 10.0.2.2:34032] AH01215: [Fri Oct 20 00:20:18 2017] > showpredictionpattern.pl: Argument "day" isn't numeric in numeric eq (==) at > /home/vagrant/kohaclone/serials/showpredictionpattern.pl line 116., referer: > http://localhost:8081/cgi-bin/koha/serials/subscription-add. > pl?op=modify&subscriptionid=4 > [Fri Oct 20 00:20:18.678244 2017] [cgi:error] [pid 7358] [client > 10.0.2.2:34032] AH01215: [Fri Oct 20 00:20:18 2017] > showpredictionpattern.pl: The method permanent_irregularity is not covered > by tests!, referer: > http://localhost:8081/cgi-bin/koha/serials/subscription-add. > pl?op=modify&subscriptionid=4 I will fix the warning "Argument x isn't numeric" while doing the patch for weekly. > > Your t/db_dependent/Serials/SubscriptionIrregularity.pm test also fails > because there is no test for permanent_irregularity function You must Launch misc/devel/update_dbix_class_files.pl to update Koha/Schema/Result/Subscription.pm Created attachment 68607 [details] [review] Bug 17656 - Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for monthly, daily or weekly serials ( 1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription (In reply to Koha Team Lyon 3 from comment #13) > Hello Alex, > > Good thing for dayly and monthly publication. > As for weekly issues, don't know if it's realistic but what about basing > calculation on week numbering (iso 8601) ? > Some review (not so unusual cases) announce clearly their "not issued > weeks". Example the New Yorker publishes 5 issues that runs along 2 weeks > and they are cited in the publication information block. > > Olivier Crouzet Hello Olivier, I did it for weekly serials. Please test and give me your opinion. Created attachment 68608 [details] [review] Bug 17656 - Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for monthly, daily or weekly serials ( 1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Created attachment 68665 [details] [review] Bug 17656 - Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for monthly, daily or weekly serials ( 1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Ok Alex, I will do it as soon as possible Test plan works as described, but QA tools warns of forbidden patterns. koha-tmpl/intranet-tmpl/prog/en/includes/serials/subscription_daily_choice.inc FAIL forbidden patterns forbidden pattern: Do not use line breaks inside template tags (bug 18675) (line 5) (In reply to David Bourgault from comment #20) > Test plan works as described, but QA tools warns of forbidden patterns. > > koha-tmpl/intranet-tmpl/prog/en/includes/serials/subscription_daily_choice. > inc > FAIL forbidden patterns > forbidden pattern: Do not use line breaks inside template tags (bug 18675) > (line 5) Hello David, I have no warning when launching the qa script (koha-qa.pl -v 3 -c 1) For the file subscription_daily_choice file i got: OK koha-tmpl/intranet-tmpl/prog/en/includes/serials/subscription_daily_choice.inc OK forbidden patterns OK git manipulation OK spelling OK tt_valid OK valid_template (In reply to Alex Arnaud from comment #21) > (In reply to David Bourgault from comment #20) > > Test plan works as described, but QA tools warns of forbidden patterns. > > > > koha-tmpl/intranet-tmpl/prog/en/includes/serials/subscription_daily_choice. > > inc > > FAIL forbidden patterns > > forbidden pattern: Do not use line breaks inside template tags (bug 18675) > > (line 5) > > Hello David, > > I have no warning when launching the qa script (koha-qa.pl -v 3 -c 1) > For the file subscription_daily_choice file i got: > > OK > koha-tmpl/intranet-tmpl/prog/en/includes/serials/subscription_daily_choice. > inc > OK forbidden patterns > OK git manipulation > OK spelling > OK tt_valid > OK valid_template Sorry you are right. My QA tools was not up to date Created attachment 69124 [details] [review] Bug 17656 - Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for monthly, daily or weekly serials ( 1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Created attachment 69207 [details] [review] Bug 17656 - Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for monthly, daily or weekly serials ( 1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Test plan worked. QA tool all green except for C4/Serials.pm which warned that "POD coverage was greater before". Inspecting the file manually, this doesnt seem to be the case. I suspect that it is triggered because you remove a function and its pod, which it shouldn't warn about. Signed off. Hello Alex, For the three types of periodicity, the "permanent" irregularities on top of table are kept over the renewal. This is working. But the detail of excluded issues is not automatically set in relation with these ones (cf joined image).We have to manually uncheck and recheck the "permanent" irregularities to have the corresponding issues checked in the "Not published column". That's a progress in the sense that there's a trace of the previous configuration but the idea was to get all this automatically. would it be possible ? Olivier Crouzet Created attachment 69279 [details]
monthly irregularities test example
Created attachment 69553 [details] [review] Bug 17656 - Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for monthly, daily or weekly serials ( 1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Hello Olivier, Please re-try with this patch (just a typo in the previous). Bonjour Alex It's ok now as far as keeping irregularities is concerned : the corrresponding issues to exclude are now automatically checked when checking the top table of "permanent" irregularities. But there's one problem remaining about multi-receiving weekly serials. The excluded issues are correctly skipped when receiving issues one by one but not when receiving through multi-receiving function. For the others two periodicities (daily and monthly), it also works with multi-receiving. I tested without patch and the problem disappears. Olivier Crouzet Olivier, thx for your great testing! (In reply to Koha Team Lyon 3 from comment #30) > Bonjour Alex > It's ok now as far as keeping irregularities is concerned : the > corrresponding issues to exclude are now automatically checked when > checking the top table of "permanent" irregularities. > But there's one problem remaining about multi-receiving weekly serials. > The excluded issues are correctly skipped when receiving issues one by one > but not when receiving through multi-receiving function. > > For the others two periodicities (daily and monthly), it also works with > multi-receiving. > > I tested without patch and the problem disappears. > > Olivier Crouzet Hello Olivier, Can you write a test plan for this bug? I've tried the following one and it works for: - create a weekly serial from week #44 to week 50 (7 issues), - place irregularities on week #47 and #48, save, - click on Receive button on subscription detail page, - click on multi-receiving, - How many issues do you want to receive ? => 7 - I got 5 issues generated. #47 and #48 has been skipped Hello Alex I tested again and realized that the problem is not bound to multi-receiving function. I was focusing last time on verifying at each renewal wether issues checkboxes was checked or not and I almost missed it (and my checkings were followed by a subscription saving as a reflex). So the remaining problem is that the irregularities are not fit to new subscription unless you edit the subscription, run the test and save. If you just make the renewal the irregularities(field irregularity) are not updated. When testing I fall again upon a bug that I have already posted for ( https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15484). It's also tight with irregularities and it would be great to have look at it. Olivier Created attachment 69814 [details] [review] Bug 17656 - Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for monthly, daily or weekly serials ( 1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 69820 [details] [review] Bug 17656 - Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for monthly, daily or weekly serials ( 1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 69915 [details] [review] Bug 17656 - Update irregularities on subscription renewals Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Renew subscription from 2017-10-16 - Check that you have the prediction pattern table on renwal page - Submit renewal - Check that irregularity and permanent_irregularity column have been updated. - Do the same for a monthly and weekly subscription Hello Olivier, The last patch add the prediction parttern table to the renewal pop-up. While renewing, you can change irregularities or let them as they were before. I think this is better than automatically update irregularities without asking the user. Please test Alex Hey Alex, Happy new year I tested again the three periodicity and it works like a charm ! I agree with you about not renewing automatically and being able to modify irregularities. The more flexibility, the better. Just a thing about why I don't sign yet : I tested on top master and I had to solve a conflict (in the first patch : staff-global.css file) to be able to apply the patch. I leave you rebase the commit and I will sign it right away. Created attachment 70335 [details] [review] Bug 17656 - Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for monthly, daily or weekly serials ( 1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 70336 [details] [review] Bug 17656 - Update irregularities on subscription renewals Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Renew subscription from 2017-10-16 - Check that you have the prediction pattern table on renwal page - Submit renewal - Check that irregularity and permanent_irregularity column have been updated. - Do the same for a monthly and weekly subscription (In reply to Koha Team Lyon 3 from comment #38) > > Just a thing about why I don't sign yet : I tested on top master and I had > to solve a conflict (in the first patch : staff-global.css file) to be able > to apply the patch. > I leave you rebase the commit and I will sign it right away. Hello Olivier, patches rebased. Happy new year! it works, thank you Alex ! Comment on attachment 70335 [details] [review] Bug 17656 - Keep irregularities during subscription renewals Review of attachment 70335 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/includes/serials/subscription_daily_choice.inc @@ +1,5 @@ > +<p><em> > + If there is a day (or more) in the week where issues are never > + published, you can check corresponding boxes below. > +</em></p> > +[% daily_choice = [ {name => 'monday', desc => 'Monday', value => 1}, {name => 'tuesday', desc => 'Tuesday', value => 2}, {name => 'wednesday', desc => 'Wednesday', value => 3}, {name => 'thursday', desc => 'Thursday', value => 4}, {name => 'friday', desc => 'Friday', value => 5}, {name => 'saturday', desc => 'Saturday', value => 6}, {name => 'sunday', desc => 'Sunday', value => 7} ] %] This way the day names are not translatable ::: koha-tmpl/intranet-tmpl/prog/en/includes/serials/subscription_monthly_choice.inc @@ +1,5 @@ > +<p><em> > + If there is a month (or more) in the year where issues are never > + published, you can check corresponding boxes below. > +</em></p> > +[% monthly_choice = [ {name => 'january', desc => 'January', value => 1}, {name => 'february', desc => 'February', value => 2}, {name => 'march', desc => 'March', value => 3}, {name => 'april', desc => 'April', value => 4}, {name => 'may', desc => 'May', value => 5}, {name => 'june', desc => 'June', value => 6}, {name => 'july', desc => 'July', value => 7}, {name => 'august', desc => 'August', value => 8}, {name => 'september', desc => 'September', value => 9}, {name => 'october', desc => 'October', value => 10}, {name => 'november', desc => 'November', value => 11}, {name => 'december', desc => 'December', value =>12} ] %] Month names are not translatable ::: t/Koha/DateUtils.t @@ +26,5 @@ > + > +my $dt = DateTime->new(year => 2017, month => 1, day => 1); > + > +eval { issue_number('2017', 'day'); }; > +is($@, 'Date passed to issue_number is not a valid DateTime object'); Could you please also test if the ref() of exception has right class? @@ +40,5 @@ > +is(issue_number($dt, 'day'), 6, '2022-09-17 is the sixth day of week'); > + > +is(issue_number($dt, 'week'), 37, '2022-09-17 is the week #37 of the year'); > + > +is(issue_number($dt, 'month'), 9, '2017-09-17 is the ninth month of the year'); should be 2022-09-17 ... Comment on attachment 70336 [details] [review] Bug 17656 - Update irregularities on subscription renewals Review of attachment 70336 [details] [review]: ----------------------------------------------------------------- ::: C4/Serials.pm @@ +2713,4 @@ > return @$serials; > } > > +=head2 GessEnddate typo @@ +2718,5 @@ > +my $enddate = GessEnddate($startdate_iso $frequencyid, $numberlength, $weeklength, $monthlength); > + > +=cut > + > +sub GuessEnddate { Better to add new subs just to Koha namespace... but I understand it here - we do not have all the objects needed for serials management (frequencies and number patterns are missing) and calling C4 subs from Koha namespace is not good idea... If you are not willing to add needed objects, we could just fill a bug for now... ::: Koha/Subscription.pm @@ +77,5 @@ > +Return the length type (issues, weeks, months) > + > +=cut > + > +sub subtype { Maybe lengthtype would be better name? Not blocker ::: koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-renew.tt @@ +52,5 @@ > + <input type="hidden" id="numbering3" value="[% numbering3 %]" /> > + <input type="hidden" id="whenmorethan1" value="[% whenmorethan1 %]" /> > + <input type="hidden" id="whenmorethan2" value="[% whenmorethan2 %]" /> > + <input type="hidden" id="whenmorethan3" value="[% whenmorethan3 %]" /> > + <input type="hidden" id="locale" value="[% locale %]" /> Why do you include all the data when renewing subscription? It should be not needed! One more thing - you missed to add the new db column to kohastructure.sql. Also, the atomic update must be idempotent - you should always use IGNORE or helper subs like column_exists and so on - see https://wiki.koha-community.org/wiki/Database_updates (In reply to Josef Moravec from comment #44) > Comment on attachment 70336 [details] [review] [review] > Bug 17656 - Update irregularities on subscription renewals > > Review of attachment 70336 [details] [review] [review]: > ----------------------------------------------------------------- > > > Better to add new subs just to Koha namespace... but I understand it here - > we do not have all the objects needed for serials management (frequencies > and number patterns are missing) and calling C4 subs from Koha namespace is > not good idea... If you are not willing to add needed objects, we could just > fill a bug for now... Don't understand. You mean create Koha::Subscription->GuessEnddate? > > ::: koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-renew.tt > @@ +52,5 @@ > > + <input type="hidden" id="numbering3" value="[% numbering3 %]" /> > > + <input type="hidden" id="whenmorethan1" value="[% whenmorethan1 %]" /> > > + <input type="hidden" id="whenmorethan2" value="[% whenmorethan2 %]" /> > > + <input type="hidden" id="whenmorethan3" value="[% whenmorethan3 %]" /> > > + <input type="hidden" id="locale" value="[% locale %]" /> > > Why do you include all the data when renewing subscription? It should be not > needed! Yes. testPredictionPattern js function needs all the data. Created attachment 70430 [details] [review] Bug 17656 - Fix QA comments - Make day and month names translatable, - Update unit tests, - Fix a typo in C4/Serials.pm (GessEndDate), - rename subtype subroutine to lengthtype - DB Change: add changes in kohastructure.sql and Bug_17656-add-permanent Comment on attachment 70430 [details] [review] Bug 17656 - Fix QA comments - Make day and month names translatable, - Update unit tests, - Fix a typo in C4/Serials.pm (GessEndDate), - rename subtype subroutine to lengthtype - DB Change: add changes in kohastructure.sql and Bug_17656-add-permanent Review of attachment 70430 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/includes/serials/subscription_daily_choice.inc @@ +4,2 @@ > </em></p> > [% daily_choice = [ {name => 'monday', desc => 'Monday', value => 1}, {name => 'tuesday', desc => 'Tuesday', value => 2}, {name => 'wednesday', desc => 'Wednesday', value => 3}, {name => 'thursday', desc => 'Thursday', value => 4}, {name => 'friday', desc => 'Friday', value => 5}, {name => 'saturday', desc => 'Saturday', value => 6}, {name => 'sunday', desc => 'Sunday', value => 7} ] %] Looks like you do not need the "desc" values here now, is that true? ::: koha-tmpl/intranet-tmpl/prog/en/includes/serials/subscription_monthly_choice.inc @@ +4,2 @@ > </em></p> > [% monthly_choice = [ {name => 'january', desc => 'January', value => 1}, {name => 'february', desc => 'February', value => 2}, {name => 'march', desc => 'March', value => 3}, {name => 'april', desc => 'April', value => 4}, {name => 'may', desc => 'May', value => 5}, {name => 'june', desc => 'June', value => 6}, {name => 'july', desc => 'July', value => 7}, {name => 'august', desc => 'August', value => 8}, {name => 'september', desc => 'September', value => 9}, {name => 'october', desc => 'October', value => 10}, {name => 'november', desc => 'November', value => 11}, {name => 'december', desc => 'December', value =>12} ] %] Looks like you do not need the "desc" values here now, is that true? (In reply to Alex Arnaud from comment #46) > (In reply to Josef Moravec from comment #44) > > Comment on attachment 70336 [details] [review] [review] [review] > > Bug 17656 - Update irregularities on subscription renewals > > > > Review of attachment 70336 [details] [review] [review] [review]: > > ----------------------------------------------------------------- > > > > > > Better to add new subs just to Koha namespace... but I understand it here - > > we do not have all the objects needed for serials management (frequencies > > and number patterns are missing) and calling C4 subs from Koha namespace is > > not good idea... If you are not willing to add needed objects, we could just > > fill a bug for now... > > Don't understand. You mean create Koha::Subscription->GuessEnddate? Yes, that is what I ment. > > > > ::: koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-renew.tt > > @@ +52,5 @@ > > > + <input type="hidden" id="numbering3" value="[% numbering3 %]" /> > > > + <input type="hidden" id="whenmorethan1" value="[% whenmorethan1 %]" /> > > > + <input type="hidden" id="whenmorethan2" value="[% whenmorethan2 %]" /> > > > + <input type="hidden" id="whenmorethan3" value="[% whenmorethan3 %]" /> > > > + <input type="hidden" id="locale" value="[% locale %]" /> > > > > Why do you include all the data when renewing subscription? It should be not > > needed! > > Yes. testPredictionPattern js function needs all the data. Then I think the showpredictionpattern.pl script should be changed to use stored data for parameters which weren't changed... (In reply to Josef Moravec from comment #49) > (In reply to Alex Arnaud from comment #46) > > (In reply to Josef Moravec from comment #44) > > > Comment on attachment 70336 [details] [review] [review] [review] [review] > > > Bug 17656 - Update irregularities on subscription renewals > > > > > > Review of attachment 70336 [details] [review] [review] [review] [review]: > > > ----------------------------------------------------------------- > > > > Yes. testPredictionPattern js function needs all the data. > > Then I think the showpredictionpattern.pl script should be changed to use > stored data for parameters which weren't changed... Why? testPredictionPattern JS function get this data and pass them to showpredictionpattern.pl. It works exactly like in subscription-add.pl (In reply to Josef Moravec from comment #49) > (In reply to Alex Arnaud from comment #46) > > (In reply to Josef Moravec from comment #44) > > > Comment on attachment 70336 [details] [review] [review] [review] [review] > > > Bug 17656 - Update irregularities on subscription renewals > > > > > > Review of attachment 70336 [details] [review] [review] [review] [review]: > > > ----------------------------------------------------------------- > > > > > > > > > Better to add new subs just to Koha namespace... but I understand it here - > > > we do not have all the objects needed for serials management (frequencies > > > and number patterns are missing) and calling C4 subs from Koha namespace is > > > not good idea... If you are not willing to add needed objects, we could just > > > fill a bug for now... > > > > Don't understand. You mean create Koha::Subscription->GuessEnddate? > > Yes, that is what I ment. I see one obstacle. GuessEnddate is used with the subscription's data itself in subscription-renew.pl and with data coming from the html form in subscription-add.pl. So method calls will be: $subscription_object->GuessEnddate() in the first case, and $subscription_object->GuessEnddate($firstacquidate, $periodicity, ...) in the second one Quite tricky no? Created attachment 70619 [details] [review] Bug 17656 - [QA followup] Remove useless description in subscription_daily_choice.inc and subscription_monthly_choice.inc Alex, should this be set back to Signed off with your follow-up? (In reply to Alex Arnaud from comment #51) > (In reply to Josef Moravec from comment #49) > > (In reply to Alex Arnaud from comment #46) > > > (In reply to Josef Moravec from comment #44) > > > > Comment on attachment 70336 [details] [review] [review] [review] [review] [review] > > > > Bug 17656 - Update irregularities on subscription renewals > > > > > > > > Review of attachment 70336 [details] [review] [review] [review] [review] [review]: > > > > ----------------------------------------------------------------- > > > > > > > > > > > > Better to add new subs just to Koha namespace... but I understand it here - > > > > we do not have all the objects needed for serials management (frequencies > > > > and number patterns are missing) and calling C4 subs from Koha namespace is > > > > not good idea... If you are not willing to add needed objects, we could just > > > > fill a bug for now... > > > > > > Don't understand. You mean create Koha::Subscription->GuessEnddate? > > > > Yes, that is what I ment. > > I see one obstacle. GuessEnddate is used with the subscription's data itself > in subscription-renew.pl and with data coming from the html form in > subscription-add.pl. So method calls will be: > > $subscription_object->GuessEnddate() in the first case, > and > $subscription_object->GuessEnddate($firstacquidate, $periodicity, ...) in > the second one > > Quite tricky no? Not that tricky I think, you should use it like this: my $subscription_object = Koha::Subscription->new( { firstacquidate => "....", periodicity => "....", ... }); my $enddate = $subscription_object->guess_end_date; the call is always the same. It doesn't matter if it is in the database or not. If you wan't to save it to db, use ->store, but it is not an obligation. Also, please consider to change ->irregularities and ->permanent_irregularities to be getter and setter. It also will need rebase again, sorry. Created attachment 75403 [details] [review] Bug 17656 - Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for monthly, daily or weekly serials ( 1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 75404 [details] [review] Bug 17656 - Update irregularities on subscription renewals Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Renew subscription from 2017-10-16 - Check that you have the prediction pattern table on renwal page - Submit renewal - Check that irregularity and permanent_irregularity column have been updated. - Do the same for a monthly and weekly subscription Created attachment 75405 [details] [review] Bug 17656 - Fix QA comments - Make day and month names translatable, - Update unit tests, - Fix a typo in C4/Serials.pm (GessEndDate), - rename subtype subroutine to lengthtype - DB Change: add changes in kohastructure.sql and Bug_17656-add-permanent-irre Created attachment 75406 [details] [review] Bug 17656 - [QA followup] Remove useless description in subscription_daily_choice.inc and subscription_monthly_choice.inc Created attachment 75407 [details] [review] Bug 17656: Update irregularities on multi-renewals Created attachment 75408 [details] [review] Bug 17656: move GuessEnddate in Koha::Subscription::guess_enddate I've rebased these patches. Particulary upon bug 7910 that has been pushed recently. SO, addition for the plan is: - create several subscription with permanent irregularities, - renew them using the multi-renew feature, - check that the new irregulaties are consistent Bonjour Alex, J'étais parti, le coeur plein d'entrain, pour une nouvelle séance de test sur la nouvelle mouture de ton patch (tu as dû te régaler..), mais après mise à jour d'une master de test que j'ai sur vbox (qu'avait pas servi depuis la dernière fois), je me retrouve avec un affichage déglingué et je constate que koha cherche à charger les css et js en leur ajoutant un suffixe 18.0500000 (ça ressemble à la date de mise à jour du patch ?!) : http://preprod:8080/intranet-tmpl/prog/css/staff-global_18.0500000.css Soit j'ai fait une grosse bourde, soit il y a quelque chose qui m'échappe (voire même les deux d'ailleurs ). ça te parle ? Olivier (In reply to Koha Team Lyon 3 from comment #63) > Bonjour Alex, > J'étais parti, le coeur plein d'entrain, pour une nouvelle séance de test > sur la nouvelle mouture de ton patch (tu as dû te régaler..), mais après > mise à jour d'une master de test que j'ai sur vbox (qu'avait pas servi > depuis la dernière fois), je me retrouve avec un affichage déglingué et je > constate que koha cherche à charger les css et js en leur ajoutant un suffixe > 18.0500000 (ça ressemble à la date de mise à jour du patch ?!) : > http://preprod:8080/intranet-tmpl/prog/css/staff-global_18.0500000.css > Soit j'ai fait une grosse bourde, soit il y a quelque chose qui m'échappe > (voire même les deux d'ailleurs ). ça te parle ? > > Olivier Hello Olivier, You have to update web server's rewrite rules. See bug 20187. For apache, it is: RewriteRule ^(.*)_[0-9][0-9]\.[0-9][0-9][0-9][0-9][0-9][0-9][0-9].js$ $1.js [L] RewriteRule ^(.*)_[0-9][0-9]\.[0-9][0-9][0-9][0-9][0-9][0-9][0-9].css$ $1.css [L] Nginx: rewrite "^(.*)_[0-9]{2}\.[0-9]{7}\.(js|css)$" $1.$2; Great ! that's much better with the js ans css rewriting rules ! I could did the test again for daily, weekly and monthly periodicities. Everything is working as expected, dates are correctly skipped, permanent irregularities are kept over the renewal. Just a little thing about ergonomy of "Week(s) not published" scrolldown list : when you tick/untick a checkbox at first time, the form jumps surprisingly. Could we avoid this ? Anyway, thank you Alex and I hope that it will be OK this time. Olivier Created attachment 75721 [details] [review] Bug 17656 - Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for monthly, daily or weekly serials ( 1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 75722 [details] [review] Bug 17656 - Update irregularities on subscription renewals Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Renew subscription from 2017-10-16 - Check that you have the prediction pattern table on renwal page - Submit renewal - Check that irregularity and permanent_irregularity column have been updated. - Do the same for a monthly and weekly subscription Created attachment 75723 [details] [review] Bug 17656 - Fix QA comments - Make day and month names translatable, - Update unit tests, - Fix a typo in C4/Serials.pm (GessEndDate), - rename subtype subroutine to lengthtype - DB Change: add changes in kohastructure.sql and Bug_17656-add-permanent-irre Created attachment 75724 [details] [review] Bug 17656 - [QA followup] Remove useless description in subscription_daily_choice.inc and subscription_monthly_choice.inc Created attachment 75725 [details] [review] Bug 17656: Update irregularities on multi-renewals Created attachment 75726 [details] [review] Bug 17656: move GuessEnddate in Koha::Subscription::guess_enddate (In reply to Koha Team Lyon 3 from comment #65) > Just a little thing about ergonomy of "Week(s) not published" scrolldown > list : when you tick/untick a checkbox at first time, the form jumps > surprisingly. > Could we avoid this ? > Fixed! I went through another round of testing. Looking good now, just one (hopefully last) issue: When the subscription is based on week, you can check irregularities for all pregenerated issues - but it works only when you check weeks from first year. Probably there should be just week numbers from 1 to 52, which could be enough. (In reply to Josef Moravec from comment #73) > I went through another round of testing. Looking good now, just one > (hopefully last) issue: > When the subscription is based on week, you can check irregularities for all > pregenerated issues - but it works only when you check weeks from first year. > > Probably there should be just week numbers from 1 to 52, which could be > enough. Don't understand. Can you give me an example ? (In reply to Alex Arnaud from comment #74) > (In reply to Josef Moravec from comment #73) > > I went through another round of testing. Looking good now, just one > > (hopefully last) issue: > > When the subscription is based on week, you can check irregularities for all > > pregenerated issues - but it works only when you check weeks from first year. > > > > Probably there should be just week numbers from 1 to 52, which could be > > enough. > > Don't understand. Can you give me an example ? Here is the screenshot: https://screenshots.firefox.com/hOUt987Krzxs6uTm/localhost the other years weeks are not working - they do not set the checkboxes on previewed issues Created attachment 75953 [details] [review] Bug 17656: predication table - check all issues with same data-dow Hello Alex I tested again but I don't see any change about what Joseph Moravec told about nor about jumping checkboxes. Am I missing something ? Olivier (In reply to Koha Team Lyon 3 from comment #77) > Hello Alex > > I tested again but I don't see any change about what Joseph Moravec told > about nor about jumping checkboxes. > > Am I missing something ? > > Olivier Hello Olivier, The problem Joseph talked about as i understood it is that clicking a permanent irregularity does not check all the correponding irregularities. It checks only the first one in fact. My last fix works well for me but maybe i still missunderstand anything. Joseph, can you confirm ? For the "jumping checkboxes", i did not manage to reproduce it. Can you remind me where and how it happens ? What I understood about Joseph comment is what follows : Say we configure a weekly review from 01/01/2018 to 31/12/2018,if you set 52 issues as subscription length, it will be okay. A priori, it makes sense that you will set permanent irregularities on a single cycle/subscription (but people are strange !). If, for some reason, you put, say, 56 issues as subscription length, you will obtain as much checkboxes in the checkboxes lists. In the "weeks not published" scrolldown list, you will have four extra checkboxes (corresponding to weeks 1,2,3,4 of year 2019). If you check one of these weeks, the corresponding checkbox will not be checked on the fixed list and you risk not to notice this. It's the reason why, I guess, Joseph suggests to limit the displayed list to 52 weeks (Some year may have 53 weeks according to iso numbering. may be we can limit by end date of current year. Problem, I'm thinking that some subscription (and numbering) begin in the middle of the year and finish the year after like New York Review. as for "jumping checkboxes", It occurs in the "weeks not published" scrolldown list and only at first checkbox checking. the second checking you do, no problem. Sorry the patches could not apply, can you please rebase Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 17656 - Keep irregularities during subscription renewals Using index info to reconstruct a base tree... M C4/Serials.pm M koha-tmpl/intranet-tmpl/prog/css/staff-global.css M koha-tmpl/intranet-tmpl/prog/en/modules/serials/showpredictionpattern.tt M serials/subscription-add.pl Falling back to patching base and 3-way merge... Auto-merging serials/subscription-add.pl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/serials/showpredictionpattern.tt Auto-merging koha-tmpl/intranet-tmpl/prog/css/staff-global.css CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/css/staff-global.css Auto-merging C4/Serials.pm Failed to merge in the changes. Patch failed at 0001 Bug 17656 - Keep irregularities during subscription renewals The copy of the patch that failed is found in: /home/vagrant/kohaclone/.git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-17656---Keep-irregularities-during-subscriptio-4ETfMG.patch Major bug, can we please get a rebase? (In reply to Katrin Fischer from comment #81) > Major bug, can we please get a rebase? Still waiting and hoping for a rebase... Created attachment 102724 [details] [review] Bug 17656 - Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for monthly, daily or weekly serials ( 1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 102725 [details] [review] Bug 17656 - Update irregularities on subscription renewals Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Renew subscription from 2017-10-16 - Check that you have the prediction pattern table on renwal page - Submit renewal - Check that irregularity and permanent_irregularity column have been updated. - Do the same for a monthly and weekly subscription Created attachment 102726 [details] [review] Bug 17656 - Fix QA comments - Make day and month names translatable, - Update unit tests, - Fix a typo in C4/Serials.pm (GessEndDate), - rename subtype subroutine to lengthtype - DB Change: add changes in kohastructure.sql and Bug_17656-add-permanent-irre Created attachment 102727 [details] [review] Bug 17656 - [QA followup] Remove useless description in subscription_daily_choice.inc and subscription_monthly_choice.inc Created attachment 102728 [details] [review] Bug 17656: Update irregularities on multi-renewals Created attachment 102729 [details] [review] Bug 17656: move GuessEnddate in Koha::Subscription::guess_enddate Created attachment 102730 [details] [review] Bug 17656: predication table - check all issues with same data-dow (In reply to Katrin Fischer from comment #82) > (In reply to Katrin Fischer from comment #81) > > Major bug, can we please get a rebase? > > Still waiting and hoping for a rebase... Patches rebased Created attachment 102860 [details] [review] Bug 17656: Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for daily, weekly and monthly serials (1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 102861 [details] [review] Bug 17656: Update irregularities on subscription renewals Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Renew subscription from 2017-10-16 - Check that you have the prediction pattern table on renwal page - Submit renewal - Check that irregularity and permanent_irregularity column have been updated. - Do the same for a monthly and weekly subscription Created attachment 102862 [details] [review] Bug 17656: Fix issues found by QA - Make day and month names translatable - Update unit tests - Fix a typo in C4/Serials.pm (GessEndDate) - Rename subtype subroutine to lengthtype - DB Change: add changes in kohastructure.sql and Bug_17656-add-permanent-irregularities.perl Created attachment 102863 [details] [review] Bug 17656: (QA follow-up) Remove useless description in subscription_daily_choice.inc and subscription_monthly_choice.inc Created attachment 102864 [details] [review] Bug 17656: Update irregularities on multi-renewals Created attachment 102865 [details] [review] Bug 17656: Move GuessEnddate in Koha::Subscription::guess_enddate Created attachment 102866 [details] [review] Bug 17656: Predication table - check all issues with same data-dow Created attachment 102867 [details] [review] Bug 17656: (follow-up) Add missing filters Hi Alex, I've cleaned up the commit messages and added the missing filters to make the QA tools happy, but I got 2 remaining issues in the QA script: FAIL serials/subscription-renew.pl FAIL valid "my" variable $subscription masks earlier declaration in same scope FAIL koha-tmpl/intranet-tmpl/prog/en/modules/serials/showpredictionpattern.tt OK filters FAIL forbidden patterns forbidden pattern: Attribute type should not be used for script tags (bug 20053) (line 9) Could you have a look at those please? Created attachment 102960 [details] [review] Bug 17656: Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for daily, weekly and monthly serials (1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 102961 [details] [review] Bug 17656: Update irregularities on subscription renewals Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Renew subscription from 2017-10-16 - Check that you have the prediction pattern table on renwal page - Submit renewal - Check that irregularity and permanent_irregularity column have been updated. - Do the same for a monthly and weekly subscription Created attachment 102962 [details] [review] Bug 17656: Fix issues found by QA - Make day and month names translatable - Update unit tests - Fix a typo in C4/Serials.pm (GessEndDate) - Rename subtype subroutine to lengthtype - DB Change: add changes in kohastructure.sql and Bug_17656-add-permanent-irregularities.perl Created attachment 102963 [details] [review] Bug 17656: (QA follow-up) Remove useless description in subscription_daily_choice.inc and subscription_monthly_choice.inc Created attachment 102964 [details] [review] Bug 17656: Update irregularities on multi-renewals Created attachment 102965 [details] [review] Bug 17656: Move GuessEnddate in Koha::Subscription::guess_enddate Created attachment 102966 [details] [review] Bug 17656: Predication table - check all issues with same data-dow Created attachment 102967 [details] [review] Bug 17656: (follow-up) Add missing filters Created attachment 102968 [details] [review] Bug 17656: fix QA tools warnings (In reply to Katrin Fischer from comment #99) > Hi Alex, > > I've cleaned up the commit messages and added the missing filters to make > the QA tools happy, but I got 2 remaining issues in the QA script: > > FAIL serials/subscription-renew.pl > FAIL valid > "my" variable $subscription masks earlier declaration in same scope > > FAIL > koha-tmpl/intranet-tmpl/prog/en/modules/serials/showpredictionpattern.tt > OK filters > FAIL forbidden patterns > forbidden pattern: Attribute type should not be used for script tags (bug > 20053) (line 9) > > Could you have a look at those please? Katrin, QA warnings are fixed. Also, i did a tiny rebase again. Thx! Patch no longer applies 8-(: Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 17656: Keep irregularities during subscription renewals Using index info to reconstruct a base tree... M C4/Serials.pm M koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss M koha-tmpl/intranet-tmpl/prog/css/staff-global.css Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/css/staff-global.css CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/css/staff-global.css Auto-merging koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss Auto-merging C4/Serials.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 17656: Keep irregularities during subscription renewals Created attachment 103623 [details] [review] Bug 17656: Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for daily, weekly and monthly serials (1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 103624 [details] [review] Bug 17656: Update irregularities on subscription renewals Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Renew subscription from 2017-10-16 - Check that you have the prediction pattern table on renwal page - Submit renewal - Check that irregularity and permanent_irregularity column have been updated. - Do the same for a monthly and weekly subscription Created attachment 103625 [details] [review] Bug 17656: Fix issues found by QA - Make day and month names translatable - Update unit tests - Fix a typo in C4/Serials.pm (GessEndDate) - Rename subtype subroutine to lengthtype - DB Change: add changes in kohastructure.sql and Bug_17656-add-permanent-irregularities.perl Created attachment 103626 [details] [review] Bug 17656: (QA follow-up) Remove useless description in subscription_daily_choice.inc and subscription_monthly_choice.inc Created attachment 103627 [details] [review] Bug 17656: Update irregularities on multi-renewals Created attachment 103628 [details] [review] Bug 17656: Move GuessEnddate in Koha::Subscription::guess_enddate Created attachment 103629 [details] [review] Bug 17656: Predication table - check all issues with same data-dow Created attachment 103630 [details] [review] Bug 17656: (follow-up) Add missing filters Created attachment 103631 [details] [review] Bug 17656: fix QA tools warnings (In reply to David Nind from comment #110) > Patch no longer applies 8-(: > > Apply? [(y)es, (n)o, (i)nteractive] y > Applying: Bug 17656: Keep irregularities during subscription renewals > Using index info to reconstruct a base tree... > M C4/Serials.pm > M koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss > M koha-tmpl/intranet-tmpl/prog/css/staff-global.css > Falling back to patching base and 3-way merge... > Auto-merging koha-tmpl/intranet-tmpl/prog/css/staff-global.css > CONFLICT (content): Merge conflict in > koha-tmpl/intranet-tmpl/prog/css/staff-global.css > Auto-merging koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss > Auto-merging C4/Serials.pm > error: Failed to merge in the changes. > Patch failed at 0001 Bug 17656: Keep irregularities during subscription > renewals Patches rebased Created attachment 103657 [details] [review] Bug 17656: Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for daily, weekly and monthly serials (1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 103658 [details] [review] Bug 17656: Update irregularities on subscription renewals Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Renew subscription from 2017-10-16 - Check that you have the prediction pattern table on renwal page - Submit renewal - Check that irregularity and permanent_irregularity column have been updated. - Do the same for a monthly and weekly subscription Signed-off-by: David Nind <david@davidnind.com> Created attachment 103659 [details] [review] Bug 17656: Fix issues found by QA - Make day and month names translatable - Update unit tests - Fix a typo in C4/Serials.pm (GessEndDate) - Rename subtype subroutine to lengthtype - DB Change: add changes in kohastructure.sql and Bug_17656-add-permanent-irregularities.perl Signed-off-by: David Nind <david@davidnind.com> Created attachment 103660 [details] [review] Bug 17656: (QA follow-up) Remove useless description in subscription_daily_choice.inc and subscription_monthly_choice.inc Signed-off-by: David Nind <david@davidnind.com> Created attachment 103661 [details] [review] Bug 17656: Update irregularities on multi-renewals Signed-off-by: David Nind <david@davidnind.com> Created attachment 103662 [details] [review] Bug 17656: Move GuessEnddate in Koha::Subscription::guess_enddate Signed-off-by: David Nind <david@davidnind.com> Created attachment 103663 [details] [review] Bug 17656: Predication table - check all issues with same data-dow Signed-off-by: David Nind <david@davidnind.com> Created attachment 103664 [details] [review] Bug 17656: (follow-up) Add missing filters Signed-off-by: David Nind <david@davidnind.com> Created attachment 103665 [details] [review] Bug 17656: fix QA tools warnings Signed-off-by: David Nind <david@davidnind.com> Created attachment 103667 [details] Bug 17656 - Image of renewal form (In reply to Alex Arnaud from comment #120) > Patches rebased Thanks Alex! Everything seems to work as per the test plan. I've signed-off, but I'm not sure that the renewal form displays correctly - see screenshot attached. Looking here FAIL installer/data/mysql/atomicupdate/Bug_17656-add-permanent-irregularities.sql FAIL git manipulation The file has been added and deleted in the same patchset - $periodicity, $firstacquidate, $irregularity, $numberpattern, $locale, - $numberlength, $weeklength, $monthlength, $lastvalue1, $innerloop1, - $lastvalue2, $innerloop2, $lastvalue3, $innerloop3, $status, - $biblionumber, $callnumber, $notes, $letter, $manualhistory, - $internalnotes, $serialsadditems, $staffdisplaycount, $opacdisplaycount, - $graceperiod, $location, $enddate, $subscriptionid, $skip_serialseq, - $itemtype, $previousitemtype, $mana_id + $periodicity, $firstacquidate, $irregularity, $permanent_irregularity, + $numberpattern, $locale, $numberlength, $weeklength, $monthlength, + $lastvalue1, $innerloop1, $lastvalue2, $innerloop2, $lastvalue3, + $innerloop3, $status, $biblionumber, $callnumber, $notes, $letter, + $manualhistory, $internalnotes, $serialsadditems, $staffdisplaycount, + $opacdisplaycount, $graceperiod, $location, $enddate, $subscriptionid, + $skip_serialseq, $itemtype, $previousitemtype, $mana_id Doing things like this does not life make easier for QA. Do I now have to check if every parameter is still there? You could have added the permanent_irregularity without touching all the other lines. And you can still fix it too? Same remark for this: - librarian => $auser, - branchcode => $branchcode, - aqbooksellerid => $aqbooksellerid, - cost => $cost, - aqbudgetid => $aqbudgetid, - biblionumber => $biblionumber, - startdate => $startdate, - periodicity => $periodicity, - numberlength => $numberlength, - weeklength => $weeklength, - monthlength => $monthlength, - lastvalue1 => $lastvalue1, - innerloop1 => $innerloop1, - lastvalue2 => $lastvalue2, - innerloop2 => $innerloop2, - lastvalue3 => $lastvalue3, - innerloop3 => $innerloop3, - status => $status, - notes => $notes, - letter => $letter, - firstacquidate => $firstacquidate, - irregularity => $irregularity, - numberpattern => $numberpattern, - locale => $locale, - callnumber => $callnumber, - manualhistory => $manualhistory, - internalnotes => $internalnotes, - serialsadditems => $serialsadditems, - staffdisplaycount => $staffdisplaycount, - opacdisplaycount => $opacdisplaycount, - graceperiod => $graceperiod, - location => $location, - enddate => $enddate, - skip_serialseq => $skip_serialseq, - itemtype => $itemtype, - previousitemtype => $previousitemtype, - mana_id => $mana_id, + librarian => $auser, + branchcode => $branchcode, + aqbooksellerid => $aqbooksellerid, + cost => $cost, + aqbudgetid => $aqbudgetid, + biblionumber => $biblionumber, + startdate => $startdate, + periodicity => $periodicity, + numberlength => $numberlength, etc +=head2 GessEnddate + +my $enddate = GessEnddate($startdate_iso $frequencyid, $numberlength, $weeklength, $monthlength); + +=cut + +sub GuessEnddate { The routine name should obviously be the same in POD and code. (In reply to Marcel de Rooy from comment #135) > +=head2 GessEnddate > + > +my $enddate = GessEnddate($startdate_iso $frequencyid, $numberlength, > $weeklength, $monthlength); > + > +=cut > + > +sub GuessEnddate { > > The routine name should obviously be the same in POD and code. Seeing a correction now. Reason for some squashing ? You add a routine to DateUtils called issue_number: +my $week_number = issue_number('2017-01-01', 'week'); What you actually want is a day-of-week, week number or month. The name is confusing. I do not expect an issue_number in DateUtils. The POD is wrong since you expect a dt and raise an exception on a string. + return Day_of_Week(split /-/, $date->ymd); Please consider using a DateTime function here: $dt->day_of_week() $dt->wday() and $dt->dow(). $dt->local_day_of_week() Actually $dt->dow is shorter than issue_number($dt, 'week') + return $date->month; In this case it is more work to call issue_number($dt, 'month') than to say $dt->month. + my $dow = issue_number( dt_from_string($date), $frequency->unit ); This is a bit confusing. If we do not know the unit, how could we save it into dow (day of week) ? Rename ? I recommend to remove issue_number from DateUtils. We could move it to Serials? It is a thin wrapper to three DateTime methods. If you want to keep it in DateUtils, we should come up with a convincing name. +my $subscription_o = Koha::Subscriptions->find($subscription->{subscriptionid}); +my @irreg = $subscription_o->irregularities; +is($irreg[0], 2, 'First element of irregularities is 2'); +is($irreg[1], 4, 'Second element of irregularities is 4'); +is($irreg[2], 7, 'Third element of irregularities is 7'); + +my @permanent_irreg = $subscription_o->permanent_irregularities; +is($permanent_irreg[0], 1, 'First element of permanent_irregularities is 1'); +is($permanent_irreg[1], 2, 'Second element of permanent_irregularities is 2'); Do we really need this? You add a separate script here to test if split /;/ works ? I appreciate your willingness to test, but I have the impression that we should focus here on complex exceptions with various frequencies, different months, leap years, etc. Think of various date peculiarities. Elaborating on that, t/db_dependent/Koha/Subscription/Irregularities.t contains a lot of code, but actually only one test. + is($irregularities, '10;11', 'Irregularities have been updated'); Since this looks like the most important test, I would really expect more testing here. Maybe you should think of some intelligent mocks here to reduce a lot of building calls? Since we already got into string freeze, I leave it for now. Please resubmit your work in some squashed way. Perhaps divide the module changes, tests, script and template changes, and db revisions over separate patches. Thanks ! Created attachment 117789 [details] [review] Bug 17656: Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for daily, weekly and monthly serials (1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: David Nind <david@davidnind.com> Rebased - 2021-03-04 - Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 117790 [details] [review] Bug 17656: (follow-up) Add missing filters Signed-off-by: David Nind <david@davidnind.com> Created attachment 117791 [details] [review] Bug 17656: [DO NOT PUSH] provide schema file Created attachment 117792 [details] [review] Bug 17656: [DO NOT PUSH] provide css file Patches rebased and squashed. @Marcel, I took your QA remarks into account. CSS is already conflicting - I will leave this patch off for testing running yarn build locally. I am sorry, but there are quite some QA fails here :( It looks like the first could be a rebase issue? FAIL C4/Serials.pm FAIL valid Constant subroutine C4::Serials::MISSING_LOST redefined Constant subroutine C4::Serials::MISSING_NEVER_RECIEVED redefined Constant subroutine C4::Serials::STOPPED redefined Constant subroutine C4::Serials::DELETED redefined Constant subroutine C4::Serials::MISSING_DAMAGED redefined Constant subroutine C4::Serials::EXPECTED redefined Constant subroutine C4::Serials::LATE redefined Constant subroutine C4::Serials::MISSING_SOLD_OUT redefined Constant subroutine C4::Serials::ARRIVED redefined Constant subroutine C4::Serials::NOT_ISSUED redefined Constant subroutine C4::Serials::CLAIMED redefined Constant subroutine C4::Serials::MISSING redefined FAIL t/Serials/IssueNumber.t OK critic FAIL file permissions File must have the exec flag FAIL t/db_dependent/Koha/Subscription/GuessEnddate.t OK critic FAIL file permissions File must have the exec flag FAIL t/db_dependent/Koha/Subscription/Irregularities.t OK critic FAIL file permissions File must have the exec flag FAIL t/db_dependent/Serials/SubscriptionIrregularity.t OK critic FAIL file permissions File must have the exec flag Created attachment 117846 [details] [review] Bug 17656: [DO NOT PUSH] provide css file Created attachment 117851 [details] [review] Bug 17656: QA warnings fixes Created attachment 117852 [details] [review] Bug 17656: QA warnings fixes Created attachment 118856 [details] [review] Bug 17656: QA warnings fixes Signed-off-by: Amarangi <amarangikarunathilaka@gmail.com> Hello Alex, Sorry but when I record a subscription after adding irregularities via Test prediction pattern and I click on Save Subscription button, I get this error : The method Koha::Subscriptions->startdate is not covered by tests! I tried to create a new subs and that time it was OK. But when I create another sub for testing another type of frequency I got this error again. Olivier Crouzet Some more details : I tried a daily frequency with the following data : 05/04/2021 to 11/04/2021 (french notation = 5 april 2021 to 11 april 2021) first issue 05/04/2021 subs start : idem issues 7 subs end : 11/04/2021 wednesday and thursday are checked as irregulars multireceiving 5 items renewing : wednesday and thursday are still checked. That's OK but when I submit the form, the following message is displayed : "Renewing is cancelled, contact the administrator" Going back to Editing subscription, in Test pattern it's said : End date is not consistent with subscription length. Indeed end date is remained on 11/04/2021 I decided to delete enddate and let it making itself but when saving I get again the error message "The method Koha::Subscriptions->startdate is not covered by tests!" Olivier Created attachment 119106 [details] [review] Bug 17656: use Koha::Subscription->startdate instead of Koha::Subscriptions Hello, I testes this time on Ptfx Sandbox. no more message "Renewal cancelled". Sorry, I think this was template problem on my instance. It's OK now for startdate error message. Everything works as expected as for daily and monthly frequencies. Exceptions are kept. date are skipped when receiving as well. But unfortunately weekly frequency does not work. Irregularities are not retained : my parameters : first issue publ. date 01/01/2021 startdate 01/01/2021 issues 52 enddate 31/12/2021 I checked 8 and 15 feb 2021 They are recorded ok I multireceived 50 issues and click on Renew button but no weeks are no more checked on the form. Olivier Thx for all the testing going on here! (In reply to Koha Team University Lyon 3 from comment #154) > > I checked 8 and 15 feb 2021 > They are recorded ok > I multireceived 50 issues and click on Renew button but no weeks are no more > checked on the form. > > Olivier Olivier, Did you check the weeks in the list above the prediction table ? Doing so works for me. If you check weeks inside the prediction table (column "Not published"), they are not saved as permanent irregularities but as irregularities. Oh ! stupid ! I did'nt get it !. I think I only checked the flat list below. I redo the test checking the irregularities in the choice list and you are right it-s OK : this time irregularities are kept. I will sign off but I have 2 last suggestions : I think all the same, the display for weeks type is little ambiguous : The presence of 2 wordings very close can be troubling Please check the issues that are NOT published (irregularities) and below : Choice the weeks in the year where the issues are not published May be, in this case, the second is redundant. I would suggest to suppress the second and make the first like this : Please check in the dropdown list, the issues that are NOT published (irregularities) and last thing : the "Weeks(s)not_published" form is jumping when we check a checkbox. Could you add a "return false" to "Check_permanent_irregularities" function to prevent this ? Olivier Created attachment 120863 [details] [review] Bug 17656: More meaningful messages in prediction pattern table (In reply to Koha Team University Lyon 3 from comment #157) > Please check the issues that are NOT published (irregularities) > and below : > Choice the weeks in the year where the issues are not published > > May be, in this case, the second is redundant. I would suggest to suppress > the second and make the first like this : > Please check in the dropdown list, the issues that are NOT published > (irregularities) > > and last thing : the "Weeks(s)not_published" form is jumping when we check a > checkbox. Could you add a "return false" to "Check_permanent_irregularities" > function to prevent this ? > > Olivier Olivier, Please test my proposal for meaningful messages. Also, fixed the jump on top of the page. Hey Alex, Great ! That's perfect. Hope this patch will be in master soon, what's the english for "je croise les doigt" ? fingers crossed ? Olivier (In reply to Koha Team University Lyon 3 from comment #160) > Hope this patch will be in master soon, what's the english for "je croise > les doigt" ? fingers crossed ? yes ^^" Too bad I hit an issue when testing however :o (In reply to Koha Team University Lyon 3 from comment #151) > The method Koha::Subscriptions->startdate is not covered by tests! same! details - rebuilt schema files[1] - new subscription - record 146 from sample data - first publication date: 05/02/2021 - Frequency 1/day - start date 05/02/2021 - numbering pattern number - Subscription length: 7 - not published: 05/04/2021 & 05/05/2021 - save The method Koha::Subscriptions->startdate is not covered by tests! [1] dbic command from koha-testing-docker mysql -h db -ppassword -uroot -e 'DROP DATABASE IF EXISTS dbic; CREATE DATABASE dbic' cd '/kohadevbox/koha' mysql -h db -ppassword -uroot dbic < installer/data/mysql/kohastructure.sql misc/devel/update_dbix_class_files.pl --db_name dbic --db_host db --db_user root --db_passwd password Did I do something wrong? Reverted to "need signoff" so it's not in the signoff queue. Which would cause someone from QA to look at it, pass it and don't look a it later. Created attachment 121082 [details] [review] Bug 17656: Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for daily, weekly and monthly serials (1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: David Nind <david@davidnind.com> Rebased - 2021-03-04 - Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 121083 [details] [review] Bug 17656: (follow-up) Add missing filters Signed-off-by: David Nind <david@davidnind.com> Created attachment 121084 [details] [review] Bug 17656: [DO NOT PUSH] provide schema file Created attachment 121085 [details] [review] Bug 17656: [DO NOT PUSH] provide css file Created attachment 121086 [details] [review] Bug 17656: QA warnings fixes Signed-off-by: Amarangi <amarangikarunathilaka@gmail.com> Created attachment 121087 [details] [review] Bug 17656: use Koha::Subscription->startdate instead of Koha::Subscriptions Created attachment 121088 [details] [review] Bug 17656: More meaningful messages in prediction pattern table (In reply to Victor Grousset/tuxayo from comment #161) > > The method Koha::Subscriptions->startdate is not covered by tests! > Victor, it's ok now. I squashed the fix in "use Koha::Subscription->startdate instead of Koha::Subscriptions" patch. It's the same problem in other place. Patches rebased. Created attachment 121192 [details] [review] Bug 17656: Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for daily, weekly and monthly serials (1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: David Nind <david@davidnind.com> Rebased - 2021-03-04 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 121193 [details] [review] Bug 17656: (follow-up) Add missing filters Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 121194 [details] [review] Bug 17656: [DO NOT PUSH] provide schema file Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 121195 [details] [review] Bug 17656: [DO NOT PUSH] provide css file Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 121196 [details] [review] Bug 17656: QA warnings fixes Signed-off-by: Amarangi <amarangikarunathilaka@gmail.com> Created attachment 121197 [details] [review] Bug 17656: use Koha::Subscription->startdate instead of Koha::Subscriptions Created attachment 121198 [details] [review] Bug 17656: More meaningful messages in prediction pattern table It works :) Thanks for the fix Created attachment 121296 [details] [review] Bug 17656: Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for daily, weekly and monthly serials (1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: David Nind <david@davidnind.com> Rebased - 2021-03-04 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 121297 [details] [review] Bug 17656: (follow-up) Add missing filters Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 121298 [details] [review] Bug 17656: [DO NOT PUSH] provide schema file Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 121299 [details] [review] Bug 17656: [DO NOT PUSH] provide css file Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 121300 [details] [review] Bug 17656: QA warnings fixes Signed-off-by: Amarangi <amarangikarunathilaka@gmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 121301 [details] [review] Bug 17656: use Koha::Subscription->startdate instead of Koha::Subscriptions Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 121302 [details] [review] Bug 17656: More meaningful messages in prediction pattern table Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> For some reason not all commits had the signed off mark. I confirm having tested with all applied. Adds unused function _get_sub_length which should be removed. Doesn't apply, please remove unused function in comment 186 Created attachment 128489 [details] [review] Bug 17656: Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for daily, weekly and monthly serials (1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: David Nind <david@davidnind.com> Rebased - 2021-03-04 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 128491 [details] [review] Bug 17656: (follow-up) Add missing filters Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 128492 [details] [review] Bug 17656: [DO NOT PUSH] provide schema file Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 128493 [details] [review] Bug 17656: [DO NOT PUSH] provide css file Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 128494 [details] [review] Bug 17656: QA warnings fixes Signed-off-by: Amarangi <amarangikarunathilaka@gmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 128495 [details] [review] Bug 17656: use Koha::Subscription->startdate instead of Koha::Subscriptions Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 128496 [details] [review] Bug 17656: More meaningful messages in prediction pattern table Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 128498 [details] [review] Bug 17656: Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for daily, weekly and monthly serials (1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: David Nind <david@davidnind.com> Rebased - 2021-03-04 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 128499 [details] [review] Bug 17656: (follow-up) Add missing filters Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 128500 [details] [review] Bug 17656: [DO NOT PUSH] provide schema file Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 128501 [details] [review] Bug 17656: [DO NOT PUSH] provide css file Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 128502 [details] [review] Bug 17656: QA warnings fixes Signed-off-by: Amarangi <amarangikarunathilaka@gmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 128503 [details] [review] Bug 17656: use Koha::Subscription->startdate instead of Koha::Subscriptions Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 128504 [details] [review] Bug 17656: More meaningful messages in prediction pattern table Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Patchs rebased Patch doesn't apply : <pre> root@kohadevbox:koha(bz17656)$ git bz apply 17656 Bug 17656 - Irregularities in serial prediction pattern are planned only for current subscription 128498 - Bug 17656: Keep irregularities during subscription renewals 128499 - Bug 17656: (follow-up) Add missing filters 128500 - Bug 17656: [DO NOT PUSH] provide schema file 128501 - Bug 17656: [DO NOT PUSH] provide css file 128502 - Bug 17656: QA warnings fixes 128503 - Bug 17656: use Koha::Subscription->startdate instead of Koha::Subscriptions 128504 - Bug 17656: More meaningful messages in prediction pattern table Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 17656: Keep irregularities during subscription renewals Using index info to reconstruct a base tree... M C4/Serials.pm M installer/data/mysql/kohastructure.sql M koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss M koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt M koha-tmpl/intranet-tmpl/prog/js/subscription-add.js M serials/subscription-add.pl Falling back to patching base and 3-way merge... Auto-merging serials/subscription-add.pl Auto-merging koha-tmpl/intranet-tmpl/prog/js/subscription-add.js CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/js/subscription-add.js Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt Auto-merging koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss Auto-merging installer/data/mysql/kohastructure.sql Auto-merging C4/Serials.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 17656: Keep irregularities during subscription renewals hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-17656-Keep-irregularities-during-subscription--JneHV9.patch </pre> Created attachment 132412 [details] [review] Bug 17656: Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for daily, weekly and monthly serials (1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: David Nind <david@davidnind.com> Rebased - 2021-03-04 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 132413 [details] [review] Bug 17656: (follow-up) Add missing filters Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 132415 [details] [review] Bug 17656: [DO NOT PUSH] provide schema file Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 132416 [details] [review] Bug 17656: QA warnings fixes Signed-off-by: Amarangi <amarangikarunathilaka@gmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 132417 [details] [review] Bug 17656: use Koha::Subscription->startdate instead of Koha::Subscriptions Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 132418 [details] [review] Bug 17656: More meaningful messages in prediction pattern table Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 132447 [details] [review] Bug 17656: fix issue_number call in showprediction.pl Created attachment 132448 [details] [review] Bug 17656: (follow-up) Fix typo for QA I've got an error trace on cgi-bin/koha/serials/subscription-renew.pl?mode=popup&subscriptionid=1 when I try to renew the subscription. See the full error in text document attached. Created attachment 132692 [details]
Error trace when renew a subscription
Created attachment 132779 [details] [review] Bug 17656: (follow-up) Update DBIC + fix missing C4::Serials functions needed I still have an error trace, this time when I save the subscription. With the patch applies, the option to set 'regular irregularities' for daily (text + checkbox) are not displayed anymore. It didn't happen the first time I test the patch. Up ^^ Can't believe we still haven't fixed this :( Created attachment 177497 [details] [review] Bug 17656: Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for daily, weekly and monthly serials (1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: David Nind <david@davidnind.com> Rebased - 2021-03-04 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 177498 [details] [review] Bug 17656: (follow-up) Add missing filters Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 177499 [details] [review] Bug 17656: QA warnings fixes Signed-off-by: Amarangi <amarangikarunathilaka@gmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 177500 [details] [review] Bug 17656: use Koha::Subscription->startdate instead of Koha::Subscriptions Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 177501 [details] [review] Bug 17656: More meaningful messages in prediction pattern table Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 177502 [details] [review] Bug 17656: fix issue_number call in showprediction.pl Created attachment 177503 [details] [review] Bug 17656: (follow-up) Fix typo for QA Created attachment 177504 [details] [review] Bug 17656: (follow-up) Update DBIC + fix missing C4::Serials functions needed Created attachment 177505 [details] [review] Bug 17656: (QA follow-up) fix compile errors woow, had to dig to bring that one back from the grave! I've just made a small patch to fix one or two compile error and fix rebase conflicts. I didn't include the Koha Schema update, but they are supposed to be generated when the test plan is run. Hope this one will finally be merged! Arthur Hi Arthur, just saw this in my emails and was going to put an RM_priority on it, but it already has one. Would love to see this long standing problem resolved. (In reply to Katrin Fischer from comment #228) > Hi Arthur, just saw this in my emails and was going to put an RM_priority on > it, but it already has one. Would love to see this long standing problem > resolved. Hi Katrin, Sorry that this patch lacked my attention for such a long time. I've tried to fix things in such a way that it is now applicable on current main + possible to test on KTD. I was able to run/edit/save subscriptions on KTD, as far as I could tell it still works if you update database / generate the schema / update dbix files as it is described in the test plan. Best, Arthur No worries - I was just trying to give this a little RM "push"! Created attachment 177663 [details] [review] Bug 17656: Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for daily, weekly and monthly serials (1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: David Nind <david@davidnind.com> Rebased - 2021-03-04 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Nind <david@davidnind.com> Created attachment 177664 [details] [review] Bug 17656: (follow-up) Add missing filters Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Nind <david@davidnind.com> Created attachment 177665 [details] [review] Bug 17656: QA warnings fixes Signed-off-by: Amarangi <amarangikarunathilaka@gmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Nind <david@davidnind.com> Created attachment 177666 [details] [review] Bug 17656: use Koha::Subscription->startdate instead of Koha::Subscriptions Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Nind <david@davidnind.com> Created attachment 177667 [details] [review] Bug 17656: More meaningful messages in prediction pattern table Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Nind <david@davidnind.com> Created attachment 177668 [details] [review] Bug 17656: fix issue_number call in showprediction.pl Signed-off-by: David Nind <david@davidnind.com> Created attachment 177669 [details] [review] Bug 17656: (follow-up) Fix typo for QA Signed-off-by: David Nind <david@davidnind.com> Created attachment 177670 [details] [review] Bug 17656: (follow-up) Update DBIC + fix missing C4::Serials functions needed Signed-off-by: David Nind <david@davidnind.com> Created attachment 177671 [details] [review] Bug 17656: (QA follow-up) fix compile errors Signed-off-by: David Nind <david@davidnind.com> I've tested, and I think that everything works as expected, so I have signed off. However, it may pay for someone who is more familiar with serials to add their signoff as well. Created attachment 177831 [details] [review] Bug 17656: Keep irregularities during subscription renewals When renewing a subscription, irregularities are lost. This patch allows to keep them for daily, weekly and monthly serials (1 unit per issue only). Test plan: - Apply this patch, - Launch installer/data/mysql/updatedatabase.pl - Launch misc/devel/update_dbix_class_files.pl - Create a daily subscription. i.e: - From: 2017-10-09, To: 2017-10-15, - Frequency: 1/day, - 7 issues, - test prediction pattern - check Wednesday and Thursday as irregularity, - save - Edit subscription, - Check that Wednesday and Thursday are still checked - Renew subscription from 2017-10-16 - Edit subscription, - Make sure that Wednesday and Thursday are still checked with corresponding serials - Do the same for a monthly and weekly subscription Rebased - 2017-12-15 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: David Nind <david@davidnind.com> Rebased - 2021-03-04 - Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Nind <david@davidnind.com> Created attachment 177832 [details] [review] Bug 17656: (follow-up) Add missing filters Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Nind <david@davidnind.com> Created attachment 177833 [details] [review] Bug 17656: QA warnings fixes Signed-off-by: Amarangi <amarangikarunathilaka@gmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Nind <david@davidnind.com> Created attachment 177834 [details] [review] Bug 17656: use Koha::Subscription->startdate instead of Koha::Subscriptions Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Nind <david@davidnind.com> Created attachment 177835 [details] [review] Bug 17656: More meaningful messages in prediction pattern table Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Nind <david@davidnind.com> Created attachment 177836 [details] [review] Bug 17656: fix issue_number call in showprediction.pl Signed-off-by: David Nind <david@davidnind.com> Created attachment 177837 [details] [review] Bug 17656: (follow-up) Fix typo for QA Signed-off-by: David Nind <david@davidnind.com> Created attachment 177838 [details] [review] Bug 17656: (follow-up) Update DBIC + fix missing C4::Serials functions needed Signed-off-by: David Nind <david@davidnind.com> Created attachment 177839 [details] [review] Bug 17656: (QA follow-up) fix compile errors Signed-off-by: David Nind <david@davidnind.com> Tricky rebase, please retest. I tried on that sandbox : https://staff-bz17656.sandboxes.ptfs-europe.co.uk/ When I created a subscription, it was impossible to choose a numbering pattern (no choices in the dropdown menu) It does not seem to be an issue on other sandbox, so probably caused by this patch I attempted to re-test using KTD and got the same: no number pattern options (just -- please choose --). Without the patch, the numbering pattern options are there. I was able to fix the numberingpattern issue by adding "[% numberpattern.label | html %] in the tt. However I still have an issue with renewals. when i renew a subscription and change startdate and number of issues for the renewal, it won't automatically calculate the prediction pattern which is displayed. Probably a JS issue which I still have to work on. Created attachment 178142 [details]
Screenshot
Describes the current issue I have:
the start date for subscription renewal is before even the start date of the previous subscription (seems beginning of the year).
Doesn't really feel very right to me but that may be normal.
The prediction pattern block do not update dates nor checkboxes when I change the start_date renewal or the number of issues for the renewal.
When it is first loaded, not all checkboxes are checked...
Here, I'd say that issues for the 03/01 and 03/02 should be checked, it is not...
If I uncheck then re-check the permanent irregularities (wednesday and thursday) it is checked by JS though.
|