Bug 7677 - Subscriptions: Ability to define default itemtype and automatically change itemtype of older issues on receive of next issue
Summary: Subscriptions: Ability to define default itemtype and automatically change it...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Matthias Meusburger
QA Contact: Testopia
URL:
Keywords:
Depends on: 7688 12161
Blocks: 20461
  Show dependency treegraph
 
Reported: 2012-03-08 14:05 UTC by delaye
Modified: 2018-10-19 14:38 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 7677: New areas in subscriptions and new functions when receiving. (27.72 KB, patch)
2013-03-19 09:43 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7677: New areas in subscriptions and new functions when receiving. (28.02 KB, patch)
2013-03-19 16:32 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7677: New areas in subscriptions and new functions when receiving. (28.08 KB, patch)
2013-03-20 08:54 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 7677: New areas in subscriptions and new functions when receiving. (28.31 KB, patch)
2013-08-21 12:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7677: UT: add unit tests for GetPreviousSerialid (1.55 KB, patch)
2013-08-21 12:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
proposed patch (19.95 KB, patch)
2014-01-09 16:14 UTC, Matthias Meusburger
Details | Diff | Splinter Review
proposed patch (17.54 KB, patch)
2014-01-23 08:51 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7677: UT: add unit tests for GetPreviousSerialid (1.35 KB, patch)
2014-03-10 11:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
BZ7677: New areas in subscriptions and new functions when receiving. (17.67 KB, patch)
2014-03-10 11:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7677: FIX QA issues (2.08 KB, patch)
2014-03-10 11:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Make the newly received serial gets its default itemtype from the subscription. (1.22 KB, patch)
2014-04-29 13:19 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7677: UT: add unit tests for GetPreviousSerialid (1.33 KB, patch)
2014-06-18 15:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
BZ7677: New areas in subscriptions and new functions when receiving. (17.63 KB, patch)
2014-06-18 15:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7677: FIX QA issues (2.08 KB, patch)
2014-06-18 15:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
BZ7677 [Follow-up]: New areas in subscriptions and new functions when receiving. (1.22 KB, patch)
2014-06-18 15:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
BZ7677: New areas in subscriptions and new functions when receiving. (17.68 KB, patch)
2014-07-08 10:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7677: FIX QA issues (2.08 KB, patch)
2014-07-08 10:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
BZ7677 [Follow-up]: New areas in subscriptions and new functions when receiving. (1.22 KB, patch)
2014-07-08 10:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7677: UT: add unit tests for GetPreviousSerialid (1.33 KB, patch)
2014-12-09 13:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
BZ7677: New areas in subscriptions and new functions when receiving. (17.86 KB, patch)
2014-12-09 13:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7677: FIX QA issues (2.07 KB, patch)
2014-12-09 13:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
BZ7677 [Follow-up]: New areas in subscriptions and new functions when receiving. (1.22 KB, patch)
2014-12-09 13:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7677: Fix tests (2.06 KB, patch)
2014-12-23 15:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
BZ7677 [Follow-up]: New areas in subscriptions and new functions when receiving. (1.05 KB, patch)
2015-03-03 11:05 UTC, Matthias Meusburger
Details | Diff | Splinter Review
BZ7677: New areas in subscriptions and new functions when receiving. (17.85 KB, patch)
2015-03-04 10:49 UTC, Matthias Meusburger
Details | Diff | Splinter Review
BZ7677: New areas in subscriptions and new functions when receiving. (17.85 KB, patch)
2015-03-04 10:58 UTC, Matthias Meusburger
Details | Diff | Splinter Review
BZ7677: New areas in subscriptions and new functions when receiving. (17.85 KB, patch)
2015-03-04 10:59 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7677: UT: add unit tests for GetPreviousSerialid (1.33 KB, patch)
2015-03-04 10:59 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7677: FIX QA issues (950 bytes, patch)
2015-03-04 10:59 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7677: Fix tests (2.07 KB, patch)
2015-03-04 11:00 UTC, Matthias Meusburger
Details | Diff | Splinter Review
BZ7677 [Follow-up]: New areas in subscriptions and new functions when receiving. (1.05 KB, patch)
2015-03-04 11:00 UTC, Matthias Meusburger
Details | Diff | Splinter Review
BZ7677: New areas in subscriptions and new functions when receiving. (17.97 KB, patch)
2015-03-06 11:13 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 7677: UT: add unit tests for GetPreviousSerialid (1.45 KB, patch)
2015-03-06 11:13 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 7677: FIX QA issues (1.04 KB, patch)
2015-03-06 11:13 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 7677: Fix tests (2.18 KB, patch)
2015-03-06 11:13 UTC, Brendan Gallagher
Details | Diff | Splinter Review
BZ7677 [Follow-up]: New areas in subscriptions and new functions when receiving. (1.17 KB, patch)
2015-03-06 11:13 UTC, Brendan Gallagher
Details | Diff | Splinter Review
BZ7677 [Follow-up]: New areas in subscriptions and new functions when receiving. (5.35 KB, patch)
2015-09-28 14:30 UTC, Matthias Meusburger
Details | Diff | Splinter Review
BZ7677 [Follow-up]: New areas in subscriptions and new functions when receiving. (6.64 KB, patch)
2015-09-28 14:39 UTC, Matthias Meusburger
Details | Diff | Splinter Review
BZ7677: New areas in subscriptions and new functions when receiving. (17.96 KB, patch)
2015-12-23 10:50 UTC, Matthias Meusburger
Details | Diff | Splinter Review
BZ7677: New areas in subscriptions and new functions when receiving. (17.96 KB, patch)
2015-12-23 10:50 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7677: UT: add unit tests for GetPreviousSerialid (1.42 KB, patch)
2015-12-23 10:51 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7677: FIX QA issues (1.04 KB, patch)
2015-12-23 10:51 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7677: Fix tests (2.21 KB, patch)
2015-12-23 10:51 UTC, Matthias Meusburger
Details | Diff | Splinter Review
BZ7677 [Follow-up]: New areas in subscriptions and new functions when receiving. (5.41 KB, patch)
2015-12-23 10:52 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7677 [Follow-up]: New areas in subscriptions and new functions when receiving. (6.08 KB, patch)
2016-01-04 07:52 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 7677 [QA Followup] - Replace GetItemnumberFromSerialId with use of Koha::Serial::Item(s) (5.65 KB, patch)
2016-01-29 18:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7677 [QA Followup] - Replace GetItemnumberFromSerialId with use of Koha::Serial::Item(s) (755 bytes, patch)
2016-06-03 13:59 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7677: QA followup (1.24 KB, patch)
2016-08-25 14:09 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7677: QA followup (1.50 KB, patch)
2016-08-30 08:42 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7677: UT: Remove $dbh->rollback (776 bytes, patch)
2016-09-08 07:16 UTC, Julian Maurice
Details | Diff | Splinter Review
[PASSED QA] BZ7677: New areas in subscriptions and new functions when receiving. (21.78 KB, patch)
2016-09-10 11:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 7677: UT: add unit tests for GetPreviousSerialid (1.49 KB, patch)
2016-09-10 11:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 7677: FIX QA issues (1.11 KB, patch)
2016-09-10 11:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 7677: Fix tests (2.28 KB, patch)
2016-09-10 11:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 7677 [Follow-up]: New areas in subscriptions and new functions when receiving. (6.14 KB, patch)
2016-09-10 11:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 7677 [QA Followup] - Replace GetItemnumberFromSerialId with use of Koha::Serial::Item(s) (5.72 KB, patch)
2016-09-10 11:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 7677 [QA Followup] - Replace GetItemnumberFromSerialId with use of Koha::Serial::Item(s) (820 bytes, patch)
2016-09-10 11:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 7677: QA followup (1.31 KB, patch)
2016-09-10 11:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 7677: QA followup (1.56 KB, patch)
2016-09-10 11:21 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 7677: UT: Remove $dbh->rollback (846 bytes, patch)
2016-09-10 11:21 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description delaye 2012-03-08 14:05:59 UTC
In the subscriptions new areas were added.

    itype for the last numero (995$r) for the last item received
    itype for the previous numero (995$r) for the previous item received

When receiving new features :

    1.
       the Issue number is automatically copied into $ 995 v (field "Issue No." copy of the record).
    2.
       The  itype for the last numero in the subscription is assigned to the last subscription copy received
    3.
       the previous copy is changed :  will be awarded the itype for the previous numero who is signaled in the subscription

    4.
      the previous copy is changed  : will be awarded the code notforloan 0 'Avaible'

The last action cited is possible only if the preference system makePreviousSerialAvailable is activated
Comment 1 Matthias Meusburger 2013-03-19 09:30:16 UTC
Test plan:

Once the makePreviousSerialAvailable syspref is enabled, receive a serial, and then another, then check that:

 - the first received itemtype has been set to the "previous item type" value (set in the subscription).

 - the first received has been made available.

 - the last received serial itemtype has been set to the "item type" value (set in the subscription).

 - 995$l is automatically prefilled.


Configure the serialsFieldsToAV syspref. When creating or editing a subscription, check that:

 - the domain and/or origin and/or support fields are correctly displaying the authorized values configured in the syspref.
Comment 2 Matthias Meusburger 2013-03-19 09:43:20 UTC Comment hidden (obsolete)
Comment 3 Matthias Meusburger 2013-03-19 16:32:47 UTC Comment hidden (obsolete)
Comment 4 Matthias Meusburger 2013-03-19 16:33:52 UTC
This patch adds explanations about how the item-level_itypes works with makePreviousSerialAvailable.
Comment 5 Paul Poulain 2013-03-20 08:54:04 UTC
Patch tested with a sandbox, by Frederic Durand <frederic.durand@unilim.fr>
Comment 6 Paul Poulain 2013-03-20 08:54:26 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2013-03-24 21:28:52 UTC
Comment on attachment 16456 [details] [review]
Bug 7677: New areas in subscriptions and new functions when receiving.

Review of attachment 16456 [details] [review]:
-----------------------------------------------------------------

This feature will have to wait for 3.14, but I did take a look at the code today and wanted to leave some notes.

::: C4/Serials.pm
@@ +45,4 @@
>        &ReNewSubscription  &GetLateIssues      &GetLateOrMissingIssues
>        &GetSerialInformation                   &AddItem2Serial
>        &PrepareSerialsData &GetNextExpected    &ModNextExpected
> +      &GetPreviousSerialid

Needs a unit test!

::: installer/data/mysql/updatedatabase.pl
@@ +6570,5 @@
> +    print "Upgrade to $DBversion done (added new syspref: serialsFieldsToAV)\n";
> +    SetVersion($DBversion);
> +}
> +
> +

Database updates should be merged into 1 update with a nice description.

::: koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-edit.tt
@@ +75,5 @@
>  	subfieldid = "subfield" + itemid.substr(4) + "h";
>  	// Setting text field 
>  	$("#" + subfieldid + " input[type='text']").val($("#serialseq" + serialId).val());
> +        // Also prefilling 995$l
> +        subfieldid = "subfield" + itemid.substr(4) + "l";

This looks like it's UNIMARC specific?

::: serials/subscription-add.pl
@@ +193,5 @@
> +    previoustypeloop         => \@previoustypearg,
> +    supportloop              => \@supportarg,
> +    originloop               => \@originarg,
> +    domainloop               => \@domainarg,
> +    DHTMLcalendar_dateformat => C4::Dates->DHTMLcalendar(),

DHTMLcalendar is no longer used.
Comment 8 Katrin Fischer 2013-03-24 21:30:53 UTC
Also I was wondering about the purpose of domain, origin and support. How do they work? :)

Failed QA for the missing unit test for now.
Comment 9 Jonathan Druart 2013-08-21 12:40:53 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2013-08-21 12:41:00 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2013-08-21 12:42:48 UTC
attachment 20537 [details] [review] is a rebased patch.
attachment 20538 [details] [review] is the unit tests for GetPreviousSerialid

Now this patch depends on bug 8435 (in order to avoir conflicts on the unit tests).

Needs QA.
Comment 12 Katrin Fischer 2013-08-21 12:44:21 UTC
Hi Jonathan and Frederic,

could you please add some documentation about the use of the new fields? Also the commit message states that 995... will be automatically filled. And this is indeed hardcoded:

+    // Also prefilling 995$l
+    subfieldid = "subfield" + itemid.substr(4) + "l";
+    $("#" + subfieldid + " input[type='text']").val($("#serialseq" + serialId).val());
+    // And prefilling for supplemental issue
+    $("#subfieldNEWl input[type='text']").val($("#serialseqNEW").val());
+
Comment 13 Katrin Fischer 2013-08-21 12:58:27 UTC
Hm, I see now that I had already commented in comment 7...
Comment 14 Matthias Meusburger 2013-08-21 13:55:33 UTC
Hi ! 

The new field previousitemtype is used to automatically set a given item type to the previously received serial (when a new one is received).

The fields domain origin and ccode are used to provide statistics, like counting the number of items per domain, origin and ccode.

As for the 995l, yes, it should be marc-agnostic.
Comment 15 Katrin Fischer 2013-08-21 13:58:39 UTC
Could you describe domain and origin in a sentence or two? I am really confused what to put in those fields.
Comment 16 Fridolin SOMERS 2013-10-30 14:14:12 UTC
> the commit message states that 995... will be automatically filled. And this is indeed hardcoded
The subfield $h was also hardcoded. It has been corrected by Bug 9947.

This patch should be corrected the same way.
Comment 17 Matthias Meusburger 2014-01-09 16:14:04 UTC Comment hidden (obsolete)
Comment 18 Katrin Fischer 2014-01-19 16:22:34 UTC
Comment on attachment 24099 [details] [review]
proposed patch

Review of attachment 24099 [details] [review]:
-----------------------------------------------------------------

Starting with a code review..., but also ran some tests in Koha.

I really like the idea behind this and have been asked about functionality to automatically set/unset a status for the newest and previous issue before. But I think the current implementation is a bit problematic.

Also there are some changes in this patch, that are not related to the main change and cause a lot of question marks for me. The change for the copynumber is especially problematic, as MARC21 uses a different field for the information already.

I think this needs a bit more work - also see comments in splinter review below.

::: C4/Items.pm
@@ +2782,5 @@
>                              push @authorised_values, $itemtype;
>                              $authorised_lib{$itemtype} = $description;
> +
> +                            # If we have default value named itemtype or itemtypes, we use it
> +                            $defaultvalue = $itemtype if ($defaultvalues and ($defaultvalues->{'itemtypes'} eq $itemtype or $defaultvalues->{'itemtype'} eq $itemtype));

How is this change related to the bug description?

@@ +2814,5 @@
>                              $authorised_lib{$value} = $lib;
> +
> +                            # If we have a default value that has the same name as the authorised value category of the field,
> +                            # we use it
> +                            $defaultvalue = $value if ($defaultvalues and $defaultvalues->{$tagslib->{$tag}->{$subfield}->{authorised_value}} and $defaultvalues->{$tagslib->{$tag}->{$subfield}->{authorised_value}} eq $value);

How is this change related to the bug description?

::: C4/Serials.pm
@@ +926,5 @@
> +
> +    return $return;
> +}
> +
> +

Unit tests included in a separate patch. I had to solve a conflict, but the tests pass. All good.

::: installer/data/mysql/updatedatabase.pl
@@ +7935,5 @@
> +    );
> +
> +    print "Upgrade to $DBversion done (Add makePreviousSerialAvailable syspref)\n";
> +    SetVersion($DBversion);
> +}

Please combine into one single database update and add an AFTER reneweddate to ensure that the sequence of fields is the same on new and updated installations.

Also - 15 :)

::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/serials.pref
@@ +54,5 @@
> +        - pref: makePreviousSerialAvailable
> +          choices: 
> +            yes: Make
> +            no: Do not make
> +        - previous serial automatically available when collecting a new serial. Please note that the item-level_itypes syspref must be set to specific item.

I think 'receiving' would be a bit better than 'collecting' here, but probably better to check wording with a native speaker. It's not clear, which status will be used from the description - What is the first, and what is the second status? Which fields will be used? 

Could it be an option to make this pref a bit more configurable, behaving like AcqItemSetSubfieldsWhenReceived?

::: koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-edit.tt
@@ +62,5 @@
> +    // Also prefilling copynumber
> +    // Getting subfield with copynumber mapping
> +    subfield_div = $(item_div).find("input[name='kohafield'][value='items.copynumber']").parent();
> +    // Setting text field
> +    $(subfield_div).children("input[type='text'][name='field_value']").val($("#serialseq" + serialId).val());

This is a bigger problem: I think copynumber is used in MARC21 to indicate different copies of the same book. The enumchron field is used for the information about issue and year. For MARC21 serialseq is now copied into both fields. I think we can't leave it that way. My suggestion would be to use enumchron for UNIMARC as well or make this marcflavour dependent.

::: koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt
@@ +621,5 @@
> +                                    </select>
> +                                </li>
> +                                [%IF makePreviousSerialAvailable %]
> +                                <li>
> +                                    <label for="previousitemtype">Previous serial Item type:</label>

Tiny capitalization issue - "Item".
As this is on the subscription form, we could maybe leave out "serial" and shorten the description ab it. Also "Serial" is missing from the first field.

::: serials/serials-edit.pl
@@ +243,5 @@
>                  $notes[$i]
>              );
>          }
> +        my $makePreviousSerialAvailable = C4::Context->preference('makePreviousSerialAvailable');
> +        if ($makePreviousSerialAvailable && $serialids[$i] ne "NEW") {

What is 'NEW' referring to? In my tests the change for the itemtype worked, but I couldn't figure out, how the status change is supposed to work.

::: serials/subscription-add.pl
@@ +153,5 @@
>  # prepare template variables common to all $op conditions:
> +my $shelflocations = GetKohaAuthorisedValues( 'items.location', '' );
> +
> +my @locationarg =
> +  map { { code => $_, value => $shelflocations->{$_}, selected => ( ( $subs->{location} and $_ eq $subs->{location} ) ? "selected=\"selected\"" : "" ), } } sort keys %{$shelflocations};

How is this change related to the bug description?
Comment 19 Matthias Meusburger 2014-01-22 16:48:23 UTC
::: C4/Items.pm
@@ +2782,5 @@
>                              push @authorised_values, $itemtype;
>                              $authorised_lib{$itemtype} = $description;
> +
> +                            # If we have default value named itemtype or itemtypes, we use it
> +                            $defaultvalue = $itemtype if ($defaultvalues and ($defaultvalues->{'itemtypes'} eq $itemtype or $defaultvalues->{'itemtype'} eq $itemtype));

and 

@@ +2814,5 @@
>                              $authorised_lib{$value} = $lib;
> +
> +                            # If we have a default value that has the same name as the authorised value category of the field,
> +                            # we use it
> +                            $defaultvalue = $value if ($defaultvalues and $defaultvalues->{$tagslib->{$tag}->{$subfield}->{authorised_value}} and $defaultvalues->{$tagslib->{$tag}->{$subfield}->{authorised_value}} eq $value);

and

::: serials/subscription-add.pl
@@ +153,5 @@
>  # prepare template variables common to all $op conditions:
> +my $shelflocations = GetKohaAuthorisedValues( 'items.location', '' );
> +
> +my @locationarg =
> +  map { { code => $_, value => $shelflocations->{$_}, selected => ( ( $subs->{location} and $_ eq $subs->{location} ) ? "selected=\"selected\"" : "" ), } } sort keys %{$shelflocations};

-> You're right, this is not needed.

::: C4/Serials.pm
@@ +926,5 @@
> +
> +    return $return;
> +}
> +
> +

-> Why do you quote this part specifically? The patch adds the whole GetPreviousSerialid, which return is needed. Am I missing something here?


Please combine into one single database update and add an AFTER reneweddate to ensure that the sequence of fields is the same on new and updated installations.

-> done

It's not clear, which status will be used from the description - What is the first, and what is the second status? Which fields will be used? 

-> when makePreviousSerialAvailable is enabled, when receiving a new serial, notforloan will be set to 0 and the itype will be set to previousitemtype for previously received serial. Are these the points I should make more explicit?

Could it be an option to make this pref a bit more configurable, behaving like AcqItemSetSubfieldsWhenReceived?

-> If I'm not mistaken, AcqItemSetSubfieldsWhenReceived applies to the serial we just received, as makePreviousSerialAvailable applies to the one that was received before (just to make things clear). 

But besides that, if there are use-cases where other fields than notforloan and itype have to be used, it might be useful to have something more configurable. Is it so?


This is a bigger problem: I think copynumber is used in MARC21 to indicate different copies of the same book. The enumchron field is used for the information about issue and year. For MARC21 serialseq is now copied into both fields. I think we can't leave it that way. My suggestion would be to use enumchron for UNIMARC as well or make this marcflavour dependent.

-> using enumchron for UNIMARC as well makes sense. Done.

Tiny capitalization issue - "Item".
As this is on the subscription form, we could maybe leave out "serial" and shorten the description ab it. Also "Serial" is missing from the first field.

-> done

What is 'NEW' referring to? In my tests the change for the itemtype worked, but I couldn't figure out, how the status change is supposed to work.

-> 'NEW' is previously set by koha : 
$cell->{'serialid'}       = 'NEW';
(serials/serials-edit.pl:185)
Comment 20 Matthias Meusburger 2014-01-23 08:51:49 UTC Comment hidden (obsolete)
Comment 21 Katrin Fischer 2014-03-10 10:44:30 UTC
Jonathan, I have a problem in the QA script I am not sure how to fix - could you take a look please?

 FAIL	serials/serials-edit.pl
   OK	  pod
   OK	  forbidden patterns
   FAIL	  valid
		"my" variable $dbh masks earlier declaration in same scope 
   OK	  critic
Comment 22 Jonathan Druart 2014-03-10 11:00:08 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2014-03-10 11:00:17 UTC Comment hidden (obsolete)
Comment 24 Jonathan Druart 2014-03-10 11:00:23 UTC Comment hidden (obsolete)
Comment 25 Koha Team AMU 2014-03-10 15:38:39 UTC
If we look at the Description http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7677#c0
Points 0, 1, 3 and 4 are OK.

But Point 2 KO.

Nota : There's a link with BZ10855
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10855
because some new areas of the subscription must appear in the item record fields.
Comment 26 Jonathan Druart 2014-04-16 09:40:40 UTC
(In reply to Koha Team AMU from comment #25)
> If we look at the Description
> http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7677#c0
> Points 0, 1, 3 and 4 are OK.
> 
> But Point 2 KO.

Matthias will try to have a look asap.

> Nota : There's a link with BZ10855
> http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10855
> because some new areas of the subscription must appear in the item record
> fields.

I would prefer to see bug 10855 pushed before to add new behaviors.
Comment 27 Matthias Meusburger 2014-04-29 13:16:50 UTC
The sentence describing the point 2 is not very clear. 

It means that the newly received serial automatically gets its itemtype from what is defined in the subscription in the "itemtype" dropdown list.
Comment 28 Matthias Meusburger 2014-04-29 13:19:27 UTC Comment hidden (obsolete)
Comment 29 Koha Team Lyon 3 2014-06-18 15:11:27 UTC
h1>Something went wrong !</h1>Applying: Bug 7677: UT: add unit tests for GetPreviousSerialid Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Serials_2.t
Applying: BZ7677: New areas in subscriptions and new functions when receiving.
Using index info to reconstruct a base tree...
<stdin>:49: trailing whitespace.
                        ORDER BY serialid DESC LIMIT ,1
<stdin>:97: trailing whitespace.
    , , , , ,
<stdin>:198: trailing whitespace.
          choices: 
warning: 3 lines add whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging C4/Serials.pm
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging installer/data/mysql/sysprefs.sql Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-edit.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt
Auto-merging serials/subscription-add.pl Failed to merge in the changes.
Patch failed at 0001 BZ7677: New areas in subscriptions and new functions when receiving.
Comment 30 Jonathan Druart 2014-06-18 15:37:17 UTC Comment hidden (obsolete)
Comment 31 Jonathan Druart 2014-06-18 15:37:23 UTC Comment hidden (obsolete)
Comment 32 Jonathan Druart 2014-06-18 15:37:29 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2014-06-18 15:37:34 UTC Comment hidden (obsolete)
Comment 34 Jonathan Druart 2014-07-08 10:37:28 UTC Comment hidden (obsolete)
Comment 35 Jonathan Druart 2014-07-08 10:37:35 UTC Comment hidden (obsolete)
Comment 36 Jonathan Druart 2014-07-08 10:37:41 UTC Comment hidden (obsolete)
Comment 37 Jonathan Druart 2014-07-08 10:38:21 UTC
rebased patches (updatedatabase conflict).
Comment 38 Katrin Fischer 2014-12-08 20:22:02 UTC
I saw this is on the list for the testing day :) It would be good to add a test plan and description. I think the feature evolved a bit over time, as this is one of the older bug reports.
Comment 39 Paola Rossi 2014-12-09 13:44:50 UTC
I've tried to apply the patches against master 3.19.00.000

The second patch needed some [almost trivial] rebasing:

Applying: BZ7677: New areas in subscriptions and new functions when receiving.
Using index info to reconstruct a base tree...
<stdin>:49: trailing whitespace.
                        ORDER BY serialid DESC LIMIT $nth,1 
<stdin>:97: trailing whitespace.
    $staffdisplaycount, $opacdisplaycount, $graceperiod, $location, $enddate, 
<stdin>:197: trailing whitespace.
          choices: 
warning: 3 lines add whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging serials/serials-edit.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-edit.tt
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/sysprefs.sql
CONFLICT (content): Merge conflict in installer/data/mysql/sysprefs.sql
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging C4/Serials.pm
Failed to merge in the changes.
Patch failed at 0001 BZ7677: New areas in subscriptions and new functions when receiving.

I pass the patch to "Patch doesn't apply" status.
Comment 40 Jonathan Druart 2014-12-09 13:57:50 UTC Comment hidden (obsolete)
Comment 41 Jonathan Druart 2014-12-09 13:57:57 UTC Comment hidden (obsolete)
Comment 42 Jonathan Druart 2014-12-09 13:58:01 UTC Comment hidden (obsolete)
Comment 43 Jonathan Druart 2014-12-09 13:58:06 UTC Comment hidden (obsolete)
Comment 44 Paola Rossi 2014-12-23 15:31:59 UTC
I've applied the patches against master 3.19.00.002

The first patch requires:

prove t/db_dependent/Serials_2.t 
t/db_dependent/Serials_2.t .. Global symbol "$subscriptionid" requires explicit package name at t/db_dependent/Serials_2.t line 208.
Global symbol "$subscriptionid" requires explicit package name at t/db_dependent/Serials_2.t line 209.
Global symbol "$subscriptionid" requires explicit package name at t/db_dependent/Serials_2.t line 210.
Global symbol "$subscriptionid" requires explicit package name at t/db_dependent/Serials_2.t line 212.
Global symbol "$subscriptionid" requires explicit package name at t/db_dependent/Serials_2.t line 213.
Global symbol "$subscriptionid" requires explicit package name at t/db_dependent/Serials_2.t line 214.
Global symbol "$subscriptionid" requires explicit package name at t/db_dependent/Serials_2.t line 215.
Execution of t/db_dependent/Serials_2.t aborted due to compilation errors.
# Looks like your test exited with 255 before it could output anything.
t/db_dependent/Serials_2.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 36/36 subtests 
 
Test Summary Report
-------------------
t/db_dependent/Serials_2.t (Wstat: 65280 Tests: 0 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 36 tests but ran 0.
Files=1, Tests=0,  1 wallclock secs ( 0.00 usr  0.01 sys +  0.72 cusr  0.05 csys =  0.78 CPU)
Result: FAIL

I pass the patch to "Failed QA" status.
Comment 45 Jonathan Druart 2014-12-23 15:52:34 UTC Comment hidden (obsolete)
Comment 46 Paola Rossi 2014-12-29 16:05:55 UTC
I've applied the patches against master 3.19.00.003
[it needed a trivial rebase]

I set "makePreviousSerialAvailable" syspref to "Make" as required.
I kept the "item-level_itypes" syspref to "specific item".

I created a subscription, which didn't create an item record when it received the serial. When I received ("Arrived") issues, no item was created: so this bug is not about this case.

So I created another subscription, which created an item record when receiving the serial, and I set its 2 new fields ["Item type" and "Previous item type"] to "Continuing Resources".

Then I "Edited" the subscription:
the "Item type" and "Previous item type" fields were not set to "Continuing Resources" anymore! (ERROR)

Anyway I went on:
Was the previously received serial's itemtype set as defined in the subscription?

When I received ("Arrived") an issue [in DB: table serial] of the subscription (the first issue), I saw that the default value of the:
"y - Koha item type" field
of the new item was set to nothing (why?).

I didn't change it to "Continuing Resources" [because koha should
do it for me, shouldn't it?]; then I filled the other fields (barcode, aso), and I saved the new item.

Then I looked at the biblio:
In the biblio's Holdings tag, the item was "Available" and its item type is CR as required (RIGHT).

I received another issue [the 2-nd] for the same subscription.
Now, in the biblio's Holdings, the new (2-nd) item was "Available" and CR, as required.
But the previous (1-st) item [still "Available"] lost its type: it had no type anymore! (ERROR)

But I could have been wrong: as Katrin wrote in comment 38:
>It would be good to add a test plan and description. I think
>the feature evolved a bit over time, as this is one of the older bug reports.

Anyway:
prove t/db_dependent/Serials_2.t
t/db_dependent/Serials_2.t .. 3/40
#   Failed test 'With IndependentBranches, superserials can edit a subscription from his branch'
#   at t/db_dependent/Serials_2.t line 106.
#          got: '0'
#     expected: '1'
 
#   Failed test 'With IndependentBranches, superserials can edit a subscription from another branch'
#   at t/db_dependent/Serials_2.t line 109.
#          got: '0'
#     expected: '1'
 
#   Failed test 'With IndependentBranches, superserials can show a subscription from his branch'
#   at t/db_dependent/Serials_2.t line 112.
#          got: '0'
#     expected: '1'
 
#   Failed test 'With IndependentBranches, superserials can show a subscription from another branch'
#   at t/db_dependent/Serials_2.t line 115.
#          got: '0'
#     expected: '1'
 
#   Failed test 'With IndependentBranches, edit_subscription can edit a subscription from his branch'
#   at t/db_dependent/Serials_2.t line 121.
#          got: '0'
#     expected: '1'
 
#   Failed test 'With IndependentBranches, show_subscription can show a subscription from his branch'
#   at t/db_dependent/Serials_2.t line 127.
#          got: '0'
#     expected: '1'
 
#   Failed test 'With IndependentBranches, renew_subscription can show a subscription from his branch'
#   at t/db_dependent/Serials_2.t line 141.
#          got: '0'
#     expected: '1'
 
#   Failed test 'Without IndependentBranches, superserials can edit a subscription from his branch'
#   at t/db_dependent/Serials_2.t line 166.
#          got: '0'
#     expected: '1'
 
#   Failed test 'Without IndependentBranches, superserials can edit a subscription from another branch'
#   at t/db_dependent/Serials_2.t line 169.
#          got: '0'
#     expected: '1'
 
#   Failed test 'Without IndependentBranches, superserials can show a subscription from his branch'
#   at t/db_dependent/Serials_2.t line 172.
#          got: '0'
#     expected: '1'
 
#   Failed test 'Without IndependentBranches, superserials can show a subscription from another branch'
#   at t/db_dependent/Serials_2.t line 175.
#          got: '0'
#     expected: '1'
 
#   Failed test 'Without IndependentBranches, edit_subscription can edit a subscription from his branch'
#   at t/db_dependent/Serials_2.t line 180.
#          got: '0'
#     expected: '1'
 
#   Failed test 'Without IndependentBranches, edit_subscription can edit a subscription from another branch'
#   at t/db_dependent/Serials_2.t line 183.
#          got: '0'
#     expected: '1'
 
#   Failed test 'Without IndependentBranches, show_subscription can show a subscription from his branch'
#   at t/db_dependent/Serials_2.t line 186.
#          got: '0'
#     expected: '1'
 
#   Failed test 'Without IndependentBranches, show_subscription can show a subscription from another branch'
#   at t/db_dependent/Serials_2.t line 189.
#          got: '0'
#     expected: '1'
 
#   Failed test 'Without IndependentBranches, renew_subscription cannot show a subscription from his branch'
#   at t/db_dependent/Serials_2.t line 200.
#          got: '0'
#     expected: '1'
 
#   Failed test 'Without IndependentBranches, renew_subscription cannot show a subscription from another branch'
#   at t/db_dependent/Serials_2.t line 203.
#          got: '0'
#     expected: '1'
# Looks like you failed 17 tests of 40.
t/db_dependent/Serials_2.t .. Dubious, test returned 17 (wstat 4352, 0x1100)
Failed 17/40 subtests
 
Test Summary Report
-------------------
t/db_dependent/Serials_2.t (Wstat: 4352 Tests: 40 Failed: 17)
  Failed tests:  9-13, 15, 19, 25-32, 35-36
  Non-zero exit status: 17
Files=1, Tests=40,  8 wallclock secs ( 0.01 usr  0.04 sys +  0.29 cusr  2.21 csys =  2.55 CPU)
Result: FAIL
------------------------------------------------------------------------------
So I pass the patch to "Failed QA" status.
Comment 47 Matthias Meusburger 2015-03-02 11:32:43 UTC
Updated test plan:

Enable the makePreviousSerialAvailable syspref. Set the item-level_itypes syspref to 'specific item'.

Create a subscription, with the 'create an item record when receiving this serial' option enabled.

Select an item-type (let's say ItemType) and a previous item-type (let's say OldItemType).

Receive a serial, and then another, then check that:

 - the first received serial's itemtype has been set to the "previous item type" value which was set in the subscription (OldItemType in our example).

 - the first received has been made available.

 - the last received serial itemtype has been set to the "item type" value, which was set in the subscription (ItemType in our example).

 - enumchron field of the item is automatically prefilled.
Comment 48 Matthias Meusburger 2015-03-03 10:47:44 UTC
Paola, about your remarks:

the "Item type" and "Previous item type" fields were not set to "Continuing Resources" anymore! (ERROR)

=> fixed in a little patch I will submit.


When I received ("Arrived") an issue [in DB: table serial] of the suscription (the first issue), I saw that the default value of the:
"y - Koha item type" field
of the new item was set to nothing (why?).

=> Is your itemtype field (995$y in your case) using itemtypes authorized values in the bibliographic framework ? It does not work if authorized values are not linked.


prove t/db_dependent/Serials_2.t

=> The tests also fail from test #3 on master, so my patch may have not broken the tests (must be fixed on master to be sure, though).
Comment 49 Matthias Meusburger 2015-03-03 11:05:57 UTC Comment hidden (obsolete)
Comment 50 Matthias Meusburger 2015-03-03 14:25:23 UTC
About the tests, my bad: they are not broken in master, BUT these are db_dependent tests, and as such, they need to be run with a proper database.

This patch allows to create such a database in your environment : 

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10337

With this database on master, the tests run fine.

With this database and my patches, the tests also run fine:

> prove t/db_dependent/Serials_2.t
[...]
All tests successful.
Files=1, Tests=40,  2 wallclock secs ( 0.04 usr  0.00 sys +  1.14 cusr  0.08 csys =  1.26 CPU)
Result: PASS
Comment 51 Matthias Meusburger 2015-03-04 10:49:06 UTC Comment hidden (obsolete)
Comment 52 Matthias Meusburger 2015-03-04 10:49:27 UTC
patch rebased for updatedatabase.
Comment 53 Matthias Meusburger 2015-03-04 10:58:27 UTC Comment hidden (obsolete)
Comment 54 Matthias Meusburger 2015-03-04 10:59:36 UTC Comment hidden (obsolete)
Comment 55 Matthias Meusburger 2015-03-04 10:59:45 UTC Comment hidden (obsolete)
Comment 56 Matthias Meusburger 2015-03-04 10:59:51 UTC Comment hidden (obsolete)
Comment 57 Matthias Meusburger 2015-03-04 11:00:05 UTC Comment hidden (obsolete)
Comment 58 Matthias Meusburger 2015-03-04 11:00:12 UTC Comment hidden (obsolete)
Comment 59 Brendan Gallagher 2015-03-06 11:13:11 UTC Comment hidden (obsolete)
Comment 60 Brendan Gallagher 2015-03-06 11:13:22 UTC Comment hidden (obsolete)
Comment 61 Brendan Gallagher 2015-03-06 11:13:28 UTC Comment hidden (obsolete)
Comment 62 Brendan Gallagher 2015-03-06 11:13:35 UTC Comment hidden (obsolete)
Comment 63 Brendan Gallagher 2015-03-06 11:13:42 UTC Comment hidden (obsolete)
Comment 64 Katrin Fischer 2015-04-06 20:18:52 UTC
Wow, first comment here from me was over a year ago :(

As this is a bigger feature with a longer history, I will leave some longer testing notes for reference:

Automated Testing:

- OK - QA script passes on current 5 patches
- OK - Unit tests, especially all in t/db_dependent/Serial* pass

Code Review:

- OK - New sub GetPreviousSerialid has unit tests and is used to get the 
  last arrived serial item.
- OK - Database changes in updatedatabase and kohastructure are the same.
- (1) serials-edit.pl: Patch adds SQL to the .pl script - please move this 
  out of the pl script.
- (2) System preference description currently reads:
  previous serial automatically available when collecting a new serial. 
  Please note that the item-level_itypes syspref must be set to specific item.
-- s/collecting/receiving
-- s/serial/serial issue (?)
-- It would be nice to add a note that this will also enable you to switch
   the itemtype of the last issue to another item type on receiving a new issue.

Testing:
- OK Adding subscription (syspref is OFF)
-- System preference not active, only item type shows on form
-- Item type is saved
- Receiving issue and creating an item
-- (3) In the item form, the selected itemtype doesn't preselect

- OK Editing subscription
-- Changes save correctly

- Editing subscription (syspref is now ON)
-- "Previous item type:" shows up now
-- (4) I think the descrption might be misleading - as it's the final and not
   really the previous item type of the issue. Not sure I can explain that
   well. Maybe we could use a longer description?
--- Item type for newest issue: / Item type for older issues:

- OK receiving issues
-- This is ok, as the last issue is set to my defined item type, 
   but (3) is still showing as a problem here

I have used numbers to mark some problematic bits. Please take a look!
Comment 65 Katrin Fischer 2015-07-09 02:44:49 UTC
I think we are not far - please fix so this long story can have a happy end :)
Comment 66 Matthias Meusburger 2015-09-28 14:30:52 UTC Comment hidden (obsolete)
Comment 67 Matthias Meusburger 2015-09-28 14:32:16 UTC
This last patch fixes the issues you've raised when testing, Katrin.
Comment 68 Matthias Meusburger 2015-09-28 14:39:27 UTC Comment hidden (obsolete)
Comment 69 Koha Team Lyon 3 2015-12-15 16:42:42 UTC
tried on a sandbox :
<h1>Something went wrong !</h1>Applying: BZ7677: New areas in subscriptions and new functions when receiving.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging serials/subscription-add.pl CONFLICT (content): Merge conflict in serials/subscription-add.pl Auto-merging serials/serials-edit.pl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-edit.tt
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/sysprefs.sql Auto-merging installer/data/mysql/kohastructure.sql
CONFLICT (content): Merge conflict in installer/data/mysql/kohastructure.sql
Auto-merging C4/Serials.pm
Failed to merge in the changes.
Patch failed at 0001 BZ7677: New areas in subscriptions and new functions when receiving.
Comment 70 Matthias Meusburger 2015-12-23 10:50:22 UTC Comment hidden (obsolete)
Comment 71 Matthias Meusburger 2015-12-23 10:50:51 UTC Comment hidden (obsolete)
Comment 72 Matthias Meusburger 2015-12-23 10:51:16 UTC Comment hidden (obsolete)
Comment 73 Matthias Meusburger 2015-12-23 10:51:32 UTC Comment hidden (obsolete)
Comment 74 Matthias Meusburger 2015-12-23 10:51:50 UTC Comment hidden (obsolete)
Comment 75 Matthias Meusburger 2015-12-23 10:52:07 UTC Comment hidden (obsolete)
Comment 76 Matthias Meusburger 2015-12-23 10:54:45 UTC
Rebased. And moved SQL update from updatedatabase to atomic update.
Comment 77 Chris Cormack 2016-01-04 07:52:08 UTC Comment hidden (obsolete)
Comment 78 Kyle M Hall 2016-01-29 18:56:41 UTC Comment hidden (obsolete)
Comment 79 Katrin Fischer 2016-03-21 07:19:19 UTC
Just starting with this - the QA tools have some complaints. Could you please take a look?

 FAIL	Koha/Serial/Item.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  pod
   OK	  spelling
		0
   FAIL	  valid
		Name "Tie::Hash::FIELDS" used only once: possible typo 
		Name "Cache::RemovalStrategy::LRU::FIELDS" used only once: possible typo 
		Name "Cache::RemovalStrategy::FIELDS" used only once: possible typo 

 FAIL	Koha/Serial/Items.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  pod
   OK	  spelling
		0
   FAIL	  valid
		Name "Cache::RemovalStrategy::FIELDS" used only once: possible typo 
		Name "Cache::RemovalStrategy::LRU::FIELDS" used only once: possible typo 
		Name "Tie::Hash::FIELDS" used only once: possible typo
Comment 80 Jonathan Druart 2016-03-21 14:04:42 UTC
(In reply to Katrin Fischer from comment #79)
>    FAIL	  valid
> 		Name "Cache::RemovalStrategy::FIELDS" used only once: possible typo 
> 		Name "Cache::RemovalStrategy::LRU::FIELDS" used only once: possible typo 
> 		Name "Tie::Hash::FIELDS" used only once: possible typo

See bug 16104.
Comment 81 Katrin Fischer 2016-03-21 22:35:40 UTC
The subscription form behaviour looks good, although it would be nice if the new settings appeared on the subscription detail page. Location and Call number are also missing, so this could be handled separately.

But I have a problem on receiving serial items with and without this patch that stops me from testing this further - see bug 16118.
Comment 82 Katrin Fischer 2016-03-21 22:37:32 UTC
Ah some note: I wonder about the change to the not for loan status that is built in here. Wouldn't a 'not for loan' itemtype have the same effect? The only difference I can think of is that the nfl in the item gives you the option of having a custom nfl status text.
Comment 83 Katrin Fischer 2016-03-23 06:57:46 UTC
Working on this!
Comment 84 Katrin Fischer 2016-03-24 07:23:11 UTC
I am experiencing bug 16118 again - so can't continue testing until I have found the issue for this. Any help appreciated.

Meanwhile, can you check my written up test plan for this is correct?

Bug 7677 - New area in subscriptions and new function when receiving

This patch:
- It's now possible to select a default item type in the subscription,
  similar to the existing default location and call number.
- A new system preference makePreviousSerialAvailable is introcuded.
  This has 2 effects: 
  - It will automatically switch the not for loan status of the last
    issue to 0 (available) when the next one is received.
  - It will allow you to pick an item type in the subscription, 
    that the older issue is set to, when the next one is received.

To test:
- Create a subscription, where you create items on receive.
- Leave preference off and test the new default item type field.
- Verify new items are generated with the configured item type on
  receive.
- Activate preference.
- Receive issue, set not for loan != available/empty
- Receive another issue.
- Verify that the item is now available (not for loan = 0).
- Add another an item type for older issues.
- Receive another 2 issues, verify that the itemtype switch happens 
  correctly.
Comment 85 Katrin Fischer 2016-03-24 22:45:09 UTC
I am sorry, this fails when I try to receive a serial issue:

Can't use an undefined value as an ARRAY reference at /home/katrin/kohaclone/serials/serials-collection.pl line 131.

And after running the Schema update script:
DBIx::Class::Schema::resultset(): resultset() expects a source name at /home/katrin/kohaclone/Koha/Objects.pm line 235

Please take a look!
Comment 86 Katrin Fischer 2016-03-28 17:34:46 UTC
I tried running misc/devel/update_dbix_class_files.pl, but I still get the error message on trying to save the received issue.
Comment 87 Matthias Meusburger 2016-06-03 13:59:13 UTC Comment hidden (obsolete)
Comment 88 Katrin Fischer 2016-07-14 22:40:53 UTC
I've run the database update and created the schema files, but sadly, I am still seeing an error:

DBIx::Class::ResultSource::unique_constraint_columns(): Unknown unique constraint primary on 'serialitems' at /home/katrin/kohaclone/Koha/Objects.pm line 83

It happens when I try to receive 2 serial issues at once. To reproduce this:
- Search for a subscription
- Go to it's serial collection page
- Create a next issue
- Try to receive both the now late issue and the expected one at once
Comment 89 Katrin Fischer 2016-08-14 14:57:27 UTC
We are so close on this... please don't give up!
Comment 90 Julian Maurice 2016-08-25 14:09:50 UTC Comment hidden (obsolete)
Comment 91 Katrin Fischer 2016-08-28 21:30:59 UTC
Hi Matthias,
in testing things looked great, but now the tests are failing for me, hoping it's easy to fix:
perl t/db_dependent/Serials_2.t:
Can't call method "lastvalue1" on an undefined value at /home/katrin/kohaclone/C4/Serials.pm line 1575.
# Looks like your test exited with 2 just after 46.
Comment 92 Matthias Meusburger 2016-08-30 08:42:54 UTC Comment hidden (obsolete)
Comment 93 Katrin Fischer 2016-09-05 19:44:50 UTC
Matthias, I am sorry, but there is still something odd about the test:
rollback ineffective with AutoCommit enabled at t/db_dependent/Serials_2.t line 257.
Comment 94 Jonathan Druart 2016-09-06 07:08:31 UTC
(In reply to Katrin Fischer from comment #93)
> Matthias, I am sorry, but there is still something odd about the test:
> rollback ineffective with AutoCommit enabled at t/db_dependent/Serials_2.t
> line 257.

Caused by a wrong fix conflict on "Bug 7677: UT: add unit tests for GetPreviousSerialid", the rollback should not be added by this patch (was not when I submitted it).
Comment 95 Katrin Fischer 2016-09-06 11:03:24 UTC
I am not sure I understand how to fix - can you please provide a new patch for the last patch? I think that's the one adding it.
Comment 96 Jonathan Druart 2016-09-06 11:48:11 UTC
Comment on attachment 45940 [details] [review]
Bug 7677: UT: add unit tests for GetPreviousSerialid

Review of attachment 45940 [details] [review]:
-----------------------------------------------------------------

::: t/db_dependent/Serials_2.t
@@ +255,5 @@
> +is( GetPreviousSerialid( $subscriptionid, 1 ), $serialid2, "get previous serialid with 1" );
> +is( GetPreviousSerialid( $subscriptionid, 2 ), $serialid1, "get previous serialid with 2" );
> +is( GetPreviousSerialid( $subscriptionid, 3 ), undef, "get previous serialid with 3, does not exist" );
> +
> +$dbh->rollback;

This rollback should not have been added by this patch (added by a wrong merge conflict).
Comment 97 Julian Maurice 2016-09-08 07:06:34 UTC
Comment on attachment 54876 [details] [review]
Bug 7677: QA followup

This patch shouldn't have been obsoleted
Comment 98 Julian Maurice 2016-09-08 07:16:17 UTC Comment hidden (obsolete)
Comment 99 Katrin Fischer 2016-09-10 11:20:22 UTC
Created attachment 55466 [details] [review]
[PASSED QA] BZ7677: New areas in subscriptions and new functions when receiving.

When enabling the makePreviousSerialAvailable syspref, the previously
 received serial's itemtype is set as defined in the subscription.
 (Please note that the item-level_itypes syspref must be set to specific item.)
 It is also made available.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

http://bugs.koha-community.org/show_bug.cgi?id=7767

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 100 Katrin Fischer 2016-09-10 11:20:27 UTC
Created attachment 55467 [details] [review]
[PASSED QA] Bug 7677: UT: add unit tests for GetPreviousSerialid

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

http://bugs.koha-community.org/show_bug.cgi?id=7767

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 101 Katrin Fischer 2016-09-10 11:20:32 UTC
Created attachment 55468 [details] [review]
[PASSED QA] Bug 7677: FIX QA issues

 FAIL    C4/Serials.pm
   FAIL      forbidden patterns
    forbidden pattern: trailing space char (line 1501)
    forbidden pattern: trailing space char (line 921)

 FAIL    serials/serials-edit.pl
   FAIL      valid
        "my" variable $dbh masks earlier declaration in same scope

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

http://bugs.koha-community.org/show_bug.cgi?id=7767

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 102 Katrin Fischer 2016-09-10 11:20:37 UTC
Created attachment 55469 [details] [review]
[PASSED QA] Bug 7677: Fix tests

They don't pass since bug 12098 has been pushed.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

http://bugs.koha-community.org/show_bug.cgi?id=7767

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 103 Katrin Fischer 2016-09-10 11:20:42 UTC
Created attachment 55470 [details] [review]
[PASSED QA] Bug 7677 [Follow-up]: New areas in subscriptions and new functions when receiving.

 - Various fixes.

Test plan:

Once the makePreviousSerialAvailable syspref is enabled, receive a serial, and then another, then check that:

 - the first received itemtype has been set to the "previous item type" value (set in the subscription).

 - the first received has been made available.

 - the last received serial itemtype has been set to the "item type" value (set in the subscription).

 - 995$l is automatically prefilled.

Configure the serialsFieldsToAV syspref. When creating or editing a subscription, check that:

 - the domain and/or origin and/or support fields are correctly displaying the authorized values configured in the syspref.

Signed-off-by: Chris <chris@bigballofwax.co.nz>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 104 Katrin Fischer 2016-09-10 11:20:46 UTC
Created attachment 55471 [details] [review]
[PASSED QA] Bug 7677 [QA Followup] - Replace GetItemnumberFromSerialId with use of Koha::Serial::Item(s)

GetItemnumberFromSerialId has no unit tests. It would be better to start
using our object system here.

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 105 Katrin Fischer 2016-09-10 11:20:51 UTC
Created attachment 55472 [details] [review]
[PASSED QA] Bug 7677 [QA Followup] - Replace GetItemnumberFromSerialId with use of Koha::Serial::Item(s)

 - use _type instead of type (see https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15446 )

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 106 Katrin Fischer 2016-09-10 11:20:55 UTC
Created attachment 55473 [details] [review]
[PASSED QA] Bug 7677: QA followup

Fix 'unique contraint primary' error when receiving serials

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 107 Katrin Fischer 2016-09-10 11:21:00 UTC
Created attachment 55474 [details] [review]
[PASSED QA] Bug 7677: QA followup

 - Fix tests

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 108 Katrin Fischer 2016-09-10 11:21:05 UTC
Created attachment 55475 [details] [review]
[PASSED QA] Bug 7677: UT: Remove $dbh->rollback

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 109 Katrin Fischer 2016-09-10 11:23:16 UTC
Long story... happy ending? :)
Comment 110 Kyle M Hall 2016-09-12 10:46:38 UTC
Pushed to master for 16.11, thanks Matthius, Jonathan, Julian!
Comment 111 Jonathan Druart 2016-09-14 13:14:55 UTC
   locations_loop=>$locations_loop,
is passed twice in serials/subscription-add.pl