Bug 36160

Summary:	Use $builder->build_object when creating patrons in Circulation.t
Product:	Koha	Reporter:	David Gustafsson <glasklas>
Component:	Test Suite	Assignee:	David Gustafsson <glasklas>
Status:	RESOLVED FIXED	QA Contact:	Martin Renvoize (ashimema) <martin.renvoize>
Severity:	normal
Priority:	P5 - low	CC:	david, fridolin.somers, lucas, martin.renvoize
Version:	Main
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:	24.05.00,23.11.06
Circulation function:
Attachments:	Bug 36160: Use $builder->build_object for patron objects in Circulation.t Bug 36160: Use $builder->build_object for patron objects in Circulation.t Bug 36160: Use $builder->build_object for patron objects in Circulation.t Bug 36160: Use $builder->build_object for patron objects in Circulation.t

Description David Gustafsson 2024-02-23 18:55:43 UTC

Since Patron::update_lastseen was introduced in Bug 15504 circulation tests can protentially crash with: "Invalid value passed, borrowers.updated_on=SCALAR(0x564a85c3e9b0) expected type is datetime" depending of the value of TrackLastPatronActivityTriggers. This probably does not happen on a fresh Koha installation, but did when I ran the tests locally with "Renew an item" activated for TrackLastPatronActivityTriggers.

The reason is that in the Circulation.t tests patron objects are created through Koha::Patron->new instead of using the builder object, and $self->lastseen($now)->store; is called in update_lastseen, on an object that does not have a valid updated_on, resulting in an invalid default value being assigned inKoha/Schema/Loader/mysql.pm.  Possible added future test might also crash if resulting in these patron objects being updated. This patch fixes this issue, the tests still fail for other reasons:

#   Failed test 'transferbook tests'
#   at t/db_dependent/Circulation.t line 5769.
Exception 'Koha::Exceptions::MissingParameter' thrown 'The to parameter is mandatory'

But will create another issue for this if not due to some strange local issues.

Comment 1 David Gustafsson 2024-02-23 19:01:07 UTC

Created attachment 162398 [details] [review]
Bug 36160: Use $builder->build_object for patron objects in Circulation.t

1) Set TrackLastPatronActivityTriggers to at least "Renew an item"
2) Run tests in t/db_dependent/Circulation.t and verify that
   failes with "Invalid value passed, borrowers.updated_on..."
3) Apply patch
4) Run Circulatoint.t tests again and verify that no longer
   produces this error

Comment 2 David Nind 2024-02-23 19:37:35 UTC

The tests don't fail for me. Is there something else that needs to be done first?

Comment 3 David Gustafsson 2024-02-26 13:45:41 UTC

Created attachment 162434 [details] [review]
Bug 36160: Use $builder->build_object for patron objects in Circulation.t

1) Set TrackLastPatronActivityTriggers to at least "Checking out an item"
2) Run tests in t/db_dependent/Circulation.t and verify that
   failes with "Invalid value passed, borrowers.updated_on..."
3) Apply patch
4) Run Circulatoint.t tests again and verify that no longer
   produces this error

Comment 4 David Gustafsson 2024-02-26 13:47:37 UTC

(In reply to David Nind from comment #2)
> The tests don't fail for me. Is there something else that needs to be done
> first?

I assumed it was "Renew an item" since "CanBookBeRenewed test" failed, but it seems it's "Checking out an item" that needs to be enabled (I had them both enabled).

Comment 5 Pedro Amorim 2024-02-26 14:25:42 UTC

Created attachment 162435 [details] [review]
Bug 36160: Use $builder->build_object for patron objects in Circulation.t

1) Set TrackLastPatronActivityTriggers to at least "Checking out an item"
2) Run tests in t/db_dependent/Circulation.t and verify that
   failes with "Invalid value passed, borrowers.updated_on..."
3) Apply patch
4) Run Circulatoint.t tests again and verify that no longer
   produces this error

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Comment 6 Martin Renvoize (ashimema) 2024-04-24 13:57:09 UTC

Created attachment 165461 [details] [review]
Bug 36160: Use $builder->build_object for patron objects in Circulation.t

1) Set TrackLastPatronActivityTriggers to at least "Checking out an item"
2) Run tests in t/db_dependent/Circulation.t and verify that
   failes with "Invalid value passed, borrowers.updated_on..."
3) Apply patch
4) Run Circulatoint.t tests again and verify that no longer
   produces this error

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

Comment 7 Martin Renvoize (ashimema) 2024-04-24 13:57:40 UTC

More reliable tests is always a win.. thanks David!

Tests continue to make sense and pass, Passing QA

Comment 8 Katrin Fischer 2024-04-26 12:16:57 UTC

Pushed for 24.05!

Well done everyone, thank you!

Comment 9 Fridolin Somers 2024-05-23 15:24:56 UTC

Pushed to 23.11.x for 23.11.06

Comment 10 Lucas Gass (lukeg) 2024-05-28 20:29:10 UTC

Not needed in 23.05