Summary: | C4::SMS does not handle drivers with more than two names well | ||
---|---|---|---|
Product: | Koha | Reporter: | Magnus Enger <magnus> |
Component: | Notices | Assignee: | Magnus Enger <magnus> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | normal | ||
Priority: | P5 - low | CC: | fridolin.somers, m.de.rooy |
Version: | Main | ||
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: | ||
Circulation function: | |||
Attachments: |
Bug 19134 - C4::SMS falils on long driver name
[SIGNED-OFF] Bug 19134: C4::SMS falils on long driver name Bug 19134: C4::SMS falils on long driver name |
Description
Magnus Enger
2017-08-17 14:44:54 UTC
Created attachment 66151 [details] [review] Bug 19134 - C4::SMS falils on long driver name Code in C4::SMS takes the part of the SMS::Send-driver that comes after SMS::Send and tries to turn it into part of a path to a YAML file that can contain additional parameters to SMS::Send. The current code works for e.g. SMS::Send::A::B, but if there is one or more extra names, it fails to turn :: into /. So we have: SMS::Send::A::B -> SMS/Send/A/B SMS::Send::A::B::C -> SMS/Send/A/B::C This patch makes sure all occurrences of :: are turned into /, by adding a "g" modifier at the end of the regex. Testing: Testing this preperly would take a whole lot of setup for a very small change. I would suggest that the following two oneliners are enough to demonstrate that the change makes sense: $ perl -e '$x = "a::b::c"; $x =~ s|::|/|; print $x, "\n";' $ perl -e '$x = "a::b::c"; $x =~ s|::|/|g; print $x, "\n";' So: - Check that the output of these oneliners make sense - Check that the patch changes the code in a similar way to the change from the first oneliner to the second. Created attachment 66256 [details] [review] [SIGNED-OFF] Bug 19134: C4::SMS falils on long driver name Code in C4::SMS takes the part of the SMS::Send-driver that comes after SMS::Send and tries to turn it into part of a path to a YAML file that can contain additional parameters to SMS::Send. The current code works for e.g. SMS::Send::A::B, but if there is one or more extra names, it fails to turn :: into /. So we have: SMS::Send::A::B -> SMS/Send/A/B SMS::Send::A::B::C -> SMS/Send/A/B::C This patch makes sure all occurrences of :: are turned into /, by adding a "g" modifier at the end of the regex. Testing: Testing this preperly would take a whole lot of setup for a very small change. I would suggest that the following two oneliners are enough to demonstrate that the change makes sense: $ perl -e '$x = "a::b::c"; $x =~ s|::|/|; print $x, "\n";' $ perl -e '$x = "a::b::c"; $x =~ s|::|/|g; print $x, "\n";' So: - Check that the output of these oneliners make sense - Check that the patch changes the code in a similar way to the change from the first oneliner to the second. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Created attachment 66463 [details] [review] Bug 19134: C4::SMS falils on long driver name Code in C4::SMS takes the part of the SMS::Send-driver that comes after SMS::Send and tries to turn it into part of a path to a YAML file that can contain additional parameters to SMS::Send. The current code works for e.g. SMS::Send::A::B, but if there is one or more extra names, it fails to turn :: into /. So we have: SMS::Send::A::B -> SMS/Send/A/B SMS::Send::A::B::C -> SMS/Send/A/B::C This patch makes sure all occurrences of :: are turned into /, by adding a "g" modifier at the end of the regex. Testing: Testing this preperly would take a whole lot of setup for a very small change. I would suggest that the following two oneliners are enough to demonstrate that the change makes sense: $ perl -e '$x = "a::b::c"; $x =~ s|::|/|; print $x, "\n";' $ perl -e '$x = "a::b::c"; $x =~ s|::|/|g; print $x, "\n";' So: - Check that the output of these oneliners make sense - Check that the patch changes the code in a similar way to the change from the first oneliner to the second. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Magnus: Assignee field ! Pushed to master for 17.11, thanks to everybody involved! This is needed in 17.05.x too. Pushed to 17.05.x, will be in 17.05.05. Doesn't apply cleanly to 16.11.x - please provide a rebased patch if you want this to be included. |