Bug 27837 - Permanent location is reverted to location when location updated and permanent_location mapped to MARC field
Summary: Permanent location is reverted to location when location updated and permanen...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 23463
Blocks: 28832 30909
  Show dependency treegraph
 
Reported: 2021-03-02 15:54 UTC by Nick Clemens
Modified: 2023-06-08 22:26 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.06,20.05.12


Attachments
Bug 27837: Update permanent location if passed into ModItemFromMarc (1.33 KB, patch)
2021-03-03 13:34 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27837: Add unit tests (2.84 KB, patch)
2021-03-03 13:55 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27837: Update permanent location if passed into ModItemFromMarc (1.38 KB, patch)
2021-03-03 21:01 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 27837: Add unit tests (2.89 KB, patch)
2021-03-03 21:01 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 27837: Update permanent location if passed into ModItemFromMarc (1.46 KB, patch)
2021-03-03 21:12 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 27837: Add unit tests (2.97 KB, patch)
2021-03-03 21:12 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 27837: Prevent the tests to fail is the field is mapped (1.30 KB, patch)
2021-03-04 08:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27837: Update permanent location if passed into ModItemFromMarc (1.49 KB, patch)
2021-03-04 14:26 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27837: Add unit tests (3.00 KB, patch)
2021-03-04 14:26 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27837: Prevent the tests to fail is the field is mapped (1.35 KB, patch)
2021-03-04 14:26 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27837: Permanent location to be empty string, check if defined (891 bytes, patch)
2021-03-04 15:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27837: Permanent location to be empty string, check if key exists (893 bytes, patch)
2021-03-04 15:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27837: Update permanent location if passed into ModItemFromMarc (1.55 KB, patch)
2021-05-02 16:16 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27837: Add unit tests (3.05 KB, patch)
2021-05-02 16:16 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27837: Prevent the tests to fail is the field is mapped (1.41 KB, patch)
2021-05-02 16:16 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27837: Permanent location to be empty string, check if key exists (934 bytes, patch)
2021-05-02 16:16 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27837: Update permanent location if passed into ModItemFromMarc (1.55 KB, patch)
2021-05-05 18:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27837: Add unit tests (3.05 KB, patch)
2021-05-05 18:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27837: Prevent the tests to fail is the field is mapped (1.41 KB, patch)
2021-05-05 18:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27837: Unit tests for blank permanent location (3.41 KB, patch)
2021-05-05 18:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27837: Permanent location to be empty string, check if key exists (934 bytes, patch)
2021-05-05 18:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27837: Check the MARC directly, not the transformed item (2.26 KB, patch)
2021-05-06 12:12 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27837: Always pass permanent location from items editor if it was defined (3.16 KB, patch)
2021-05-06 13:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27837: Always pass permanent location from items editor if it was defined (3.16 KB, patch)
2021-05-06 13:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27837: (follow-up) Do not check subfield unless tag defined (1.23 KB, patch)
2021-05-11 11:09 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27837: Update permanent location if passed into ModItemFromMarc (1.61 KB, patch)
2021-05-17 13:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27837: Add unit tests (3.11 KB, patch)
2021-05-17 13:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27837: Prevent the tests to fail is the field is mapped (1.47 KB, patch)
2021-05-17 13:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27837: Unit tests for blank permanent location (3.41 KB, patch)
2021-05-17 13:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27837: Permanent location to be empty string, check if key exists (999 bytes, patch)
2021-05-17 13:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27837: Check the MARC directly, not the transformed item (2.31 KB, patch)
2021-05-17 13:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27837: Always pass permanent location from items editor if it was defined (3.21 KB, patch)
2021-05-17 13:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27837: (follow-up) Do not check subfield unless tag defined (1.29 KB, patch)
2021-05-17 13:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27837: (QA follow-up) Minor Spelling/Typo correction (1011 bytes, patch)
2021-05-17 13:27 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2021-03-02 15:54:47 UTC
Generally the permanent_location and location should match, however, prior to bug 23463 one could:
1 - Map permanent_location to a marc field
2 - Expose that field in the item editor
3 - Change the location of the item and retain the permanent location on saving
4 - This created a 'temporary' location that was cleared upon next checkin

Many libraries used this for adding items to displays.

Since bug 23463, however, the item updates only fields that have changed - if one changes the location but not the permanent location then koha sees that no permanent location was set and reverts it to the location

We need to force passing through the permanent_location if it is visible+mapped in the item edit form, regardless of whether it changes or not
Comment 1 Jonathan Druart 2021-03-03 07:33:34 UTC
Can you have a look at bug 27545? I feel like the second patch will fix this.
Comment 2 Kyle M Hall 2021-03-03 13:34:22 UTC
Created attachment 117570 [details] [review]
Bug 27837: Update permanent location if passed into ModItemFromMarc
Comment 3 Kyle M Hall 2021-03-03 13:55:23 UTC
Created attachment 117572 [details] [review]
Bug 27837: Add unit tests
Comment 4 Kyle M Hall 2021-03-03 17:32:45 UTC
(In reply to Jonathan Druart from comment #1)
> Can you have a look at bug 27545? I feel like the second patch will fix this.

It does not. I ran some manual tests that failed, and also ran the unit tests from this patch set, which also failed.
Comment 5 Andrew Fuerste-Henry 2021-03-03 19:50:08 UTC
Test plan: 
1 - Map permanent_location to a marc field
2 - Expose that field in the item editor
3 - Edit an item, changing location to X and permanent location to Y, save
4 - confirm record shows location=x, permanent_location=y
5 - Edit item again, change location to Z, do not change permanent location, save
6 - confirm location and permanent_location both equal Z
7 - apply patch, restart
8 - repeat steps 3-5
9 - confirm location=Z, permanent location=Y
10 - prove t/db_dependent/Items.t
Comment 6 Andrew Fuerste-Henry 2021-03-03 20:25:59 UTC
Tests fail if permanent_location is mapped to something in Koha to MARC Mapping. 

root@kohadevbox:koha(master)$ prove t/db_dependent/Items.t
t/db_dependent/Items.t .. 11/14 
        #   Failed test 'new permanent location set as expected'
        #   at t/db_dependent/Items.t line 1057.
        #          got: 'A'
        #     expected: 'B'

        #   Failed test 'permanent location remains unchanged as expected'
        #   at t/db_dependent/Items.t line 1078.
        #          got: 'A'
        #     expected: 'B'
        # Looks like you failed 2 tests of 6.

    #   Failed test 'permanent_location'
    #   at t/db_dependent/Items.t line 1079.
    # Looks like you failed 1 test of 6.

#   Failed test 'ModItemFromMarc'
#   at t/db_dependent/Items.t line 1082.
# Looks like you failed 1 test of 14.
t/db_dependent/Items.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/14 subtests 

Test Summary Report
-------------------
t/db_dependent/Items.t (Wstat: 256 Tests: 14 Failed: 1)
  Failed test:  14
  Non-zero exit status: 1
Files=1, Tests=14,  5 wallclock secs ( 0.02 usr  0.00 sys +  3.11 cusr  0.47 csys =  3.60 CPU)
Result: FAIL
Comment 7 Andrew Fuerste-Henry 2021-03-03 21:00:05 UTC
Talked to Kyle, clarified expected behavior. Tests pass with patch applied but permanent_location not mapped in Koha to MARC Mapping. Once it's added to Koha to MARC Mapping, tests fail. This is expected behavior.
Comment 8 Andrew Fuerste-Henry 2021-03-03 21:01:06 UTC
Created attachment 117625 [details] [review]
Bug 27837: Update permanent location if passed into ModItemFromMarc

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 9 Andrew Fuerste-Henry 2021-03-03 21:01:09 UTC
Created attachment 117626 [details] [review]
Bug 27837: Add unit tests

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 10 Andrew Fuerste-Henry 2021-03-03 21:12:01 UTC
Created attachment 117627 [details] [review]
Bug 27837: Update permanent location if passed into ModItemFromMarc

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov>
Comment 11 Andrew Fuerste-Henry 2021-03-03 21:12:05 UTC
Created attachment 117628 [details] [review]
Bug 27837: Add unit tests

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Rhonda 



Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov>
Comment 12 Jonathan Druart 2021-03-04 08:42:08 UTC
Indeed, bug 27545 is only about item creation!

The fix looks a bit weird but I didn't find some better, I've tried to fix at Koha::Item->store level but we definitely need to tell the column is dirty from ModItemFromMarc.

However I think we should mock the situation where the tests are failing if the field is mapped.
Comment 13 Jonathan Druart 2021-03-04 08:42:37 UTC
Created attachment 117634 [details] [review]
Bug 27837: Prevent the tests to fail is the field is mapped
Comment 14 Tomás Cohen Arazi 2021-03-04 14:25:49 UTC
Works as expected, tests and the QA script are happy.
Comment 15 Tomás Cohen Arazi 2021-03-04 14:26:24 UTC
Created attachment 117728 [details] [review]
Bug 27837: Update permanent location if passed into ModItemFromMarc

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Tomás Cohen Arazi 2021-03-04 14:26:28 UTC
Created attachment 117729 [details] [review]
Bug 27837: Add unit tests

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 17 Tomás Cohen Arazi 2021-03-04 14:26:33 UTC
Created attachment 117730 [details] [review]
Bug 27837: Prevent the tests to fail is the field is mapped

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 18 Kyle M Hall 2021-03-04 15:30:44 UTC
Created attachment 117735 [details] [review]
Bug 27837: Permanent location to be empty string, check if defined
Comment 19 Kyle M Hall 2021-03-04 15:39:40 UTC
Created attachment 117738 [details] [review]
Bug 27837: Permanent location to be empty string, check if key exists
Comment 20 Jonathan Druart 2021-03-05 09:39:42 UTC
Can you explain this follow-up, Kyle?
What's the use case, how did you find it?
We certainly need a couple of tests to cover it.
Comment 21 Katrin Fischer 2021-05-02 16:14:20 UTC
I've tested, QA script and unit tests pass.

Also the following test plan checks out:

Test plan: 
1 - Map permanent_location to a marc field
2 - Expose that field in the item editor
3 - Edit an item, changing location to X and permanent location to Y, save
4 - confirm record shows location=x, permanent_location=y
5 - Edit item again, change location to Z, do not change permanent location, save
6 - confirm location and permanent_location both equal Z
7 - apply patch, restart
8 - repeat steps 3-5
9 - confirm location=Z, permanent location=Y
10 - prove t/db_dependent/Items.t
Comment 22 Katrin Fischer 2021-05-02 16:16:34 UTC
Created attachment 120361 [details] [review]
Bug 27837: Update permanent location if passed into ModItemFromMarc

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 23 Katrin Fischer 2021-05-02 16:16:40 UTC
Created attachment 120362 [details] [review]
Bug 27837: Add unit tests

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 24 Katrin Fischer 2021-05-02 16:16:46 UTC
Created attachment 120363 [details] [review]
Bug 27837: Prevent the tests to fail is the field is mapped

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Katrin Fischer 2021-05-02 16:16:51 UTC
Created attachment 120364 [details] [review]
Bug 27837: Permanent location to be empty string, check if key exists

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 26 Katrin Fischer 2021-05-02 16:20:12 UTC
(In reply to Jonathan Druart from comment #20)
> Can you explain this follow-up, Kyle?
> What's the use case, how did you find it?
> We certainly need a couple of tests to cover it.

I read this as that we need 2 more unit tests that cover some more cases? Setting to failed QA to alert Kyle.
Comment 27 Nick Clemens 2021-05-05 18:46:52 UTC
Created attachment 120562 [details] [review]
Bug 27837: Update permanent location if passed into ModItemFromMarc

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 28 Nick Clemens 2021-05-05 18:46:56 UTC
Created attachment 120563 [details] [review]
Bug 27837: Add unit tests

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 29 Nick Clemens 2021-05-05 18:47:00 UTC
Created attachment 120564 [details] [review]
Bug 27837: Prevent the tests to fail is the field is mapped

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 30 Nick Clemens 2021-05-05 18:47:03 UTC
Created attachment 120565 [details] [review]
Bug 27837: Unit tests for blank permanent location
Comment 31 Nick Clemens 2021-05-05 18:47:07 UTC
Created attachment 120566 [details] [review]
Bug 27837: Permanent location to be empty string, check if key exists

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 32 Nick Clemens 2021-05-05 18:48:08 UTC
(In reply to Katrin Fischer from comment #26)
> (In reply to Jonathan Druart from comment #20)
> > Can you explain this follow-up, Kyle?
> > What's the use case, how did you find it?
> > We certainly need a couple of tests to cover it.
> 
> I read this as that we need 2 more unit tests that cover some more cases?
> Setting to failed QA to alert Kyle.

I added Unit tests, but neither the tests nor testing in the staff client pass - it seems the followup doesn't fix the issue.

Can you look Kyle?
Comment 33 Kyle M Hall 2021-05-06 12:12:44 UTC
Created attachment 120616 [details] [review]
Bug 27837: Check the MARC directly, not the transformed item

If permanent location is mapped, it will exist in the MARC.
However, when we are transforming our item hash to MARC,
we are losing our permanent location mapping, at least in unit tests.

By checking the original MARC for the permenent location, we remove the
risk of losing it the "empty" permenent location through the
transformation process. We only need to know that it existed in the
original item marc to know we need to update it in the database.
Comment 34 Kyle M Hall 2021-05-06 13:11:20 UTC
Created attachment 120628 [details] [review]
Bug 27837: Always pass permanent location from items editor if it was defined

By always passing permenent location from the items editor, we can know
if permanent location was defined in the framework. This means we can
know if we should update permanent location independently of location
even if permenent location has an empty value.
Comment 35 Kyle M Hall 2021-05-06 13:13:58 UTC
Created attachment 120629 [details] [review]
Bug 27837: Always pass permanent location from items editor if it was defined

By always passing permenent location from the items editor, we can know
if permanent location was defined in the framework. This means we can
know if we should update permanent location independently of location
even if permenent location has an empty value.
Comment 36 Katrin Fischer 2021-05-10 20:54:26 UTC
I am a bit worried about the added warnings from running the tests with and without this patch set:

With patch:
kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Items.t
t/db_dependent/Items.t .. 2/14 Use of uninitialized value in numeric eq (==) at t/db_dependent/Items.t line 247.
Use of uninitialized value $tag in hash element at /usr/share/perl5/MARC/Record.pm line 202.
Use of uninitialized value $tag in regexp compilation at /usr/share/perl5/MARC/Record.pm line 206.
Use of uninitialized value $tag in hash element at /usr/share/perl5/MARC/Record.pm line 207.
t/db_dependent/Items.t .. 12/14 Use of uninitialized value $tag in hash element at /usr/share/perl5/MARC/Record.pm line 202.
Use of uninitialized value $tag in hash element at /usr/share/perl5/MARC/Record.pm line 202.
Use of uninitialized value $tag in hash element at /usr/share/perl5/MARC/Record.pm line 202.
Use of uninitialized value $tag in hash element at /usr/share/perl5/MARC/Record.pm line 202.
t/db_dependent/Items.t .. ok     
All tests successful.
Files=1, Tests=14, 11 wallclock secs ( 0.05 usr  0.01 sys +  7.52 cusr  1.11 csys =  8.69 CPU)
Result: PASS


Without patch:
kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ git checkout master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Items.t
t/db_dependent/Items.t .. 2/14 Use of uninitialized value in numeric eq (==) at t/db_dependent/Items.t line 247.
t/db_dependent/Items.t .. ok     
All tests successful.
Comment 37 Nick Clemens 2021-05-11 11:09:41 UTC
Created attachment 120832 [details] [review]
Bug 27837: (follow-up) Do not check subfield unless tag defined

This was causing extra warnings when the permanent_location field was
unmapped.

We only need to check if there is a field defined as you can define a mapping
without both a field and subfield, so the existence of one implies the other
Comment 38 Martin Renvoize 2021-05-17 13:26:21 UTC
Created attachment 121044 [details] [review]
Bug 27837: Update permanent location if passed into ModItemFromMarc

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 39 Martin Renvoize 2021-05-17 13:26:26 UTC
Created attachment 121045 [details] [review]
Bug 27837: Add unit tests

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 40 Martin Renvoize 2021-05-17 13:26:30 UTC
Created attachment 121046 [details] [review]
Bug 27837: Prevent the tests to fail is the field is mapped

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 41 Martin Renvoize 2021-05-17 13:26:34 UTC
Created attachment 121047 [details] [review]
Bug 27837: Unit tests for blank permanent location
Comment 42 Martin Renvoize 2021-05-17 13:26:39 UTC
Created attachment 121048 [details] [review]
Bug 27837: Permanent location to be empty string, check if key exists

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 43 Martin Renvoize 2021-05-17 13:26:43 UTC
Created attachment 121049 [details] [review]
Bug 27837: Check the MARC directly, not the transformed item

If permanent location is mapped, it will exist in the MARC.
However, when we are transforming our item hash to MARC,
we are losing our permanent location mapping, at least in unit tests.

By checking the original MARC for the permenent location, we remove the
risk of losing it the "empty" permenent location through the
transformation process. We only need to know that it existed in the
original item marc to know we need to update it in the database.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 44 Martin Renvoize 2021-05-17 13:26:47 UTC
Created attachment 121050 [details] [review]
Bug 27837: Always pass permanent location from items editor if it was defined

By always passing permenent location from the items editor, we can know
if permanent location was defined in the framework. This means we can
know if we should update permanent location independently of location
even if permenent location has an empty value.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 45 Martin Renvoize 2021-05-17 13:26:51 UTC
Created attachment 121051 [details] [review]
Bug 27837: (follow-up) Do not check subfield unless tag defined

This was causing extra warnings when the permanent_location field was
unmapped.

We only need to check if there is a field defined as you can define a mapping
without both a field and subfield, so the existence of one implies the other

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 46 Martin Renvoize 2021-05-17 13:27:00 UTC
Created attachment 121052 [details] [review]
Bug 27837: (QA follow-up) Minor Spelling/Typo correction

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 47 Martin Renvoize 2021-05-17 13:27:41 UTC
Works as expected, nice use of TDD, tests all passing, QA script happy and no new warnings introduced now.

Passing QA
Comment 48 Jonathan Druart 2021-05-19 13:07:11 UTC
Last patch will be squashed when pushed.
Comment 49 Jonathan Druart 2021-05-19 13:08:06 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 50 Fridolin Somers 2021-05-20 12:51:37 UTC
Pushed to 20.11.x for 20.11.06
Comment 51 Andrew Fuerste-Henry 2021-05-25 14:20:00 UTC
Pushed to 20.05.x for 20.05.12
Comment 52 Victor Grousset/tuxayo 2021-05-25 14:53:08 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.