Bug 20851

Summary: Missing module in circ/article-request-slip.pl
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: Architecture, internals, and plumbingAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: trivial    
Priority: P5 - low CC: fridolin.somers, katrin.fischer, martin.renvoize, nick
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
Attachments: Bug 20851: Add module to circ/article-request-slip.pl
Bug 20851: Add module to circ/article-request-slip.pl
Bug 20851: Add module to circ/article-request-slip.pl

Description Marcel de Rooy 2018-06-01 07:21:08 UTC
From 20468, comment7 by Owen Leonard:
By the way, I get an error when I try to print a slip: "Undefined subroutine &C4::Letters::GetPreparedLetter called at /home/vagrant/kohaclone/circ/article-request-slip.pl line 49."
Comment 1 Marcel de Rooy 2018-06-01 07:23:31 UTC
Created attachment 75719 [details] [review]
Bug 20851: Add module to circ/article-request-slip.pl

See bug 20468, comment7.
Although the script seems to compile well, we better add the module here.
Trivial change.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2018-06-01 07:24:33 UTC
Moving trivial fix to SO rightaway
Comment 3 Katrin Fischer 2018-06-02 21:50:38 UTC
I don't see a sign off or a test plan. What is this supposed to fix?

Printing article request slips works without problems for me.
Can you please explain and reset to needs signoff?
Comment 4 Marcel de Rooy 2018-06-03 17:08:46 UTC
(In reply to Katrin Fischer from comment #3)
> I don't see a sign off or a test plan. What is this supposed to fix?
> 
> Printing article request slips works without problems for me.
> Can you please explain and reset to needs signoff?

LOL
Comment 5 Katrin Fischer 2018-06-03 18:37:19 UTC
If you think my request is so silly, maybe you should epxlain it anyway.
Comment 6 Marcel de Rooy 2018-06-04 06:27:09 UTC
(In reply to Katrin Fischer from comment #5)
> If you think my request is so silly, maybe you should epxlain it anyway.

Sure :)
As mentioned in comment1 Owen got an error. And you and I did not (see also commit message). The error comes from a missing module; might be a devbox config issue for Owen.
But the script itself calls C4::Letters but does not load it. So it depends on implicit loading by one of the other modules or deeper down the chain. We should not assume it will be loaded. If one day Letters is removed elsewhere, the script does not compile. So what does this oneliner do: add the missing module for that reason. Which is indeed stated ultra compact in the commit msg.
No test plan needed. Back to SO.
Comment 7 Owen Leonard 2018-06-04 12:41:29 UTC
Created attachment 75764 [details] [review]
Bug 20851: Add module to circ/article-request-slip.pl

See bug 20468, comment7.
Although the script seems to compile well, we better add the module here.
Trivial change.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 8 Owen Leonard 2018-06-04 12:42:09 UTC
This indeed fixes the error I was getting in my devbox.
Comment 9 Katrin Fischer 2018-06-04 19:48:43 UTC
Created attachment 75792 [details] [review]
Bug 20851: Add module to circ/article-request-slip.pl

See bug 20468, comment7.
Although the script seems to compile well, we better add the module here.
Trivial change.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Katrin Fischer 2018-06-04 19:50:13 UTC
Thanks, Owen, for the sign-off and retesting.
Article slips still work for me.
Comment 11 Marcel de Rooy 2018-06-05 11:28:09 UTC
(In reply to Katrin Fischer from comment #10)
> Thanks, Owen, for the sign-off and retesting.
> Article slips still work for me.

Thx Katrin for QA
Comment 12 Nick Clemens 2018-06-08 13:53:06 UTC
Pushed to master for 18.11, awesome work all!
Comment 13 Martin Renvoize 2018-06-11 09:34:02 UTC
This patch has been pushed to 18.05.x and will be in 18.05.01
Comment 14 Fridolin SOMERS 2018-06-14 13:53:12 UTC
Pushed to 17.11.x for 17.11.07