Created attachment 8110 [details] 'New subscription' page This development made by BibLibre improve management of numbering patterns and frequencies for subscriptions. It add the possibility to create (and of course edit and delete) numbering patterns and frequencies in two new pages. When you create a new subscription (or edit an existing one) you can change easily the numbering pattern (and the frequency) with a drop-down list. It also change the way the prediction pattern is calculated. It's no more calculated client-side, but server-side, with a Perl script called by ajax. That way it is more consistent because the Perl script use the same functions Koha use to determine the next serial number. (see screenshot attached)
*** Bug 5351 has been marked as a duplicate of this bug. ***
Created attachment 8941 [details] [review] Database files kohastructure.sql and updatedatabase.pl changes + sample frequencies data and sample numberpatterns data for fresh installs (sample data is included in updatedatabase.pl)
Created attachment 8942 [details] [review] Change subscription numbering pattern and frequencies Serials numbering pattern and frequencies are no more hard-coded. Now it's possible to create, edit and delete numbering patterns (and frequencies). This implies new sql tables (subscription_numberpatterns and subscription_frequencies) Numbering patterns behave almost as before, there are still the same values to configure (addX, everyX, settoX, whenmorethanX). lastvalueX and innerloopX remain in subscription tables. There is a new value in numbering patterns: numberingX. For each "column" (X, Y or Z) you can tell how to format the number. Actually numberingX can be set to: - 'dayname' (name of the day) (0-6 or 1-7 depending on which day is the first of the week) - 'monthname' (name of the month) (0-11) - 'season' (name of the season) (0-3) (0 is Spring) These names are localized by using POSIX::setlocale and POSIX::strftime and setting a 'locale' value to the subscription. Locale have to be installed on the system. Frequencies can be configured using 3 parameters: - 'unit': one of 'day', 'week', 'month', 'year' - 'issuesperunit': integer >= 1, the number of received issues per 'unit' - 'unitsperissue': integer >= 1, the number of 'unit' between two issues One of 'issuesperunit' and 'unitsperissue' must be equal to 1. Examples: unit = 'day', issuesperunit=3, unitsperissue=1 => 3 issues per day unit = 'week', issuesperunit=1, unitsperissue=3 => 1 issue each 3 weeks Prediction pattern is now computed server-side and is more consistent with what Koha will do. The publication date is displayed alongside the serial number. Irregularities can now be checked one by one, in the prediction pattern table, or if frequency is 'day-based' (unit is 'day'), there is the possibility to check all issues for a week day at once. When an irregularity is found, there is the possibility to keep the serial number unchanged, or to skip it. It is configured at subscription creation or modification. For instance, with a daily subscription you can have: skip serial number | keep serial number ----------------------+---------------------- 2012-01-01 ¦ No 1 | 2012-01-01 ¦ No 1 2012-01-03 ¦ No 3 | 2012-01-03 ¦ No 2 To lighten the subscription modification page, manual history has been moved in its own page subscription-history.pl which is accessible on subscription-detail.pl, tab 'Planning'. Important note: updatedatabase.pl script takes into account existing subscriptions and create a numbering pattern for each one. Frequency is mapped to the correct entry in subscription_frequencies table
Created attachment 8943 [details] Test plan
This is interesting and very needed. I would like to test it, but patch does not apply any more.
I agree with Marijana :)
Created attachment 10145 [details] [review] Database files Rebased on master
Created attachment 10146 [details] [review] Change subscription numbering pattern and frequencies Rebased on master and fixed two minor things that stopped working with new version of jQuery.
Created attachment 10468 [details] [review] Database files Rebased on master
Created attachment 10469 [details] [review] Database files Bad manipulation, this one is the rebased patch.
Created attachment 10470 [details] [review] Change subscription numbering pattern and frequencies Rebased on master
Applying: Bug 7688: Change subscription numbering pattern and frequencies fatal: sha1 information is lacking or useless (installer/data/mysql/kohastructure.sql). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge.
I will just add you test plan directly as a comment, rather than as an attachment, and reformat it, for readability.
Test plan for Bug 7688: Create a new subscription: - Go to Serials module and click "New subscription" button - On the first page, choose a biblio and click next to go to the second page - Pick a first issue publication date - Choose frequency '1/day' - Choose a subscription length of 15 issues - Choose a subscription start date - Choose numbering pattern 'Volume, Number' - A table appears, fill 'Begins with' cells with '1' - Click on 'Test prediction pattern' button The prediction pattern is displayed at the right of the page. You can see in it the serial number, the publication date and a checkbox to allow you to choose which serials will not be received (irregularities). You can see that serial number start from "Vol 1, No 1" continue to "Vol 1, No 12" and then restart with "Vol 2, No 1". Frequency is '1/day' so you can see that publication date is incremented by one day line after line. - Now you can play a little with frequencies and numbering patterns, change one of them (or both) and click again on 'Test prediction pattern' - For example, choose frequency '3/weeks' and click on 'Test prediction pattern' button'. There is a little behaviour change compared with current master. Publication date will not be guessed within the week. Koha can't know when you will receive issues. So the publication date stay the same (monday of each week) for 3 consecutive issues and then jump to the next week. - Now choose frequency '1/3 months' and numbering pattern 'Seasonal' - Fill 'Begins with' cells with '2012' for Year and '0' for Season - Click on 'Test prediction pattern' - You should have something like 'Spring 2012', 'Summer 2012', ..., 'Winter 2012', 'Spring 2013' - Note that you can have seasons for south hemisphere by entering '2' in 'Year/Inner counter' - 2nd note: if you have some locales installed on your system, you can type its name in the 'Locale' field (actually it does not work for seasons name, only for month names and day names) If you want to modify the numbering pattern you can still do it here: - Click on 'Show/Hide advanced pattern' link. The advanced pattern table is shown but all fields are readonly - Click on 'Modify pattern' button. All readonly fields are now editable. Note that 'Begins with' and 'Inner counter' line are repeated here and any modifications in the small table will be replicated in the big table, and vice versa. - Pattern name is emptied, if you type a new name, a new pattern will be created, and if you type the same name as an existing numbering pattern, this one will be modified (with a confirmation message) - There is two new lines in this table: - Label: it's what is displayed in the smaller table headers above - Numbering: used to format numbers in different ways. can be 'seasons', 'monthname' or 'dayname'. Month name and day name can be localized using the 'Locale' field. Seasons can't (values for english and french are hard-coded in Serials.pm) - You can modify what you want in the table and click on 'Test prediction pattern' button each time you want to see your modifications. (Note that checkboxes for irregularities aren't displayed in this mode, and you can't save the subscription until you have saved or cancelled your changes). - To cancel your modifications, just click on 'Cancel modifications' button. - To save them, click on 'Save as new pattern'. If the pattern name is already existing, a confirmation box will ask you if you want to modify the existing numbering pattern. Otherwise a new pattern will be created and automatically selected. Once you have finished modifying numbering pattern. You can click again on 'Test prediction pattern' to define irregularities, and then click on 'Save subscription'. Now you can check the serials module still works correctly: - Check the subscription detail page to confirm that nothing is missing. Especially the 'Frequency' and 'Number pattern' infos - Try to receive some issues. Check that the serial number is correctly generated and if irregularities you have defined are taken into account (if you have defined some). - Check that receiving is blocked once you have reached the number of issues you have defined in subscription length (or once you have reached the subscription end date) In serials menu (to the left of almost each page of serials menu) you have two new links: 'Manage frequencies' and 'Manage numbering patterns'. 'Manage numbering patterns' lead to a page which list all numbering patterns and allow you to create, edit or delete them. The interface is almost the same as numbering pattern modification in subscription-add.pl 'Manage frequencies' lead to a page which list all frequencies and allow you to create, edit or delete them. Try to create a new frequency: - Click on 'Manage frequencies' link in the serials menu and then click on 'New frequency': - Fill in the description (mandatory). - Unit is one of 'day', 'week', 'month', year' or 'None' ('None' is for an irregular subscription) - If unit is different from 'None' you have to fill the two following fields (Issues per unit, and Units per issue) - Note that at least one of those must be equal to 1 - Issues per unit is the number of received issues by 'unit' and Units per issue is the number of 'unit' between two issues - Display order is used to build the drop-down list. Leave empty and it will be set to 0 (top of the list) - Then click on 'Save' - Check that this new frequency appears in the frequencies table and in the drop-down list in subscription-add.pl Subscription history has been moved in its own page. To test if it still works, choose a subscription with manual history enabled (or modify an existing subscription to turn on manual history). - On the detail page, tab 'Planning', you should have a link 'Edit history'. - Click on it - Modify history and click on Save - In tab 'Summary' you should have the infos you just entered And finally, you can check that old subscriptions (by old I mean subscriptions that existed before the update) are correctly linked to an existing numbering pattern and an existing frequency. Numbering patterns should be named 'Backup pattern X' where X is a number.
Created attachment 10739 [details] [review] Database files Rebased patch
Created attachment 10740 [details] [review] Change subscription numbering pattern and frequencies Rebased patch
You patch contains merging markers like: <<<<<<< HEAD So subscription-add.tt is unusable. Could you fix and resubmit? Thanks.
Created attachment 10742 [details] [review] Change subscription numbering pattern and frequencies Oooups... I made it too fast :-/
Thanks. I can go one step further. There is now another blocker. On the page Add a new subscription (2/2), the 3 date pickers are broken, and doesn't display an image representing a calendar. There is a reference to this image: /intranet-tmpl/prog/en/lib/calendar/cal.gif which doesn't exist anymore on HEAD. The issue is that your patch uses YUI date picker which is deprecated. You're now supposed to use jQuery UI date picker. You can see a example on this page: Tools > Export data. The Start date and End date use jQuery UI.
Created attachment 10743 [details] [review] Bug 7688: replace yui datepicker with jQuery datepicker (subscription-add)
It works as described. I have few questions/remarks: PATCH DESCRIPTION: These names are localized by using POSIX::setlocale and POSIX::strftime and setting a 'locale' value to the subscription. Locale have to be installed on the system. QUESTION: It doesn't work for me. I've tried entering 'fr' or 'fr-FR'. Could you elaborate on this point? Wouldn't it be possible to display a list of available languages? PATCH DESCRIPTION: Important note: updatedatabase.pl script takes into account existing subscriptions and create a numbering pattern for each one. Frequency is mapped to the correct entry in subscription_frequencies table QUESTION: So it means that for a library having, let say, 500 subscriptions, 500 numbering patterns will be created. Is there a way to avoid such inflation of numbering patterns? Since a some subscriptions share the same numbering pattern, it would be nice to link them to a unique numbering pattern. Side note: There is no way to delete Numbering patterns. REMARKS: [1] If I click on Modify pattern, and then Save a new patter, without filling Pattern name field, and 'empty' pattern is saved. It appears in Numbering pattern list box. And all pattern field contain 'undefined'. [2] 'Numbering' field in the prediction pattern is quiet cryptic. There is no indication that you can enter a text defining how the variable will be formatted. And why 'Numbering', and not 'Formating', or 'Displayed as'?
Created attachment 10903 [details] [review] Bug 7688: Follow-up: Fixes and improvements - constraint between subscription and subscription_frequencies changed (added ON DELETE/ON UPDATE) - constraint between subscription and subscription_numberpatterns changed (added ON DELETE/ON UPDATE) - When deleting a frequency or a numbering pattern, ask for a confirmation if it's still used by subscriptions - updatedatabase.pl do not blindly insert numbering pattern anymore. Now it will first check if a similar pattern already exists in database. - "Numbering" reworded to "Formatting", text inputs replaced by hard-coded selects - Avoid creating a new numbering pattern with no name - On subscription-numberpatterns.pl, replace Calendar by jQueryUI datepicker
(In reply to comment #21) > QUESTION: > > It doesn't work for me. I've tried entering 'fr' or 'fr-FR'. Could you > elaborate on this point? Wouldn't it be possible to display a list of > available languages? I've tested something within this patch to provide a list of available locales. It calls `locale -a` to get the list of installed locales. (will not work with Windows I think) > QUESTION: > > So it means that for a library having, let say, 500 subscriptions, > 500 numbering patterns will be created. Is there a way to avoid > such inflation of numbering patterns? Since a some subscriptions share > the same numbering pattern, it would be nice to link them to a unique > numbering pattern. The update process will now check for similar patterns already existing before insert. > Side note: There is no way to delete Numbering patterns. I think this was because of foreign key constraints, which were too restrictive. Changed that so you can delete numbering patterns and frequencies even if some subscriptions use them (but it asks for a confirmation). > REMARKS: > > [1] If I click on Modify pattern, and then Save a new patter, without > filling Pattern name field, and 'empty' pattern is saved. It appears in > Numbering pattern list box. And all pattern field contain 'undefined'. Fixed. > [2] 'Numbering' field in the prediction pattern is quiet cryptic. There > is no indication that you can enter a text defining how the variable > will be formatted. And why 'Numbering', and not 'Formating', or > 'Displayed as'? text inputs have been replaced by selects, so you can only enter valid values.
Oh and notice that when using "season" in "Numbering" (now "Formatting"), localization will only work for english and french as season names are hardcoded (POSIX::strftime cannot return name of seasons). I think it could be a good place to use something like Koha::I18N (Bug 8044). Maybe there are better solutions?
Julian, do you think you could merge you 4 patches into one? (I can do it when signing-off if you prefer). It would help at this stage, will help for QA. IMHO, follow-up patches are best suited later, when the main patch has already been pushed, and that some minor improvements, tweaking are done.
Created attachment 10920 [details] [review] Bug 7688: Change subscription numbering pattern and frequencies Squash of the 4 previous patches. Freshly rebased on current master
(In reply to comment #25) > Julian, do you think you could merge you 4 patches into one? (I can do it > when signing-off if you prefer). It would help at this stage, will help for > QA. IMHO, follow-up patches are best suited later, when the main patch has > already been pushed, and that some minor improvements, tweaking are done. Just for the record = having follow-ups is useful when someone has tested, made some comments, and the patcher want to show the differences between previous patch, and updated patch. In this case, follow-up is OK an even better, and maybe squashed after signoff/QA process. Note that for a 2 or 3 lines patch, it can be squashed immediately, because changing just a line is easy to spot. My comment apply to larger patches
It works as advertised. Good job! It's a great improvement for Koha Serials module. The subscription editing in two pages had a degree of clarity which changes the game, even if I'm a sure visually more 'elegant' panning could be designed (Owen Leonard?) On Manage Numbering patterns page, it would be interesting to see the number of subscriptions attached to each pattern. And if this number is a link searching for all related subscriptions it would be perfect. But it can wait. Even if I sign-off this patch, I think that a second signing would be welcomed. It's a large patch, and someone familiar with Serials module could find side effects I haven't detected with my tests. Another solution would be to push it quickly, far from 3.10 release, and continue improving the subscription module with feedback from parties interested in this module, and the whole community.
Created attachment 10985 [details] [review] Signed-off patch
QA comment: * this path is huge, but most of it's size is due to sample files in various language. Well done, even if they are not all translated. Will call for more translation when pushing * passing our QA script says: 1st problem = # Subroutines::ProhibitExplicitReturnUndef: Got 1 violation(s). I see that there is + return undef unless $subscriptionid; in GetSubscriptionIrregularities Should be + return unless $subscriptionid; 2nd problem = - xt/tt_valid.t tests... FAIL intranet-tmpl/prog/en/modules/serials/subscription-add.tt: 656, 666, 687 FAIL Those lines contain a construction that is still invalid (despite joubu effort): <option value="[% frequency.id %]" [% IF (frequency.selected) %] selected="selected" [% END %]> <option value="[% subt.name %]" [% IF (subt.selected) %] selected="selected" [% END %] > <option value="[% numberpattern.id %]" [% IF (numberpattern.selected) %] selected="selected" [% END %]>[% numberpattern.label %]</option> My git hook says nothing wrong. This patch will earn "passed QA" once the 2 previous problems have been fixed. Also note that there is a conflict easy to solve in C4/Serials.pm I've added a sub in bug 7839, just keep it, with your own ones (beware the closing } for subs !)
Created attachment 11299 [details] [review] [SIGNED-OFF] Bug 7688: Change subscription numbering pattern and frequencies Rebased on master Fixed conflict in updatedatabase.pl and trivial conflict in C4/Serials.pm as said by Paul
Created attachment 11300 [details] [review] Bug 7688 follow-up: Small fixes for QA - # Subroutines::ProhibitExplicitReturnUndef: Got 1 violation(s) in C4::Serials::GetSubscriptionIrregularities - Bad template constructions fixed in serials/subscription-add.tt
I have a ticket from a library asking if Koha can do the following - I'm wondering if this patch does any of these things (just a yes or no would be great for each point): Ability to customize Chronology Type More options and customizability in Publication Cycle Omit dates/months Ability to modify independent issues after they arrive Ability to combine issues (ex. June and July are usually separate but the publication decides to combine them for that year) Date functionality in dropbox (ex. Volume, Number, Date) And have the actual Month spelled out (January...)
(In reply to comment #33) > I have a ticket from a library asking if Koha can do the following - I'm > wondering if this patch does any of these things (just a yes or no would be > great for each point): > > > Ability to customize Chronology Type I'm not sure what you mean by 'chronology type' but you are able to create "frequencies" which are defined by a "unit" (day, week, month or year), a "number of issues per unit" (for example if you receive 2 issues per day), and a number of unit between each issues (for example if you receive 1 issue every 2 days) > More options and customizability in Publication Cycle I don't understand, what is the difference between Publication Cycle and Chronology Type ? > Omit dates/months What does it mean exactly? > Ability to modify independent issues after they arrive Hmm I think you actually can modify already received issues via serials-collection.pl page, by checking appropriate issues and clicking "Edit serials". > Ability to combine issues (ex. June and July are usually separate but the > publication decides to combine them for that year) No. This patch do not allow that. > Date functionality in dropbox (ex. Volume, Number, Date) Do you mean having the received date of issue in its serial number ? In this case, no, but I think this could be added without much effort > And have the actual Month spelled out (January...) Yes, you can indicate 'monthname' in the new field 'Formatting' of numbering pattern, and a value of 1 will be saved as "January". You can choose the locale in the subscription edit page. ('dayname' is also possible)
As Fréderic asked for a second signoff, I reset the status to "Needs signoff"
Created attachment 11489 [details] [review] [SIGNED-OFF] Bug 7688: Change subscription numbering pattern and frequencies Serials numbering pattern and frequencies are no more hard-coded. Now it's possible to create, edit and delete numbering patterns (and frequencies). This implies new sql tables (subscription_numberpatterns and subscription_frequencies) Numbering patterns behave almost as before, there are still the same values to configure (addX, everyX, settoX, whenmorethanX). lastvalueX and innerloopX remain in subscription tables. There is a new value in numbering patterns: numberingX. For each "column" (X, Y or Z) you can tell how to format the number. Actually numberingX can be set to: - 'dayname' (name of the day) (0-6 or 1-7 depending on which day is the first of the week) - 'monthname' (name of the month) (0-11) - 'season' (name of the season) (0-3) (0 is Spring) These names are localized by using POSIX::setlocale and POSIX::strftime and setting a 'locale' value to the subscription. Locale have to be installed on the system. Note that season names are not localized using POSIX::strftime (it can't do this), so names are hardcoded into the code (available languages: en, fr). This could be fixed in the future by using a Perl localization framework. Frequencies can be configured using 3 parameters: - 'unit': one of 'day', 'week', 'month', 'year' - 'issuesperunit': integer >= 1, the number of received issues per 'unit' - 'unitsperissue': integer >= 1, the number of 'unit' between two issues One of 'issuesperunit' and 'unitsperissue' must be equal to 1. Examples: unit = 'day', issuesperunit=3, unitsperissue=1 => 3 issues per day unit = 'week', issuesperunit=1, unitsperissue=3 => 1 issue each 3 weeks Prediction pattern is now computed server-side and is more consistent with what Koha will do. The publication date is displayed alongside the serial number. Irregularities can now be checked one by one, in the prediction pattern table, or if frequency is 'day-based' (unit is 'day'), there is the possibility to check all issues for a week day at once. When an irregularity is found, there is the possibility to keep the serial number unchanged, or to skip it. It is configured at subscription creation or modification. For instance, with a daily subscription you can have: skip serial number | keep serial number ----------------------+---------------------- 2012-01-01 ¦ No 1 | 2012-01-01 ¦ No 1 2012-01-03 ¦ No 3 | 2012-01-03 ¦ No 2 To lighten the subscription modification page, manual history has been moved in its own page subscription-history.pl which is accessible on subscription-detail.pl, tab 'Planning'. Important note: updatedatabase.pl script takes into account existing subscriptions and create appropriate numbering patterns for them (it tries to create as few patterns as possible). Frequency is mapped to the correct entry in subscription_frequencies table. This patch includes kohastructure.sql and updatedatabase.pl changes + sample frequencies data and sample numberpatterns data for fresh installs (sample data is included in updatedatabase.pl) === TEST PLAN: === Create a new subscription: - Go to Serials module and click "New subscription" button - On the first page, choose a biblio and click next to go to the second page - Pick a first issue publication date - Choose frequency '1/day' - Choose a subscription length of 15 issues - Choose a subscription start date - Choose numbering pattern 'Volume, Number' - A table appears, fill 'Begins with' cells with '1' - Click on 'Test prediction pattern' button The prediction pattern is displayed at the right of the page. You can see in it the serial number, the publication date and a checkbox to allow you to choose which serials will not be received (irregularities). You can see that serial number start from "Vol 1, No 1" continue to "Vol 1, No 12" and then restart with "Vol 2, No 1". Frequency is '1/day' so you can see that publication date is incremented by one day line after line. - Now you can play a little with frequencies and numbering patterns, change one of them (or both) and click again on 'Test prediction pattern' - For example, choose frequency '3/weeks' and click on 'Test prediction pattern' button'. There is a little behaviour change compared with current master. Publication date will not be guessed within the week. Koha can't know when you will receive issues. So the publication date stay the same (monday of each week) for 3 consecutive issues and then jump to the next week. - Now choose frequency '1/3 months' and numbering pattern 'Seasonal' - Fill 'Begins with' cells with '2012' for Year and '0' for Season - Click on 'Test prediction pattern' - You should have something like 'Spring 2012', 'Summer 2012', ..., 'Winter 2012', 'Spring 2013' - Note that you can have seasons for south hemisphere by entering '2' in 'Year/Inner counter' - 2nd note: if you have some locales installed on your system, you can type its name in the 'Locale' field (actually it does not work for seasons name, only for month names and day names) If you want to modify the numbering pattern you can still do it here: - Click on 'Show/Hide advanced pattern' link. The advanced pattern table is shown but all fields are readonly - Click on 'Modify pattern' button. All readonly fields are now editable. Note that 'Begins with' and 'Inner counter' line are repeated here and any modifications in the small table will be replicated in the big table, and vice versa. - Pattern name is emptied, if you type a new name, a new pattern will be created, and if you type the same name as an existing numbering pattern, this one will be modified (with a confirmation message) - There is two new lines in this table: - Label: it's what is displayed in the smaller table headers above - Numbering: used to format numbers in different ways. can be 'seasons', 'monthname' or 'dayname'. Month name and day name can be localized using the 'Locale' field. Seasons can't (values for english and french are hard-coded in Serials.pm) - You can modify what you want in the table and click on 'Test prediction pattern' button each time you want to see your modifications. (Note that checkboxes for irregularities aren't displayed in this mode, and you can't save the subscription until you have saved or cancelled your changes). - To cancel your modifications, just click on 'Cancel modifications' button. - To save them, click on 'Save as new pattern'. If the pattern name is already existing, a confirmation box will ask you if you want to modify the existing numbering pattern. Otherwise a new pattern will be created and automatically selected. Once you have finished modifying numbering pattern. You can click again on 'Test prediction pattern' to define irregularities, and then click on 'Save subscription'. Now you can check the serials module still works correctly: - Check the subscription detail page to confirm that nothing is missing. Especially the 'Frequency' and 'Number pattern' infos - Try to receive some issues. Check that the serial number is correctly generated and if irregularities you have defined are taken into account (if you have defined some). - Check that receiving is blocked once you have reached the number of issues you have defined in subscription length (or once you have reached the subscription end date) In serials menu (to the left of almost each page of serials menu) you have two new links: 'Manage frequencies' and 'Manage numbering patterns'. 'Manage numbering patterns' lead to a page which list all numbering patterns and allow you to create, edit or delete them. The interface is almost the same as numbering pattern modification in subscription-add.pl 'Manage frequencies' lead to a page which list all frequencies and allow you to create, edit or delete them. Try to create a new frequency: - Click on 'Manage frequencies' link in the serials menu and then click on 'New frequency': - Fill in the description (mandatory). - Unit is one of 'day', 'week', 'month', year' or 'None' ('None' is for an irregular subscription) - If unit is different from 'None' you have to fill the two following fields (Issues per unit, and Units per issue) - Note that at least one of those must be equal to 1 - Issues per unit is the number of received issues by 'unit' and Units per issue is the number of 'unit' between two issues - Display order is used to build the drop-down list. Leave empty and it will be set to 0 (top of the list) - Then click on 'Save' - Check that this new frequency appears in the frequencies table and in the drop-down list in subscription-add.pl Subscription history has been moved in its own page. To test if it still works, choose a subscription with manual history enabled (or modify an existing subscription to turn on manual history). - On the detail page, tab 'Planning', you should have a link 'Edit history'. - Click on it - Modify history and click on Save - In tab 'Summary' you should have the infos you just entered And finally, you can check that old subscriptions (by old I mean subscriptions that existed before the update) are correctly linked to an existing numbering pattern and an existing frequency. Numbering patterns should be named 'Backup pattern X' where X is a number. Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 11490 [details] [review] [SIGNED-OFF] Bug 7688 follow-up: Small fixes for QA - # Subroutines::ProhibitExplicitReturnUndef: Got 1 violation(s) in C4::Serials::GetSubscriptionIrregularities - Bad template constructions fixed in serials/subscription-add.tt Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
QA Comment: It would be better that someone not from BibLibre make QA on this patch. To pass QA, some points would be great: - mandatory: perltidy on all new files - minor: you change prototype for ModReceiveOrder but you don't modify files into the test directory - some unit tests for your invoices routines would be perfect. See Bug 5327, using DBD::Mock it is very simple.
wrong bug for last QA note, sorry
QA comment * the database changes says: ADD CONSTRAINT subscription_ibfk_1 FOREIGN KEY (periodicity) REFERENCES subscription_frequencies (id) ON DELETE SET NULL ON UPDATE CASCADE, ADD CONSTRAINT subscription_ibfk_2 FOREIGN KEY (numberpattern) REFERENCES subscription_numberpatterns (id) ON DELETE SET NULL ON UPDATE CASCADE ON DELETE SET NULL = I would have preferred ON DELETE RESTRICT, but SET NULL is OK. * koha-qa.pl show many QA (most minor: :utf8 mistakes) problems: - C4/Serials/Frequency.pm FAIL critic FAIL - C4/Serials/Numberpattern.pm FAIL critic FAIL - serials/create-numberpattern.pl FAIL critic FAIL - serials/subscription-frequency.pl FAIL critic FAIL - serials/subscription-numberpattern.pl FAIL critic FAIL QUESTION : why in the FR-fr samples do you have english strings (and not french) : diff --git a/installer/data/mysql/fr-FR/2-Optionel/sample_frequencies.sql b/installer/data/mysql/fr-FR/2-Optionel/sample_frequencies.sql shouldn't : + ('2/day', 'day', 1, 2, 1), + ('1/day', 'day', 1, 1, 2), + ('3/week', 'week', 1, 3, 3), + ('1/week', 'week', 1, 1, 4), + ('1/2 weeks', 'week', 2, 1, 5), + ('1/3 weeks', 'week', 3, 1, 6), be + ('2/jour', 'day', 1, 2, 1), + ('1/jour', 'day', 1, 1, 2), + ('3/semaine', 'week', 1, 3, 3), + ('1/semaine', 'week', 1, 1, 4), + ('1/2 semaines', 'week', 2, 1, 5), + ('1/3 semaines', 'week', 3, 1, 6), (if yes, please confirm -so i can ask koha-translate mailing list for SP, IT, NO,... translations, and provide a french version) marking failed QA for now, I have no other QA comment (spent almost 1H checking the code). (I haven't tested this patch yet, will do once it passes QA)
oups wanted to FAIL qa, not pass !
Created attachment 12268 [details] [review] Bug 7688 follow-up: Small fixes for QA #2 - "return undef" -> "return" - ":utf8" -> ":encoding(UTF-8)" - TAB -> SPACES
Created attachment 12269 [details] [review] Bug 7688: Translate sample frequencies for french
(In reply to comment #40) > QUESTION : why in the FR-fr samples do you have english strings (and not > french) : > diff --git a/installer/data/mysql/fr-FR/2-Optionel/sample_frequencies.sql > b/installer/data/mysql/fr-FR/2-Optionel/sample_frequencies.sql > shouldn't : > + ('2/day', 'day', 1, 2, 1), > + ('1/day', 'day', 1, 1, 2), > + ('3/week', 'week', 1, 3, 3), > + ('1/week', 'week', 1, 1, 4), > + ('1/2 weeks', 'week', 2, 1, 5), > + ('1/3 weeks', 'week', 3, 1, 6), > be > + ('2/jour', 'day', 1, 2, 1), > + ('1/jour', 'day', 1, 1, 2), > + ('3/semaine', 'week', 1, 3, 3), > + ('1/semaine', 'week', 1, 1, 4), > + ('1/2 semaines', 'week', 2, 1, 5), > + ('1/3 semaines', 'week', 3, 1, 6), > > (if yes, please confirm -so i can ask koha-translate mailing list for SP, > IT, NO,... translations, and provide a french version) Yes it should be translated. Note that it can also be done through staff interface.
Created attachment 13193 [details] [review] [SIGNED-OFF] Bug 7688: Change subscription numbering pattern and frequencies Rebased on master
Created attachment 13194 [details] [review] [SIGNED-OFF] Bug 7688 follow-up: Small fixes for QA Rebased on master
Created attachment 13195 [details] [review] Bug 7688 follow-up: Small fixes for QA #2 Rebased on master
Created attachment 13196 [details] [review] Bug 7688: Translate sample frequencies for french rebased on master
Created attachment 14122 [details] [review] [SIGNED-OFF] Bug 7688: Change subscription numbering pattern and frequencies Rebased on master
Created attachment 14123 [details] [review] [SIGNED-OFF] Bug 7688 follow-up: Small fixes for QA Rebased on master
Created attachment 14124 [details] [review] Bug 7688 follow-up: Small fixes for QA #2 rebased on master
Created attachment 14125 [details] [review] Bug 7688: Translate sample frequencies for french rebased on master
fix minor conflicts with Bug 8782
Created attachment 14996 [details] [review] Bug 7688: Fix generating next serial when there is no 'Expected' issue It can happen when the Expected issue is claimed. In this case the status of the last serial is 'Claimed' This patch change the API of GetNextSeq and GetSeq Test plan: - Create a subscription which starts a long time ago so that serials automatically appear in late issues - Receive the first serial - Go to claims page and claim the 2nd serial. - Go back to the subscription page and click on 'Serial collection' - You should have 2 serials, one 'Arrived' and one 'Claimed'. - Click on Generate Next. This should fail with a software error message ("can't call method output ...") - Apply this patch and click again on Generate Next. A new issue must be created with status 'Expected'.
I tried to test.
Created attachment 15492 [details] [review] Bug 7688: Change subscription numbering pattern and frequencies Serials numbering pattern and frequencies are no more hard-coded. Now it's possible to create, edit and delete numbering patterns (and frequencies). This implies new sql tables (subscription_numberpatterns and subscription_frequencies) Numbering patterns behave almost as before, there are still the same values to configure (addX, everyX, settoX, whenmorethanX). lastvalueX and innerloopX remain in subscription tables. There is a new value in numbering patterns: numberingX. For each "column" (X, Y or Z) you can tell how to format the number. Actually numberingX can be set to: - 'dayname' (name of the day) (0-6 or 1-7 depending on which day is the first of the week) - 'monthname' (name of the month) (0-11) - 'season' (name of the season) (0-3) (0 is Spring) These names are localized by using POSIX::setlocale and POSIX::strftime and setting a 'locale' value to the subscription. Locale have to be installed on the system. Note that season names are not localized using POSIX::strftime (it can't do this), so names are hardcoded into the code (available languages: en, fr). This could be fixed in the future by using a Perl localization framework. Frequencies can be configured using 3 parameters: - 'unit': one of 'day', 'week', 'month', 'year' - 'issuesperunit': integer >= 1, the number of received issues per 'unit' - 'unitsperissue': integer >= 1, the number of 'unit' between two issues One of 'issuesperunit' and 'unitsperissue' must be equal to 1. Examples: unit = 'day', issuesperunit=3, unitsperissue=1 => 3 issues per day unit = 'week', issuesperunit=1, unitsperissue=3 => 1 issue each 3 weeks Prediction pattern is now computed server-side and is more consistent with what Koha will do. The publication date is displayed alongside the serial number. Irregularities can now be checked one by one, in the prediction pattern table, or if frequency is 'day-based' (unit is 'day'), there is the possibility to check all issues for a week day at once. When an irregularity is found, there is the possibility to keep the serial number unchanged, or to skip it. It is configured at subscription creation or modification. For instance, with a daily subscription you can have: skip serial number | keep serial number ----------------------+---------------------- 2012-01-01 ¦ No 1 | 2012-01-01 ¦ No 1 2012-01-03 ¦ No 3 | 2012-01-03 ¦ No 2 To lighten the subscription modification page, manual history has been moved in its own page subscription-history.pl which is accessible on subscription-detail.pl, tab 'Planning'. Important note: updatedatabase.pl script takes into account existing subscriptions and create appropriate numbering patterns for them (it tries to create as few patterns as possible). Frequency is mapped to the correct entry in subscription_frequencies table. This patch includes kohastructure.sql and updatedatabase.pl changes + sample frequencies data and sample numberpatterns data for fresh installs (sample data is included in updatedatabase.pl) === TEST PLAN: === Create a new subscription: - Go to Serials module and click "New subscription" button - On the first page, choose a biblio and click next to go to the second page - Pick a first issue publication date - Choose frequency '1/day' - Choose a subscription length of 15 issues - Choose a subscription start date - Choose numbering pattern 'Volume, Number' - A table appears, fill 'Begins with' cells with '1' - Click on 'Test prediction pattern' button The prediction pattern is displayed at the right of the page. You can see in it the serial number, the publication date and a checkbox to allow you to choose which serials will not be received (irregularities). You can see that serial number start from "Vol 1, No 1" continue to "Vol 1, No 12" and then restart with "Vol 2, No 1". Frequency is '1/day' so you can see that publication date is incremented by one day line after line. - Now you can play a little with frequencies and numbering patterns, change one of them (or both) and click again on 'Test prediction pattern' - For example, choose frequency '3/weeks' and click on 'Test prediction pattern' button'. There is a little behaviour change compared with current master. Publication date will not be guessed within the week. Koha can't know when you will receive issues. So the publication date stay the same (monday of each week) for 3 consecutive issues and then jump to the next week. - Now choose frequency '1/3 months' and numbering pattern 'Seasonal' - Fill 'Begins with' cells with '2012' for Year and '0' for Season - Click on 'Test prediction pattern' - You should have something like 'Spring 2012', 'Summer 2012', ..., 'Winter 2012', 'Spring 2013' - Note that you can have seasons for south hemisphere by entering '2' in 'Year/Inner counter' - 2nd note: if you have some locales installed on your system, you can type its name in the 'Locale' field (actually it does not work for seasons name, only for month names and day names) If you want to modify the numbering pattern you can still do it here: - Click on 'Show/Hide advanced pattern' link. The advanced pattern table is shown but all fields are readonly - Click on 'Modify pattern' button. All readonly fields are now editable. Note that 'Begins with' and 'Inner counter' line are repeated here and any modifications in the small table will be replicated in the big table, and vice versa. - Pattern name is emptied, if you type a new name, a new pattern will be created, and if you type the same name as an existing numbering pattern, this one will be modified (with a confirmation message) - There is two new lines in this table: - Label: it's what is displayed in the smaller table headers above - Numbering: used to format numbers in different ways. can be 'seasons', 'monthname' or 'dayname'. Month name and day name can be localized using the 'Locale' field. Seasons can't (values for english and french are hard-coded in Serials.pm) - You can modify what you want in the table and click on 'Test prediction pattern' button each time you want to see your modifications. (Note that checkboxes for irregularities aren't displayed in this mode, and you can't save the subscription until you have saved or cancelled your changes). - To cancel your modifications, just click on 'Cancel modifications' button. - To save them, click on 'Save as new pattern'. If the pattern name is already existing, a confirmation box will ask you if you want to modify the existing numbering pattern. Otherwise a new pattern will be created and automatically selected. Once you have finished modifying numbering pattern. You can click again on 'Test prediction pattern' to define irregularities, and then click on 'Save subscription'. Now you can check the serials module still works correctly: - Check the subscription detail page to confirm that nothing is missing. Especially the 'Frequency' and 'Number pattern' infos - Try to receive some issues. Check that the serial number is correctly generated and if irregularities you have defined are taken into account (if you have defined some). - Check that receiving is blocked once you have reached the number of issues you have defined in subscription length (or once you have reached the subscription end date) In serials menu (to the left of almost each page of serials menu) you have two new links: 'Manage frequencies' and 'Manage numbering patterns'. 'Manage numbering patterns' lead to a page which list all numbering patterns and allow you to create, edit or delete them. The interface is almost the same as numbering pattern modification in subscription-add.pl 'Manage frequencies' lead to a page which list all frequencies and allow you to create, edit or delete them. Try to create a new frequency: - Click on 'Manage frequencies' link in the serials menu and then click on 'New frequency': - Fill in the description (mandatory). - Unit is one of 'day', 'week', 'month', year' or 'None' ('None' is for an irregular subscription) - If unit is different from 'None' you have to fill the two following fields (Issues per unit, and Units per issue) - Note that at least one of those must be equal to 1 - Issues per unit is the number of received issues by 'unit' and Units per issue is the number of 'unit' between two issues - Display order is used to build the drop-down list. Leave empty and it will be set to 0 (top of the list) - Then click on 'Save' - Check that this new frequency appears in the frequencies table and in the drop-down list in subscription-add.pl Subscription history has been moved in its own page. To test if it still works, choose a subscription with manual history enabled (or modify an existing subscription to turn on manual history). - On the detail page, tab 'Planning', you should have a link 'Edit history'. - Click on it - Modify history and click on Save - In tab 'Summary' you should have the infos you just entered And finally, you can check that old subscriptions (by old I mean subscriptions that existed before the update) are correctly linked to an existing numbering pattern and an existing frequency. Numbering patterns should be named 'Backup pattern X' where X is a number. Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 15493 [details] [review] Bug 7688 follow-up: Small fixes for QA - # Subroutines::ProhibitExplicitReturnUndef: Got 1 violation(s) in C4::Serials::GetSubscriptionIrregularities - Bad template constructions fixed in serials/subscription-add.tt Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 15494 [details] [review] Bug 7688 follow-up: Small fixes for QA #2 - "return undef" -> "return" - ":utf8" -> ":encoding(UTF-8)" - TAB -> SPACES
Created attachment 15495 [details] [review] Bug 7688: Translate sample frequencies for french
Created attachment 15496 [details] [review] Bug 7688: Fix generating next serial when there is no 'Expected' issue It can happen when the Expected issue is claimed. In this case the status of the last serial is 'Claimed' This patch change the API of GetNextSeq and GetSeq Test plan: - Create a subscription which starts a long time ago so that serials automatically appear in late issues - Receive the first serial - Go to claims page and claim the 2nd serial. - Go back to the subscription page and click on 'Serial collection' - You should have 2 serials, one 'Arrived' and one 'Claimed'. - Click on Generate Next. This should fail with a software error message ("can't call method output ...") - Apply this patch and click again on Generate Next. A new issue must be created with status 'Expected'.
rebased patches
Bad status, switch back to Needs Signoff
Testing now, a few notes: 1) applying patches gives a warning about added trailing whitespaces warning: 3 lines applied after fixing whitespace errors. 2) updatedatabase: Is this important? ~/kohaclone$ perl installer/data/mysql/updatedatabase.pl DBD::mysql::st execute failed: Column 'numberingmethod' cannot be null at installer/data/mysql/updatedatabase.pl line 6584. Upgrade to XXX done (Add subscription_frequencies and subscription_numberpatterns tables) 3) koha-qa * C4/Serials.pm FAIL pod FAIL *** ERROR: Unknown command 'over4' in file C4/Serials.pm *** ERROR: =item without previous =over in file C4/Serials.pm
(In reply to comment #63) > Testing now, a few notes: > > 1) applying patches gives a warning about added trailing whitespaces > warning: 3 lines applied after fixing whitespace errors. > You are using this eh? That tidies the whitespace up so once its signed off its all fixed :) http://blog.bigballofwax.co.nz/2011/12/15/fixing-whitespace-when-apply-patches-with-git/
(In reply to comment #64) > You are using this eh? That tidies the whitespace up so once its signed off > its all fixed :) > > http://blog.bigballofwax.co.nz/2011/12/15/fixing-whitespace-when-apply- > patches-with-git/ that's right :)
Well, as far as I can test, all is working. I tried what was described in the original test plan, also did follow the comments and successive tests and fixes. I do not have in my DB many subscriptions but those where transformed correctly. I could generate custom number patterns, rename, reuse, mark and verify irregularities, etc. For me it's very very good. I suppose that someone with more experience or data in subscriptions could test more extensively. Only note for my part are this simple errors: a) C4/Serial.pm, line 1327, missing space in "=over 4", I could do that. b) C4/Serials/Frequency.pm and C4/Serials/Numberpattern.pm these files do not obey coding guidelines over POD comments http://wiki.koha-community.org/wiki/Coding_Guidelines#PERL13:_POD what is missing is general description, etc (koha-qa complains about =head3 without 2 and 1) I couldn't do that. Without these corrections is guaranteed to go "Failed QA", and it would be a shame.
(In reply to comment #66) > For me it's very very good. I suppose that someone with more experience or > data in subscriptions could test more extensively. Just FYI, this dev is in production on a 3.2 since something like 18 months. Things had to be rewritten to T::T, that's why the library had not signed-off itself. But the feature is working well, and if you signoff, I think we can move to QA without another signoff. > Without these corrections is guaranteed to go "Failed QA", and it would be a > shame. Agreed !
Created attachment 15534 [details] [review] Bug 7688: Followup FIX perldoc
Created attachment 15536 [details] [review] [SIGNED-OFF] Bug 7688: Change subscription numbering pattern and frequencies Serials numbering pattern and frequencies are no more hard-coded. Now it's possible to create, edit and delete numbering patterns (and frequencies). This implies new sql tables (subscription_numberpatterns and subscription_frequencies) Numbering patterns behave almost as before, there are still the same values to configure (addX, everyX, settoX, whenmorethanX). lastvalueX and innerloopX remain in subscription tables. There is a new value in numbering patterns: numberingX. For each "column" (X, Y or Z) you can tell how to format the number. Actually numberingX can be set to: - 'dayname' (name of the day) (0-6 or 1-7 depending on which day is the first of the week) - 'monthname' (name of the month) (0-11) - 'season' (name of the season) (0-3) (0 is Spring) These names are localized by using POSIX::setlocale and POSIX::strftime and setting a 'locale' value to the subscription. Locale have to be installed on the system. Note that season names are not localized using POSIX::strftime (it can't do this), so names are hardcoded into the code (available languages: en, fr). This could be fixed in the future by using a Perl localization framework. Frequencies can be configured using 3 parameters: - 'unit': one of 'day', 'week', 'month', 'year' - 'issuesperunit': integer >= 1, the number of received issues per 'unit' - 'unitsperissue': integer >= 1, the number of 'unit' between two issues One of 'issuesperunit' and 'unitsperissue' must be equal to 1. Examples: unit = 'day', issuesperunit=3, unitsperissue=1 => 3 issues per day unit = 'week', issuesperunit=1, unitsperissue=3 => 1 issue each 3 weeks Prediction pattern is now computed server-side and is more consistent with what Koha will do. The publication date is displayed alongside the serial number. Irregularities can now be checked one by one, in the prediction pattern table, or if frequency is 'day-based' (unit is 'day'), there is the possibility to check all issues for a week day at once. When an irregularity is found, there is the possibility to keep the serial number unchanged, or to skip it. It is configured at subscription creation or modification. For instance, with a daily subscription you can have: skip serial number | keep serial number ----------------------+---------------------- 2012-01-01 ¦ No 1 | 2012-01-01 ¦ No 1 2012-01-03 ¦ No 3 | 2012-01-03 ¦ No 2 To lighten the subscription modification page, manual history has been moved in its own page subscription-history.pl which is accessible on subscription-detail.pl, tab 'Planning'. Important note: updatedatabase.pl script takes into account existing subscriptions and create appropriate numbering patterns for them (it tries to create as few patterns as possible). Frequency is mapped to the correct entry in subscription_frequencies table. This patch includes kohastructure.sql and updatedatabase.pl changes + sample frequencies data and sample numberpatterns data for fresh installs (sample data is included in updatedatabase.pl) === TEST PLAN: === Create a new subscription: - Go to Serials module and click "New subscription" button - On the first page, choose a biblio and click next to go to the second page - Pick a first issue publication date - Choose frequency '1/day' - Choose a subscription length of 15 issues - Choose a subscription start date - Choose numbering pattern 'Volume, Number' - A table appears, fill 'Begins with' cells with '1' - Click on 'Test prediction pattern' button The prediction pattern is displayed at the right of the page. You can see in it the serial number, the publication date and a checkbox to allow you to choose which serials will not be received (irregularities). You can see that serial number start from "Vol 1, No 1" continue to "Vol 1, No 12" and then restart with "Vol 2, No 1". Frequency is '1/day' so you can see that publication date is incremented by one day line after line. - Now you can play a little with frequencies and numbering patterns, change one of them (or both) and click again on 'Test prediction pattern' - For example, choose frequency '3/weeks' and click on 'Test prediction pattern' button'. There is a little behaviour change compared with current master. Publication date will not be guessed within the week. Koha can't know when you will receive issues. So the publication date stay the same (monday of each week) for 3 consecutive issues and then jump to the next week. - Now choose frequency '1/3 months' and numbering pattern 'Seasonal' - Fill 'Begins with' cells with '2012' for Year and '0' for Season - Click on 'Test prediction pattern' - You should have something like 'Spring 2012', 'Summer 2012', ..., 'Winter 2012', 'Spring 2013' - Note that you can have seasons for south hemisphere by entering '2' in 'Year/Inner counter' - 2nd note: if you have some locales installed on your system, you can type its name in the 'Locale' field (actually it does not work for seasons name, only for month names and day names) If you want to modify the numbering pattern you can still do it here: - Click on 'Show/Hide advanced pattern' link. The advanced pattern table is shown but all fields are readonly - Click on 'Modify pattern' button. All readonly fields are now editable. Note that 'Begins with' and 'Inner counter' line are repeated here and any modifications in the small table will be replicated in the big table, and vice versa. - Pattern name is emptied, if you type a new name, a new pattern will be created, and if you type the same name as an existing numbering pattern, this one will be modified (with a confirmation message) - There is two new lines in this table: - Label: it's what is displayed in the smaller table headers above - Numbering: used to format numbers in different ways. can be 'seasons', 'monthname' or 'dayname'. Month name and day name can be localized using the 'Locale' field. Seasons can't (values for english and french are hard-coded in Serials.pm) - You can modify what you want in the table and click on 'Test prediction pattern' button each time you want to see your modifications. (Note that checkboxes for irregularities aren't displayed in this mode, and you can't save the subscription until you have saved or cancelled your changes). - To cancel your modifications, just click on 'Cancel modifications' button. - To save them, click on 'Save as new pattern'. If the pattern name is already existing, a confirmation box will ask you if you want to modify the existing numbering pattern. Otherwise a new pattern will be created and automatically selected. Once you have finished modifying numbering pattern. You can click again on 'Test prediction pattern' to define irregularities, and then click on 'Save subscription'. Now you can check the serials module still works correctly: - Check the subscription detail page to confirm that nothing is missing. Especially the 'Frequency' and 'Number pattern' infos - Try to receive some issues. Check that the serial number is correctly generated and if irregularities you have defined are taken into account (if you have defined some). - Check that receiving is blocked once you have reached the number of issues you have defined in subscription length (or once you have reached the subscription end date) In serials menu (to the left of almost each page of serials menu) you have two new links: 'Manage frequencies' and 'Manage numbering patterns'. 'Manage numbering patterns' lead to a page which list all numbering patterns and allow you to create, edit or delete them. The interface is almost the same as numbering pattern modification in subscription-add.pl 'Manage frequencies' lead to a page which list all frequencies and allow you to create, edit or delete them. Try to create a new frequency: - Click on 'Manage frequencies' link in the serials menu and then click on 'New frequency': - Fill in the description (mandatory). - Unit is one of 'day', 'week', 'month', year' or 'None' ('None' is for an irregular subscription) - If unit is different from 'None' you have to fill the two following fields (Issues per unit, and Units per issue) - Note that at least one of those must be equal to 1 - Issues per unit is the number of received issues by 'unit' and Units per issue is the number of 'unit' between two issues - Display order is used to build the drop-down list. Leave empty and it will be set to 0 (top of the list) - Then click on 'Save' - Check that this new frequency appears in the frequencies table and in the drop-down list in subscription-add.pl Subscription history has been moved in its own page. To test if it still works, choose a subscription with manual history enabled (or modify an existing subscription to turn on manual history). - On the detail page, tab 'Planning', you should have a link 'Edit history'. - Click on it - Modify history and click on Save - In tab 'Summary' you should have the infos you just entered And finally, you can check that old subscriptions (by old I mean subscriptions that existed before the update) are correctly linked to an existing numbering pattern and an existing frequency. Numbering patterns should be named 'Backup pattern X' where X is a number. Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comment: Great development! Work as described. No koha-qa errors (with all patches applied). Please QA this fast.
Created attachment 15537 [details] [review] [SIGNED-OFF] Bug 7688 follow-up: Small fixes for QA - # Subroutines::ProhibitExplicitReturnUndef: Got 1 violation(s) in C4::Serials::GetSubscriptionIrregularities - Bad template constructions fixed in serials/subscription-add.tt Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Created attachment 15538 [details] [review] [SIGNED-OFF] Bug 7688 follow-up: Small fixes for QA #2 - "return undef" -> "return" - ":utf8" -> ":encoding(UTF-8)" - TAB -> SPACES Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Created attachment 15539 [details] [review] [SIGNED-OFF] Bug 7688: Translate sample frequencies for french Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Created attachment 15540 [details] [review] [SIGNED-OFF] Bug 7688: Fix generating next serial when there is no 'Expected' issue It can happen when the Expected issue is claimed. In this case the status of the last serial is 'Claimed' This patch change the API of GetNextSeq and GetSeq Test plan: - Create a subscription which starts a long time ago so that serials automatically appear in late issues - Receive the first serial - Go to claims page and claim the 2nd serial. - Go back to the subscription page and click on 'Serial collection' - You should have 2 serials, one 'Arrived' and one 'Claimed'. - Click on Generate Next. This should fail with a software error message ("can't call method output ...") - Apply this patch and click again on Generate Next. A new issue must be created with status 'Expected'. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Created attachment 15541 [details] [review] [SIGNED-OFF] Bug 7688: Followup FIX perldoc Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
QA comment: * after so many tests, QA comments and follow-up, it's time to pass QA ;-) * passes koha-qa.pl nothing to add, many comments already done and fixed ! passed QA note, that there is a POD error in updatedatabase: * installer/data/mysql/updatedatabase.pl FAIL pod FAIL *** ERROR: Spurious text after =cut in file installer/data/mysql/updatedatabase.pl but it was already here before the patch, and is fixed in another patch i've passed QA a few hours ago.
Created attachment 16679 [details] [review] [PASSED QA] Bug 7688: Change subscription numbering pattern and frequencies Serials numbering pattern and frequencies are no more hard-coded. Now it's possible to create, edit and delete numbering patterns (and frequencies). This implies new sql tables (subscription_numberpatterns and subscription_frequencies) Numbering patterns behave almost as before, there are still the same values to configure (addX, everyX, settoX, whenmorethanX). lastvalueX and innerloopX remain in subscription tables. There is a new value in numbering patterns: numberingX. For each "column" (X, Y or Z) you can tell how to format the number. Actually numberingX can be set to: - 'dayname' (name of the day) (0-6 or 1-7 depending on which day is the first of the week) - 'monthname' (name of the month) (0-11) - 'season' (name of the season) (0-3) (0 is Spring) These names are localized by using POSIX::setlocale and POSIX::strftime and setting a 'locale' value to the subscription. Locale have to be installed on the system. Note that season names are not localized using POSIX::strftime (it can't do this), so names are hardcoded into the code (available languages: en, fr). This could be fixed in the future by using a Perl localization framework. Frequencies can be configured using 3 parameters: - 'unit': one of 'day', 'week', 'month', 'year' - 'issuesperunit': integer >= 1, the number of received issues per 'unit' - 'unitsperissue': integer >= 1, the number of 'unit' between two issues One of 'issuesperunit' and 'unitsperissue' must be equal to 1. Examples: unit = 'day', issuesperunit=3, unitsperissue=1 => 3 issues per day unit = 'week', issuesperunit=1, unitsperissue=3 => 1 issue each 3 weeks Prediction pattern is now computed server-side and is more consistent with what Koha will do. The publication date is displayed alongside the serial number. Irregularities can now be checked one by one, in the prediction pattern table, or if frequency is 'day-based' (unit is 'day'), there is the possibility to check all issues for a week day at once. When an irregularity is found, there is the possibility to keep the serial number unchanged, or to skip it. It is configured at subscription creation or modification. For instance, with a daily subscription you can have: skip serial number | keep serial number ----------------------+---------------------- 2012-01-01 ¦ No 1 | 2012-01-01 ¦ No 1 2012-01-03 ¦ No 3 | 2012-01-03 ¦ No 2 To lighten the subscription modification page, manual history has been moved in its own page subscription-history.pl which is accessible on subscription-detail.pl, tab 'Planning'. Important note: updatedatabase.pl script takes into account existing subscriptions and create appropriate numbering patterns for them (it tries to create as few patterns as possible). Frequency is mapped to the correct entry in subscription_frequencies table. This patch includes kohastructure.sql and updatedatabase.pl changes + sample frequencies data and sample numberpatterns data for fresh installs (sample data is included in updatedatabase.pl) === TEST PLAN: === Create a new subscription: - Go to Serials module and click "New subscription" button - On the first page, choose a biblio and click next to go to the second page - Pick a first issue publication date - Choose frequency '1/day' - Choose a subscription length of 15 issues - Choose a subscription start date - Choose numbering pattern 'Volume, Number' - A table appears, fill 'Begins with' cells with '1' - Click on 'Test prediction pattern' button The prediction pattern is displayed at the right of the page. You can see in it the serial number, the publication date and a checkbox to allow you to choose which serials will not be received (irregularities). You can see that serial number start from "Vol 1, No 1" continue to "Vol 1, No 12" and then restart with "Vol 2, No 1". Frequency is '1/day' so you can see that publication date is incremented by one day line after line. - Now you can play a little with frequencies and numbering patterns, change one of them (or both) and click again on 'Test prediction pattern' - For example, choose frequency '3/weeks' and click on 'Test prediction pattern' button'. There is a little behaviour change compared with current master. Publication date will not be guessed within the week. Koha can't know when you will receive issues. So the publication date stay the same (monday of each week) for 3 consecutive issues and then jump to the next week. - Now choose frequency '1/3 months' and numbering pattern 'Seasonal' - Fill 'Begins with' cells with '2012' for Year and '0' for Season - Click on 'Test prediction pattern' - You should have something like 'Spring 2012', 'Summer 2012', ..., 'Winter 2012', 'Spring 2013' - Note that you can have seasons for south hemisphere by entering '2' in 'Year/Inner counter' - 2nd note: if you have some locales installed on your system, you can type its name in the 'Locale' field (actually it does not work for seasons name, only for month names and day names) If you want to modify the numbering pattern you can still do it here: - Click on 'Show/Hide advanced pattern' link. The advanced pattern table is shown but all fields are readonly - Click on 'Modify pattern' button. All readonly fields are now editable. Note that 'Begins with' and 'Inner counter' line are repeated here and any modifications in the small table will be replicated in the big table, and vice versa. - Pattern name is emptied, if you type a new name, a new pattern will be created, and if you type the same name as an existing numbering pattern, this one will be modified (with a confirmation message) - There is two new lines in this table: - Label: it's what is displayed in the smaller table headers above - Numbering: used to format numbers in different ways. can be 'seasons', 'monthname' or 'dayname'. Month name and day name can be localized using the 'Locale' field. Seasons can't (values for english and french are hard-coded in Serials.pm) - You can modify what you want in the table and click on 'Test prediction pattern' button each time you want to see your modifications. (Note that checkboxes for irregularities aren't displayed in this mode, and you can't save the subscription until you have saved or cancelled your changes). - To cancel your modifications, just click on 'Cancel modifications' button. - To save them, click on 'Save as new pattern'. If the pattern name is already existing, a confirmation box will ask you if you want to modify the existing numbering pattern. Otherwise a new pattern will be created and automatically selected. Once you have finished modifying numbering pattern. You can click again on 'Test prediction pattern' to define irregularities, and then click on 'Save subscription'. Now you can check the serials module still works correctly: - Check the subscription detail page to confirm that nothing is missing. Especially the 'Frequency' and 'Number pattern' infos - Try to receive some issues. Check that the serial number is correctly generated and if irregularities you have defined are taken into account (if you have defined some). - Check that receiving is blocked once you have reached the number of issues you have defined in subscription length (or once you have reached the subscription end date) In serials menu (to the left of almost each page of serials menu) you have two new links: 'Manage frequencies' and 'Manage numbering patterns'. 'Manage numbering patterns' lead to a page which list all numbering patterns and allow you to create, edit or delete them. The interface is almost the same as numbering pattern modification in subscription-add.pl 'Manage frequencies' lead to a page which list all frequencies and allow you to create, edit or delete them. Try to create a new frequency: - Click on 'Manage frequencies' link in the serials menu and then click on 'New frequency': - Fill in the description (mandatory). - Unit is one of 'day', 'week', 'month', year' or 'None' ('None' is for an irregular subscription) - If unit is different from 'None' you have to fill the two following fields (Issues per unit, and Units per issue) - Note that at least one of those must be equal to 1 - Issues per unit is the number of received issues by 'unit' and Units per issue is the number of 'unit' between two issues - Display order is used to build the drop-down list. Leave empty and it will be set to 0 (top of the list) - Then click on 'Save' - Check that this new frequency appears in the frequencies table and in the drop-down list in subscription-add.pl Subscription history has been moved in its own page. To test if it still works, choose a subscription with manual history enabled (or modify an existing subscription to turn on manual history). - On the detail page, tab 'Planning', you should have a link 'Edit history'. - Click on it - Modify history and click on Save - In tab 'Summary' you should have the infos you just entered And finally, you can check that old subscriptions (by old I mean subscriptions that existed before the update) are correctly linked to an existing numbering pattern and an existing frequency. Numbering patterns should be named 'Backup pattern X' where X is a number. Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comment: Great development! Work as described. No koha-qa errors (with all patches applied). Please QA this fast. Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Created attachment 16680 [details] [review] [PASSED QA] Bug 7688 follow-up: Small fixes for QA - # Subroutines::ProhibitExplicitReturnUndef: Got 1 violation(s) in C4::Serials::GetSubscriptionIrregularities - Bad template constructions fixed in serials/subscription-add.tt Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Created attachment 16681 [details] [review] [PASSED QA] Bug 7688 follow-up: Small fixes for QA #2 - "return undef" -> "return" - ":utf8" -> ":encoding(UTF-8)" - TAB -> SPACES Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Created attachment 16682 [details] [review] [PASSED QA] Bug 7688: Translate sample frequencies for french Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Created attachment 16683 [details] [review] [PASSED QA] Bug 7688: Fix generating next serial when there is no 'Expected' issue It can happen when the Expected issue is claimed. In this case the status of the last serial is 'Claimed' This patch change the API of GetNextSeq and GetSeq Test plan: - Create a subscription which starts a long time ago so that serials automatically appear in late issues - Receive the first serial - Go to claims page and claim the 2nd serial. - Go back to the subscription page and click on 'Serial collection' - You should have 2 serials, one 'Arrived' and one 'Claimed'. - Click on Generate Next. This should fail with a software error message ("can't call method output ...") - Apply this patch and click again on Generate Next. A new issue must be created with status 'Expected'. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Created attachment 16684 [details] [review] [PASSED QA] Bug 7688: Followup FIX perldoc Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
This looks very, very promising. Except there's a problem. When I edit an existing subscription, I end up in an impossible situation: http://screencast.com/t/49ZDRNGGKHI 1) There is no box for "First issue publication date," which means I can't test the prediction 2) It doesn't display the numbering pattern that was associated with this subscription (which is saved as Backup pattern 1). 3) Finally, wouldn't it be a good idea to have a "Custom" numbering pattern or something like that for when librarians will be creating a custom pattern? Right now it is very unclear how to do that.
Created attachment 16744 [details] [review] Bug 7688: Set firstacquidate in updatedatabase if it's not set firstacquidate is now a mandatory field, so it must be set. The updatedatabase process set firstacquidate value to one of: - the planned date of the first serial for this subscription, or if not found, - the subscription start date. + auto-select the numbering pattern when modifying a subscription.
Hi Jared, this patch is for 1) and 2) 3) It is possible to create a new numbering pattern from the subscription add/modify screen. Just click on 'Show advanced pattern', then click on 'Modify pattern' button. Fill the new pattern name and change the pattern values, then click on 'Save as new pattern'. Do you mean you want to add a "Custom" option to the numbering pattern list that automatically show the advanced pattern form ? I switch back the patch status to Passed QA so you can see it, change it to Needs signoff if you think it must be signed.
(In reply to comment #84) > this patch is for 1) and 2) Thanks. > 3) It is possible to create a new numbering pattern from the subscription > add/modify screen. Just click on 'Show advanced pattern', then click on > 'Modify pattern' button. Fill the new pattern name and change the pattern > values, then click on 'Save as new pattern'. > Do you mean you want to add a "Custom" option to the numbering pattern list > that automatically show the advanced pattern form ? Ah-ha. I don't think it even needs to automatically show the advanced pattern form. I just wanted some sort of blank numbering pattern that doesn't set anything so that the form isn't instructing me to select a numbering pattern from among a series of inappropriate options. > I switch back the patch status to Passed QA so you can see it, change it to > Needs signoff if you think it must be signed. The follow-up looks pretty simple, so I don't think I'll require another sign-off.
This patch has been pushed to master.
These patches have been reverted from master. The caused t/db_dependent/Serials.t to fail: not ok 8 - test getting history from sub-scription Bumping back for 3.14.
Created attachment 17283 [details] [review] [PASSED QA] Bug 7688: Change subscription numbering pattern and frequencies This is a squashed and rebased version of all patches that were previously marked [PASSED QA]
Created attachment 17284 [details] [review] Bug 7688: Set firstacquidate in updatedatabase if it's not set Rebased on master
Created attachment 17285 [details] [review] Bug 7688: Rewrite t/db_dependant/Serials.t A lot of tests were just a call to a funtion without any arguments, whereas the function expect at least one argument. These tests were kept, but all return values are now undef when a mandatory argument is missing, so return values are consistent. The part where subscription periodicity is changed could not work because of ',' appended to each key in ModSubscription call. So it's rewritten, taking into account the new API for subscription frequencies. This script should leave your database intact because it revert any modification made. Also fix some warnings in C4::Serials and in C4::Items. And fix a typo in koha-tmpl/.../subscription-numberpatterns.tt
Created attachment 17855 [details] [review] [SIGNED-OFF] Bug 7688: Change subscription numbering pattern and frequencies Serials numbering pattern and frequencies are no more hard-coded. Now it's possible to create, edit and delete numbering patterns (and frequencies). This implies new sql tables (subscription_numberpatterns and subscription_frequencies) Numbering patterns behave almost as before, there are still the same values to configure (addX, everyX, settoX, whenmorethanX). lastvalueX and innerloopX remain in subscription tables. There is a new value in numbering patterns: numberingX. For each "column" (X, Y or Z) you can tell how to format the number. Actually numberingX can be set to: - 'dayname' (name of the day) (0-6 or 1-7 depending on which day is the first of the week) - 'monthname' (name of the month) (0-11) - 'season' (name of the season) (0-3) (0 is Spring) These names are localized by using POSIX::setlocale and POSIX::strftime and setting a 'locale' value to the subscription. Locale have to be installed on the system. Note that season names are not localized using POSIX::strftime (it can't do this), so names are hardcoded into the code (available languages: en, fr). This could be fixed in the future by using a Perl localization framework. Frequencies can be configured using 3 parameters: - 'unit': one of 'day', 'week', 'month', 'year' - 'issuesperunit': integer >= 1, the number of received issues per 'unit' - 'unitsperissue': integer >= 1, the number of 'unit' between two issues One of 'issuesperunit' and 'unitsperissue' must be equal to 1. Examples: unit = 'day', issuesperunit=3, unitsperissue=1 => 3 issues per day unit = 'week', issuesperunit=1, unitsperissue=3 => 1 issue each 3 weeks Prediction pattern is now computed server-side and is more consistent with what Koha will do. The publication date is displayed alongside the serial number. Irregularities can now be checked one by one, in the prediction pattern table, or if frequency is 'day-based' (unit is 'day'), there is the possibility to check all issues for a week day at once. When an irregularity is found, there is the possibility to keep the serial number unchanged, or to skip it. It is configured at subscription creation or modification. For instance, with a daily subscription you can have: skip serial number | keep serial number ----------------------+---------------------- 2012-01-01 ¦ No 1 | 2012-01-01 ¦ No 1 2012-01-03 ¦ No 3 | 2012-01-03 ¦ No 2 To lighten the subscription modification page, manual history has been moved in its own page subscription-history.pl which is accessible on subscription-detail.pl, tab 'Planning'. Important note: updatedatabase.pl script takes into account existing subscriptions and create appropriate numbering patterns for them (it tries to create as few patterns as possible). Frequency is mapped to the correct entry in subscription_frequencies table. This patch includes kohastructure.sql and updatedatabase.pl changes + sample frequencies data and sample numberpatterns data for fresh installs (sample data is included in updatedatabase.pl) === TEST PLAN: === Create a new subscription: - Go to Serials module and click "New subscription" button - On the first page, choose a biblio and click next to go to the second page - Pick a first issue publication date - Choose frequency '1/day' - Choose a subscription length of 15 issues - Choose a subscription start date - Choose numbering pattern 'Volume, Number' - A table appears, fill 'Begins with' cells with '1' - Click on 'Test prediction pattern' button The prediction pattern is displayed at the right of the page. You can see in it the serial number, the publication date and a checkbox to allow you to choose which serials will not be received (irregularities). You can see that serial number start from "Vol 1, No 1" continue to "Vol 1, No 12" and then restart with "Vol 2, No 1". Frequency is '1/day' so you can see that publication date is incremented by one day line after line. - Now you can play a little with frequencies and numbering patterns, change one of them (or both) and click again on 'Test prediction pattern' - For example, choose frequency '3/weeks' and click on 'Test prediction pattern' button'. There is a little behaviour change compared with current master. Publication date will not be guessed within the week. Koha can't know when you will receive issues. So the publication date stay the same (monday of each week) for 3 consecutive issues and then jump to the next week. - Now choose frequency '1/3 months' and numbering pattern 'Seasonal' - Fill 'Begins with' cells with '2012' for Year and '0' for Season - Click on 'Test prediction pattern' - You should have something like 'Spring 2012', 'Summer 2012', ..., 'Winter 2012', 'Spring 2013' - Note that you can have seasons for south hemisphere by entering '2' in 'Year/Inner counter' - 2nd note: if you have some locales installed on your system, you can type its name in the 'Locale' field (actually it does not work for seasons name, only for month names and day names) If you want to modify the numbering pattern you can still do it here: - Click on 'Show/Hide advanced pattern' link. The advanced pattern table is shown but all fields are readonly - Click on 'Modify pattern' button. All readonly fields are now editable. Note that 'Begins with' and 'Inner counter' line are repeated here and any modifications in the small table will be replicated in the big table, and vice versa. - Pattern name is emptied, if you type a new name, a new pattern will be created, and if you type the same name as an existing numbering pattern, this one will be modified (with a confirmation message) - There is two new lines in this table: - Label: it's what is displayed in the smaller table headers above - Numbering: used to format numbers in different ways. can be 'seasons', 'monthname' or 'dayname'. Month name and day name can be localized using the 'Locale' field. Seasons can't (values for english and french are hard-coded in Serials.pm) - You can modify what you want in the table and click on 'Test prediction pattern' button each time you want to see your modifications. (Note that checkboxes for irregularities aren't displayed in this mode, and you can't save the subscription until you have saved or cancelled your changes). - To cancel your modifications, just click on 'Cancel modifications' button. - To save them, click on 'Save as new pattern'. If the pattern name is already existing, a confirmation box will ask you if you want to modify the existing numbering pattern. Otherwise a new pattern will be created and automatically selected. Once you have finished modifying numbering pattern. You can click again on 'Test prediction pattern' to define irregularities, and then click on 'Save subscription'. Now you can check the serials module still works correctly: - Check the subscription detail page to confirm that nothing is missing. Especially the 'Frequency' and 'Number pattern' infos - Try to receive some issues. Check that the serial number is correctly generated and if irregularities you have defined are taken into account (if you have defined some). - Check that receiving is blocked once you have reached the number of issues you have defined in subscription length (or once you have reached the subscription end date) In serials menu (to the left of almost each page of serials menu) you have two new links: 'Manage frequencies' and 'Manage numbering patterns'. 'Manage numbering patterns' lead to a page which list all numbering patterns and allow you to create, edit or delete them. The interface is almost the same as numbering pattern modification in subscription-add.pl 'Manage frequencies' lead to a page which list all frequencies and allow you to create, edit or delete them. Try to create a new frequency: - Click on 'Manage frequencies' link in the serials menu and then click on 'New frequency': - Fill in the description (mandatory). - Unit is one of 'day', 'week', 'month', year' or 'None' ('None' is for an irregular subscription) - If unit is different from 'None' you have to fill the two following fields (Issues per unit, and Units per issue) - Note that at least one of those must be equal to 1 - Issues per unit is the number of received issues by 'unit' and Units per issue is the number of 'unit' between two issues - Display order is used to build the drop-down list. Leave empty and it will be set to 0 (top of the list) - Then click on 'Save' - Check that this new frequency appears in the frequencies table and in the drop-down list in subscription-add.pl Subscription history has been moved in its own page. To test if it still works, choose a subscription with manual history enabled (or modify an existing subscription to turn on manual history). - On the detail page, tab 'Planning', you should have a link 'Edit history'. - Click on it - Modify history and click on Save - In tab 'Summary' you should have the infos you just entered And finally, you can check that old subscriptions (by old I mean subscriptions that existed before the update) are correctly linked to an existing numbering pattern and an existing frequency. Numbering patterns should be named 'Backup pattern X' where X is a number. Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comment: Great development! Work as described. No koha-qa errors (with all patches applied). Please QA this fast. Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Squashed commits: ----------------- Bug 7688 follow-up: Small fixes for QA - # Subroutines::ProhibitExplicitReturnUndef: Got 1 violation(s) in C4::Serials::GetSubscriptionIrregularities - Bad template constructions fixed in serials/subscription-add.tt Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> ---- Bug 7688 follow-up: Small fixes for QA #2 - "return undef" -> "return" - ":utf8" -> ":encoding(UTF-8)" - TAB -> SPACES Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> ---- Bug 7688: Translate sample frequencies for french Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> ---- Bug 7688: Fix generating next serial when there is no 'Expected' issue It can happen when the Expected issue is claimed. In this case the status of the last serial is 'Claimed' This patch change the API of GetNextSeq and GetSeq Test plan: - Create a subscription which starts a long time ago so that serials automatically appear in late issues - Receive the first serial - Go to claims page and claim the 2nd serial. - Go back to the subscription page and click on 'Serial collection' - You should have 2 serials, one 'Arrived' and one 'Claimed'. - Click on Generate Next. This should fail with a software error message ("can't call method output ...") - Apply this patch and click again on Generate Next. A new issue must be created with status 'Expected'. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> ---- Bug 7688: Followup FIX perldoc Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comment on last patch.
Created attachment 17856 [details] [review] [SIGNED-OFF] Bug 7688: Set firstacquidate in updatedatabase if it's not set firstacquidate is now a mandatory field, so it must be set. The updatedatabase process set firstacquidate value to one of: - the planned date of the first serial for this subscription, or if not found, - the subscription start date. + auto-select the numbering pattern when modifying a subscription. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comment on last patch
Created attachment 17857 [details] [review] [SIGNED-OFF] Bug 7688: Rewrite t/db_dependant/Serials.t A lot of tests were just a call to a funtion without any arguments, whereas the function expect at least one argument. These tests were kept, but all return values are now undef when a mandatory argument is missing, so return values are consistent. The part where subscription periodicity is changed could not work because of ',' appended to each key in ModSubscription call. So it's rewritten, taking into account the new API for subscription frequencies. This script should leave your database intact because it revert any modification made. Also fix some warnings in C4::Serials and in C4::Items. And fix a typo in koha-tmpl/.../subscription-numberpatterns.tt Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comment: Work as described. No errors Tested, again, without trouble. With a suscription, tests successful. No koha-qa errors
Created attachment 18298 [details] [review] [PASSED-QA] Bug 7688: Change subscription numbering pattern and frequencies Rebased on master
Created attachment 18299 [details] [review] [SIGNED-OFF] Bug 7688: Set firstacquidate in updatedatabase if it's not set firstacquidate is now a mandatory field, so it must be set. The updatedatabase process set firstacquidate value to one of: - the planned date of the first serial for this subscription, or if not found, - the subscription start date. + auto-select the numbering pattern when modifying a subscription. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Created attachment 18300 [details] [review] [SIGNED-OFF] Bug 7688: Rewrite t/db_dependant/Serials.t A lot of tests were just a call to a funtion without any arguments, whereas the function expect at least one argument. These tests were kept, but all return values are now undef when a mandatory argument is missing, so return values are consistent. The part where subscription periodicity is changed could not work because of ',' appended to each key in ModSubscription call. So it's rewritten, taking into account the new API for subscription frequencies. This script should leave your database intact because it revert any modification made. Also fix some warnings in C4::Serials and in C4::Items. And fix a typo in koha-tmpl/.../subscription-numberpatterns.tt Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comment: Work as described. No errors Tested, again, without trouble. With a suscription, tests successful. No koha-qa errors
Created attachment 18793 [details] [review] [PASSED-QA] Bug 7688: Change subscription numbering pattern and frequencies Rebased on master
Created attachment 18794 [details] [review] [SIGNED-OFF] Bug 7688: Set firstacquidate in updatedatabase if it's not set Rebased on master
Created attachment 18795 [details] [review] [SIGNED-OFF] Bug 7688: Rewrite t/db_dependant/Serials.t Rebased on master
This patch had already been QAed by me a long time ago. It also has been validated by Jared, then reverted because of a jenkins failure. The new follow-up is about test, and Bernardo has validated it once again. I think it's fair to switch to passed QA, so switching (Galen, this patch is a pain to rebase, please don't let it wait)
This is a huge patch. I am a bit worried of incorporating this without feedback from a library using serials and subscriptions - especially worried about the database update. Is it possible that we could get an anonymizsed database from a library with lots of subscriptions to test with?
(In reply to Katrin Fischer from comment #101) > This is a huge patch. I am a bit worried of incorporating this without > feedback from a library using serials and subscriptions - especially worried > about the database update. Is it possible that we could get an anonymizsed > database from a library with lots of subscriptions to test with? Katrin, this improvement is used by 5 of our customers since more than one year. The feature has been signed off by frederic Demians, Kyle, and Bernardo. Not counting my (as QAer) tests, and Jared ones (as he pushed the patch before reverting it) So I think we should go forward, and push this patch (otherwise it will never be pushed).
(In reply to Katrin Fischer from comment #101) > This is a huge patch. I am a bit worried of incorporating this without > feedback from a library using serials and subscriptions - especially worried > about the database update. Is it possible that we could get an anonymizsed > database from a library with lots of subscriptions to test with? I have a Koha database available to me with a large number of subscriptions that will be suitable for verifying that no regressions occur during upgrade, and I will be using that during testing. By the way, at the point in time that I am reading to push this patch, I intend to include a follow-up that makes the SQL for frequencies and numbering patterns mandatory during installation. If a user doesn't plan to use serials, populating the table doesn't hurt; if they do plan to use serials, making them start from scratch if they didn't run those scripts during installation only increases the height of the bar for a module that is inherently already difficult.
(In reply to Paul Poulain from comment #102) > (In reply to Katrin Fischer from comment #101) > > This is a huge patch. I am a bit worried of incorporating this without > > feedback from a library using serials and subscriptions - especially worried > > about the database update. Is it possible that we could get an anonymizsed > > database from a library with lots of subscriptions to test with? > > Katrin, this improvement is used by 5 of our customers since more than one > year. The feature has been signed off by frederic Demians, Kyle, and > Bernardo. > Not counting my (as QAer) tests, and Jared ones (as he pushed the patch > before reverting it) > So I think we should go forward, and push this patch (otherwise it will > never be pushed). Let me be clear: I will always exercise my own technical judgment. The fact that others have reviewed a patch is necessary but not sufficient for me to push it. That said, upon looking at this, I am seeing a lot that I like about this approach, but given that the main patch revamps the core of serials prediction without adding any new test cases, I will have to write some myself as part of validating the patch. This won't be instantaneous, but it won't take forever, either.
Created attachment 19368 [details] [review] Bug 7688: Add unit tests for C4::Serials::Frequency
Created attachment 19369 [details] [review] Bug 7688: Add unit tests for C4::Serials::Numberpattern
Created attachment 19370 [details] [review] Bug 7688: Add unit tests for C4::Serials::GetNextDate()
Here are some unit tests for the two added Perl modules, plus unit tests for GetNextDate(). I'm currently working on unit tests for GetNextSeq().
Created attachment 19393 [details] [review] Bug 7688: Add unit tests for C4::Serials::GetNextSeq() These tests are splitted in two parts: "db dependent" and "db independent". GetNextSeq requires DB only when checking for irregularities.
Created attachment 19395 [details] [review] Bug 7688: Rewrite 'for' loop in unit tests
*** Bug 3681 has been marked as a duplicate of this bug. ***
Pushed to master, along with the usual bunch of follow-ups. Thanks, Julian!