TEST PLAN

Part 1:
1) Apply patch and run updatedatabase.pl
2) Verify the new fields have been created. The new serialseq fields should be the correct data, assuming the serials are following Koha standards. NOTE: It is not necessary for this data to be absolutely correct. This data is really for statistical purposes at the moment, but do pave the way for future serials improvements.

Part 2:
1) Apply the patch
2) Create a new subscription, and add serials to it. The new x y and z fields should be populated with the same data the replaces {X} {Y} and {Z} in the enumeration field.

Part 3:
1) Apply the patch
2) Add/update a serial, and choose dates via the datepicker.
3) Verify the data is saved correctly in the database.

Part 4:
1) Apply the patch. Add/update a serial, and choose a published end date.
2) Verify the date was stored in the database correctly ( serials.published_end_date )
NOTE: The purpose of this feature is to better handle the dates of bound volumes of serials. Right now, we specify the first published date, but not the published date of the last issue in a bound volume.

Part 5-7:
1) Apply the patches
2) Create a new serial. You can now see that instead of having the labels described in the enumeration field, we have three new replacement variables {XL}, {YL} and {ZL}.
3) Generate a new serial and verify the placeholders are replaced with the correct labels.
Note: This feature will not affect existing serials, so it should be upgrade safe.

I think this bug needs some exposition. We have libraries that are finding it difficult to generate reports for serials for the following reasons:
1) Parsing serialseq to get issue/vol/no/whatever data is difficult
2) Librarians put incorrectly formatted dates in the date fields
3) There is no proper way to store the published date of the last issue of a bound volume.

Parts 1 through 4 are the only critical patches to resolve these issue. Parts 5-7 are just something I thought would be a nice way to advance the serials module. In the future, it may be possible to stop using serialseq and use the new fields in serial and subscription to display that data.

First patch does not apply.
So, don't try to apply others.

I have created a subscription with numbering pattern to "Volume, number" and set:
{X} Label: Year
{Y} Label: Number

I cre

I created a serial and description is ok: "Year 2013, Number 1".
But if i edit this subscription again, x and y label are reset to "Vol" and "Number". But serials are not affected. Adding a new one create a description like "Year 2013, Number 2".

Also, it could be better the Test prediction pattern use defined labels instead of "{XL} 2013, {YL} 1"

Kyle, after review by our serials staff I now sign off on this dev.

Paul Landers
Texas Tech University Health Sciences Center
Library I.T.

Created attachment 18576 [details] [review]
[Signed off] Bug 8956 - Split serials enumeration data into separate fields - Part 1 - Add fields to DB

Signed-off-by: Paul Landers <paul.landers@ttuhsc.edu>

Created attachment 18577 [details] [review]
[Signed off] Bug 8956 - Split serials enumeration data into separate fields - Part 2 - Store x, y and z in DB when generating new serials

Signed-off-by: Paul Landers <paul.landers@ttuhsc.edu>

Created attachment 18578 [details] [review]
[Signed off] Bug 8956 - Split serials enumeration data into separate fields - Part 3 - Add date pickers to serials-edit.pl

Signed-off-by: Paul Landers <paul.landers@ttuhsc.edu>

Created attachment 18579 [details] [review]
[Signed off] Bug 8956 - Split serials enumeration data into separate fields - Part 4 - Add saving/editing of published end date for bound volumes

Signed-off-by: Paul Landers <paul.landers@ttuhsc.edu>

Created attachment 18580 [details] [review]
[Signed off] Bug 8956 - Split serials enumeration data into separate fields - Part 5 - Add label fields to subscription editor.

Signed-off-by: Paul Landers <paul.landers@ttuhsc.edu>

Created attachment 18581 [details] [review]
[Signed off] Bug 8956 - Split serials enumeration data into separate fields - Part 6 - Perltidy serials/subscription-add.pl

Signed-off-by: Paul Landers <paul.landers@ttuhsc.edu>

Created attachment 18582 [details] [review]
[Signed off] Bug 8956 - Split serials enumeration data into separate fields - Part 7 - Add backend processing of field labels.

Signed-off-by: Paul Landers <paul.landers@ttuhsc.edu>

Created attachment 18583 [details] [review]
[Signed off] Bug 8956 - Split serials enumeration data into separate fields - Part 8 - Perltidy serials/subscription-add.pl again

Signed-off-by: Paul Landers <paul.landers@ttuhsc.edu>

Helping Paul add his sign off.

Just some advance remarks:

In total this is a larger patch. It could be advantageous if Paul as the one signing off, could further elaborate on what he exactly tested. Did you test various numbering patterns, how much data was converted in the upgrade, etc. etc. In this way we can build up some more confidence in this development.

I just had a quick look:

The upgrade reports lines like : NUMBERING METHOD: '{X}' Does not seem to be very informative?
The label X, etc with Appears in numbering formula as {XL} do not seem very easy to understand on Edit subscription? [This already held for numbering patterns, but life is not simpler now..]
Funny date in next issue publication date: 02/00/0 Do not know what happened here, but could be an existing problem..

Thanks.

Created attachment 18679 [details] [review]
Bug 8956 - Split serials enumeration data into separate fields - Part 9 - Remove print from database update

He followed the test plan in part 8.

(In reply to comment #54)
> Just some advance remarks:
> 
> In total this is a larger patch. It could be advantageous if Paul as the one
> signing off, could further elaborate on what he exactly tested. Did you test
> various numbering patterns, how much data was converted in the upgrade, etc.
> etc. In this way we can build up some more confidence in this development.
> 
> I just had a quick look:
> 
> The upgrade reports lines like : NUMBERING METHOD: '{X}' Does not seem to be
> very informative?
> The label X, etc with Appears in numbering formula as {XL} do not seem very
> easy to understand on Edit subscription? [This already held for numbering
> patterns, but life is not simpler now..]
> Funny date in next issue publication date: 02/00/0 Do not know what happened
> here, but could be an existing problem..
> 
> Thanks.

The test plan in comment 8 that is ; )

I've also attached a qa followup to remove that print line from the db update.


(In reply to comment #56)
> He followed the test plan in part 8.
> 
> (In reply to comment #54)
> > Just some advance remarks:
> > 
> > In total this is a larger patch. It could be advantageous if Paul as the one
> > signing off, could further elaborate on what he exactly tested. Did you test
> > various numbering patterns, how much data was converted in the upgrade, etc.
> > etc. In this way we can build up some more confidence in this development.
> > 
> > I just had a quick look:
> > 
> > The upgrade reports lines like : NUMBERING METHOD: '{X}' Does not seem to be
> > very informative?
> > The label X, etc with Appears in numbering formula as {XL} do not seem very
> > easy to understand on Edit subscription? [This already held for numbering
> > patterns, but life is not simpler now..]
> > Funny date in next issue publication date: 02/00/0 Do not know what happened
> > here, but could be an existing problem..
> > 
> > Thanks.

For Paul's testing, he created 2 new serials using an X Y Z numbering pattern (I believe he tried Volume, Number, Issue, as well as Volume, Number, Year, if I remember correctly).  He verified that the new fields were showing up, and he tried using the datepicker a few times.  He also edited an existing serial to make sure the new fields looked ok.

(I was watching with screen sharing while he went through the test plan, and then I helped him with the Git signoff.)

I think this change might be in conflict with the big serials pattern patch that has reached 'Passed QA' already (bug 7688).

(In reply to Katrin Fischer from comment #59)
> I think this change might be in conflict with the big serials pattern patch
> that has reached 'Passed QA' already (bug 7688).

I already talk about this possible problem with Julian (author of the feature 7688). On a functional point of view the 2 features are not in conflict but, inevitably, the patches will not apply cleanly.

(In reply to Jonathan Druart from comment #60)
> (In reply to Katrin Fischer from comment #59)
> > I think this change might be in conflict with the big serials pattern patch
> > that has reached 'Passed QA' already (bug 7688).
> 
> I already talk about this possible problem with Julian (author of the
> feature 7688). On a functional point of view the 2 features are not in
> conflict but, inevitably, the patches will not apply cleanly.

I suppose the big question is: which of the two will be easier to rebase over top the other.

Starting with a code review based on squashed patches:

1) Bug number missing from database update

2) In subscription-add.pl there is a typo, not sure the consequences:
   -    $templte->param(letterloop => $letterloop);
   +    $templte->param( letterloop => $letterloop );

3) I'd really like to have more information about the use of the 
   published_end_date and bound volumes - this seems to be a separate 
   feature. The test plan says:

>Part 4:
>1) Apply the patch. Add/update a serial, and choose a published end date.
>2) Verify the date was stored in the database correctly ( >serials.published_end_date )
>NOTE: The purpose of this feature is to better handle the dates of bound >volumes of serials. Right now, we specify the first published date, but not the >published date of the last issue in a bound volume.

There is currently no real workflow for adding bound volumes - what are you referring to here? What steps lead to a bound volume for this library?

--
I am a bit worried about the amount of testing this has seen - was an existing database converted with this patch? What were the results? Were other more unusual subscription patterns tested as well?

Hi Kyle, 

failing to get your attention and answers to my questions :)

*** This bug has been marked as a duplicate of bug 12375 ***