Bug 24213

Summary: Koha::Object->get_from_storage should return undef if the object has been deleted
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: 1joynelson, hayleypelham, lucas, m.de.rooy, martin.renvoize
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.03, 19.05.08, 18.11.14
Bug Depends on:    
Bug Blocks: 21901    
Attachments: Bug 24213: Add tests
Bug 24213: Fix Koha::Object->get_from_storage if the object has been deleted
Bug 24213: Add tests
Bug 24213: Fix Koha::Object->get_from_storage if the object has been deleted
Bug 24213: Add tests
Bug 24213: Fix Koha::Object->get_from_storage if the object has been deleted

Description Jonathan Druart 2019-12-11 08:22:41 UTC
It currently explodes with something like:
  DBIC result _type  isn't of the _type Subscription at t/db_dependent/Biblio.t line 627.
Comment 1 Jonathan Druart 2019-12-11 08:41:09 UTC
Created attachment 96179 [details] [review]
Bug 24213: Add tests
Comment 2 Jonathan Druart 2019-12-11 08:41:12 UTC
Created attachment 96180 [details] [review]
Bug 24213: Fix Koha::Object->get_from_storage if the object has been deleted

Without this patch it fails with something like:
  DBIC result _type  isn't of the _type Subscription at t/db_dependent/Biblio.t line 627.

Test plan:
- Apply the first patch (Add tests)
- Confirm that they fail
- Apply this patch
- Confirm that the tests pass
Comment 3 Martin Renvoize 2020-01-30 16:05:50 UTC
Created attachment 98175 [details] [review]
Bug 24213: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize 2020-01-30 16:05:53 UTC
Created attachment 98176 [details] [review]
Bug 24213: Fix Koha::Object->get_from_storage if the object has been deleted

Without this patch it fails with something like:
  DBIC result _type  isn't of the _type Subscription at t/db_dependent/Biblio.t line 627.

Test plan:
- Apply the first patch (Add tests)
- Confirm that they fail
- Apply this patch
- Confirm that the tests pass

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2020-01-30 16:06:44 UTC
Works as expected and makes sense.. Signing off.
Comment 6 Marcel de Rooy 2020-01-31 06:38:39 UTC
QAing
Comment 7 Marcel de Rooy 2020-01-31 06:40:52 UTC
    # Subtest: Bad value tests
        1..3
        # No tests run!
    not ok 16 - No tests run for subtest "Bad value tests"
    #   Failed test 'No tests run for subtest "Bad value tests"'
    #   at t/db_dependent/Koha/Object.t line 685.
    # Looks like you failed 1 test of 16.
not ok 16 - store() tests
#   Failed test 'store() tests'
#   at t/db_dependent/Koha/Object.t line 688.

Stumbling across this fail, but also without your patch.
Comment 8 Marcel de Rooy 2020-01-31 06:43:43 UTC
(In reply to Marcel de Rooy from comment #7)
>     # Subtest: Bad value tests
>         1..3
>         # No tests run!
>     not ok 16 - No tests run for subtest "Bad value tests"
>     #   Failed test 'No tests run for subtest "Bad value tests"'
>     #   at t/db_dependent/Koha/Object.t line 685.
>     # Looks like you failed 1 test of 16.
> not ok 16 - store() tests
> #   Failed test 'store() tests'
> #   at t/db_dependent/Koha/Object.t line 688.
> 
> Stumbling across this fail, but also without your patch.

Can be ignored. Resolved with strict_sql_mode. Maybe an idea to put this key in the default koha-conf.xml (commented or zero)?
Comment 9 Marcel de Rooy 2020-01-31 06:46:21 UTC
Created attachment 98203 [details] [review]
Bug 24213: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2020-01-31 06:46:26 UTC
Created attachment 98204 [details] [review]
Bug 24213: Fix Koha::Object->get_from_storage if the object has been deleted

Without this patch it fails with something like:
  DBIC result _type  isn't of the _type Subscription at t/db_dependent/Biblio.t line 627.

Test plan:
- Apply the first patch (Add tests)
- Confirm that they fail
- Apply this patch
- Confirm that the tests pass

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Jonathan Druart 2020-01-31 10:08:35 UTC
(In reply to Marcel de Rooy from comment #8)
> (In reply to Marcel de Rooy from comment #7)
> >     # Subtest: Bad value tests
> >         1..3
> >         # No tests run!
> >     not ok 16 - No tests run for subtest "Bad value tests"
> >     #   Failed test 'No tests run for subtest "Bad value tests"'
> >     #   at t/db_dependent/Koha/Object.t line 685.
> >     # Looks like you failed 1 test of 16.
> > not ok 16 - store() tests
> > #   Failed test 'store() tests'
> > #   at t/db_dependent/Koha/Object.t line 688.
> > 
> > Stumbling across this fail, but also without your patch.
> 
> Can be ignored. Resolved with strict_sql_mode. Maybe an idea to put this key
> in the default koha-conf.xml (commented or zero)?

I do not understand what you meant.

The test passes for me, with strict_sql_mode set.
When run with prove, the strict mode is enabled automatically.
Also the flag is set for devs (so from koha-testing-docker), that's is expected, we want devs to catch such problems.
Comment 12 Martin Renvoize 2020-01-31 13:56:17 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 13 Joy Nelson 2020-02-10 23:53:01 UTC
Pushed to 19.11.x branch for 19.11.03
Comment 14 Lucas Gass 2020-02-14 21:01:10 UTC
backported to 19.05.x for 19.05.08
Comment 15 Hayley Pelham 2020-02-18 04:28:18 UTC
Backported to 18.11.x for 18.11.14.