[Bug 14683] New: Unable to clear SMS number
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Bug ID: 14683 Summary: Unable to clear SMS number Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: OPAC Assignee: oleonard@myacpl.org Reporter: larit@student.uef.fi QA Contact: testopia@bugs.koha-community.org In opac-messaging.pl (your messaging) you cannot unset your SMS number once it has been set if your mobile (other phone) field is empty. When checking for modified SMS number, we also check if SMS number does not equal mobile phone. This is causing the bug: $query->param('SMSnumber') ne $borrower->{'mobile'} Since mobile is empty and the given SMS number is also empty, the given SMS number will not be saved. Does anybody have an idea why we have this kind of condition? to reproduce: -2. Make sure borrower does not have mobile (other phone) set -1. Make sure you have SMS messaging enabled 1. Navigate to your messaging 2. Insert SMS number and submit 3. Clear SMS number and submit 4. Observe that the sms number did not change -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 --- Comment #1 from Joonas Kylmälä <j.kylmala@gmail.com> --- Created attachment 41664 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41664&action=edit Bug 14683: Unable to clear SMS number Enables to clear SMS number. To test: 1. Go to opac-messaging.pl 2. Insert SMS number and submit 3. Clear SMS number and submit 4. Observe that the sms number did not change 5. Apply patch 6. Clear SMS number and submit 7. Observe that the sms number changes Sponsored-by: Vaara-kirjastot -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Joonas Kylmälä <j.kylmala@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff CC| |j.kylmala@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Joonas Kylmälä <j.kylmala@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|oleonard@myacpl.org |j.kylmala@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Still testing this, still see an issue here. Finish it tomorrow. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #41664|0 |1 is obsolete| | --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 42050 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42050&action=edit Bug 14683: Unable to clear SMS number Enables to clear SMS number. To test: 1. Go to opac-messaging.pl 2. Insert SMS number and submit 3. Clear SMS number and submit 4. Observe that the sms number did not change 5. Apply patch 6. Clear SMS number and submit 7. Observe that the sms number changes Sponsored-by: Vaara-kirjastot Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Adding a follow-up. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 42051 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42051&action=edit Bug 14683: [QA Follow-up] Mixup between mobile and smsalertnumber This is an issue discussed on older reports already in the past. Column mobile in borrowers is actually 'Other phone', not necessary a mobile number. The name of the field is confusing. (Renaming it is outside the scope of this report.) The field that we are editing here is smsalertnumber. It should not be compared with mobile at all. What could be the side-effect of this correction? === First, the change is only relevant for libraries with pref SMSSendDriver enabled. In the past patrons editing their message preferences saw mobile (read: other phone) in their smsalertnumber field (if the latter was still empty). If they saved it, it was copied to smsalertnumber. This change does not affect these patrons. They just have the same number in two columns. No big deal. What if a patron does not yet have a smsalertnumber? In that case no sms is sent in Letters.pm. So no change in behavior. If he submits opac-messaging now, he will no longer copy his other phone to smsalert [we cannot assume that it was mobile anyway!]. If he enters a mobile number, it will be saved correctly in the right field. Conclusion: this change will not break things or hurt anyone. It only prevents unwanted copying other phone to smsalertnumber. Also modified the compare to prevent uninitialized warnings. And removed a commented warn. Test plan: [1] Add, edit or delete the SMS number on opac-messaging regardless of the value of Other Phone (in the badly named mobile field). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=14750 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- The same line should be affected at the intranet too (members/memberentry.pl) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 42162 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42162&action=edit Bug 14683: [QA Follow-up] Similar change for staff Script memberentry.pl contained a similar line. Solution is simpler here. Test plan: [1] Add, change or clear the sms number at staff side. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 42187 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42187&action=edit [PASSED QA] Bug 14683: Unable to clear SMS number Enables to clear SMS number. To test: 1. Go to opac-messaging.pl 2. Insert SMS number and submit 3. Clear SMS number and submit 4. Observe that the sms number did not change 5. Apply patch 6. Clear SMS number and submit 7. Observe that the sms number changes Sponsored-by: Vaara-kirjastot Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Adding a follow-up. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 42188 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42188&action=edit [PASSED QA] Bug 14683: [QA Follow-up] Mixup between mobile and smsalertnumber This is an issue discussed on older reports already in the past. Column mobile in borrowers is actually 'Other phone', not necessary a mobile number. The name of the field is confusing. (Renaming it is outside the scope of this report.) The field that we are editing here is smsalertnumber. It should not be compared with mobile at all. What could be the side-effect of this correction? === First, the change is only relevant for libraries with pref SMSSendDriver enabled. In the past patrons editing their message preferences saw mobile (read: other phone) in their smsalertnumber field (if the latter was still empty). If they saved it, it was copied to smsalertnumber. This change does not affect these patrons. They just have the same number in two columns. No big deal. What if a patron does not yet have a smsalertnumber? In that case no sms is sent in Letters.pm. So no change in behavior. If he submits opac-messaging now, he will no longer copy his other phone to smsalert [we cannot assume that it was mobile anyway!]. If he enters a mobile number, it will be saved correctly in the right field. Conclusion: this change will not break things or hurt anyone. It only prevents unwanted copying other phone to smsalertnumber. Also modified the compare to prevent uninitialized warnings. And removed a commented warn. Test plan: [1] Add, edit or delete the SMS number on opac-messaging regardless of the value of Other Phone (in the badly named mobile field). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 42189 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42189&action=edit [PASSED QA] Bug 14683: [QA Follow-up] Similar change for staff Script memberentry.pl contained a similar line. Solution is simpler here. Test plan: [1] Add, change or clear the sms number at staff side. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #42050|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #42051|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #42162|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I feel a bit like Marcel kidnapped the bug ;)... but the changes make sense to me. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Joonas! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz Status|Pushed to Master |Pushed to Stable --- Comment #12 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.20.x will be in 3.20.4 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14683 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |liz@catalyst.net.nz --- Comment #13 from Liz Rea <liz@catalyst.net.nz> --- Pushed to 3.18.x will be in 3.18.11 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org