Description
Fridolin Somers
2017-05-29 14:26:55 UTC
Created attachment 63814 [details] [review] Bug 18692 - When SMS is enabled the OPAC messaging table is misaligned Bug 6726 had corrected the fact that when SMS is enabled the messaging table is missing a column. Bug 6458 has broken this. The SMS column is missing an else case with cell containing only "-" like other columns. Test plan : - set SMSSendDriver preference empty - go to OPAC patron messaging - column SMS should not be visible - set SMSSendDriver preference not empty - go to OPAC patron messaging - column SMS appears with checkboxes Created attachment 63830 [details]
Email notifications can no longer be edited with patch
Patch is applied, syspref SMSSendDriver is empty.
Result: In column email we have a minus sign (-) instead of check boxes as without patch.
See screenshot.
Patch tested on sandbox 1 by Biblibre. I agree with Marc Véron. When syspref SMSDriver is full filled (for example with email), columns are ok. When syspref SMSDriver is empty, it doesn't work because in column email we should see check boxes instead of "-". Check boxes should be in colunm email and in column not notify to allow our user to choose the messages they want to receive. I'm not sure I understood the original problem, perhaps screenshots would help. Is message_transports / message_transport_types table properly configured? Anyway I actually wanted to comment that I also noticed "sms" is missing "-" in the messaging preferences table both in OPAC and staff when it has no sms transport configured in message_transports database table. The same for "phone". To my understanding, message_transports table defines availability of transport options for the messaging preferences table. sms and phone should not have checkboxes when they are removed from those options. This feature used to exist but for some reason the "-" got completely removed to fix misalignment issues. Created attachment 63872 [details] [review] Bug 18692: (follow-up) Fix misplaced columns and add this feature for phone as well Fixes misplaced columns introduced by previous patch and adds the "-" for phone transport type. To test: 1. Set SMSSendDriver system preference on 2. Go to intra and OPAC messaging preferences 3. By default you should see checkboxes for all messages for SMS 4. Ensure columns are not misplaced (pushing one column too much to the right) 5. Delete sms method from one of the messages in message_transports table 6. Observe that "-" is displayed instead of checkbox for that message for SMS 7. Repeat same for TalkingTechItivaPhoneNotification system preference. By default it may not have transports in message_transports, so make sure to assign some in order to have the checkboxes visible. Ahhhh, I did not see that the TT vars where different : messaging_preference.transport_sms and messaging_preference.transport[s]_sms I will correct my path. Created attachment 63897 [details] [review] Bug 18692 - When SMS is enabled the OPAC messaging table is misaligned Bug 6726 had corrected the fact that when SMS is enabled the messaging table is missing a column. Bug 6458 has broken this. The SMS column is missing an else case with cell containing only "-" like other columns. Test plan : - set SMSSendDriver preference empty - go to OPAC patron messaging - column SMS should not be visible - set SMSSendDriver preference not empty - go to OPAC patron messaging - column SMS appears with checkboxes Created attachment 63898 [details] [review] Bug 18692 - same with syspref TalkingTechItivaPhone Created attachment 63900 [details] [review] Bug 18692 - same with syspref TalkingTechItivaPhone Fixes misplaced columns introduced by previous patch and adds the "-" for phone transport type. To test: 1. Set SMSSendDriver system preference on 2. Go to intra and OPAC messaging preferences 3. By default you should see checkboxes for all messages for SMS 4. Ensure columns are not misplaced (pushing one column too much to the right) 5. Delete sms method from one of the messages in message_transports table 6. Observe that "-" is displayed instead of checkbox for that message for SMS 7. Repeat same for TalkingTechItivaPhoneNotification system preference. By default it may not have transports in message_transports, so make sure to assign some in order to have the checkboxes visible. https://bugs.koha-community.org/show_bug.cgi?id=8692 @Lari Taskula, I have adapted you patch. Tell me if its ok. Created attachment 64409 [details] [review] Bug 18692 - When SMS is enabled the OPAC messaging table is misaligned Bug 6726 had corrected the fact that when SMS is enabled the messaging table is missing a column. Bug 6458 has broken this. The SMS column is missing an else case with cell containing only "-" like other columns. Test plan : - set SMSSendDriver preference empty - go to OPAC patron messaging - column SMS should not be visible - set SMSSendDriver preference not empty - go to OPAC patron messaging - column SMS appears with checkboxes Signed-off-by: Michael Andrew Cabus <michael@bywatersolutons.com> Created attachment 64410 [details] [review] Bug 18692 - same with syspref TalkingTechItivaPhone Fixes misplaced columns introduced by previous patch and adds the "-" for phone transport type. To test: 1. Set SMSSendDriver system preference on 2. Go to intra and OPAC messaging preferences 3. By default you should see checkboxes for all messages for SMS 4. Ensure columns are not misplaced (pushing one column too much to the right) 5. Delete sms method from one of the messages in message_transports table 6. Observe that "-" is displayed instead of checkbox for that message for SMS 7. Repeat same for TalkingTechItivaPhoneNotification system preference. By default it may not have transports in message_transports, so make sure to assign some in order to have the checkboxes visible. https://bugs.koha-community.org/show_bug.cgi?id=8692 Signed-off-by: Michael Andrew Cabus <michael@bywatersolutons.com> Created attachment 65170 [details] [review] Bug 18692 - When SMS is enabled the OPAC messaging table is misaligned Bug 6726 had corrected the fact that when SMS is enabled the messaging table is missing a column. Bug 6458 has broken this. The SMS column is missing an else case with cell containing only "-" like other columns. Test plan : - set SMSSendDriver preference empty - go to OPAC patron messaging - column SMS should not be visible - set SMSSendDriver preference not empty - go to OPAC patron messaging - column SMS appears with checkboxes Signed-off-by: Michael Andrew Cabus <michael@bywatersolutons.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 65171 [details] [review] Bug 18692 - same with syspref TalkingTechItivaPhone Fixes misplaced columns introduced by previous patch and adds the "-" for phone transport type. To test: 1. Set SMSSendDriver system preference on 2. Go to intra and OPAC messaging preferences 3. By default you should see checkboxes for all messages for SMS 4. Ensure columns are not misplaced (pushing one column too much to the right) 5. Delete sms method from one of the messages in message_transports table 6. Observe that "-" is displayed instead of checkbox for that message for SMS 7. Repeat same for TalkingTechItivaPhoneNotification system preference. By default it may not have transports in message_transports, so make sure to assign some in order to have the checkboxes visible. https://bugs.koha-community.org/show_bug.cgi?id=8692 Signed-off-by: Michael Andrew Cabus <michael@bywatersolutons.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Patches work nicely, Lari's patches included fixes for the intranet as well and those seem to have been dropped. Can we add those back in? (In reply to Nick Clemens from comment #15) > Patches work nicely, Lari's patches included fixes for the intranet as well > and those seem to have been dropped. Can we add those back in? Oh indeed. I create a follow up from patch https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63872 Created attachment 65451 [details] [review] Bug 18692 - intranet part Fixes misplaced columns introduced by previous patch and adds the "-" for phone transport type. To test: 1. Set SMSSendDriver system preference on 2. Go to intranet messaging preferences 3. By default you should see checkboxes for all messages for SMS 4. Ensure columns are not misplaced (pushing one column too much to the right) 5. Delete sms method from one of the messages in message_transports table 6. Observe that "-" is displayed instead of checkbox for that message for SMS 7. Repeat same for TalkingTechItivaPhoneNotification system preference. By default it may not have transports in message_transports, so make sure to assign some in order to have the checkboxes visible. Created attachment 65906 [details] [review] [SIGNED-OFF] Bug 18692 - intranet part Fixes misplaced columns introduced by previous patch and adds the "-" for phone transport type. To test: 1. Set SMSSendDriver system preference on 2. Go to intranet messaging preferences 3. By default you should see checkboxes for all messages for SMS 4. Ensure columns are not misplaced (pushing one column too much to the right) 5. Delete sms method from one of the messages in message_transports table 6. Observe that "-" is displayed instead of checkbox for that message for SMS 7. Repeat same for TalkingTechItivaPhoneNotification system preference. By default it may not have transports in message_transports, so make sure to assign some in order to have the checkboxes visible. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 66731 [details] [review] Bug 18692 - intranet part Fixes misplaced columns introduced by previous patch and adds the "-" for phone transport type. To test: 1. Set SMSSendDriver system preference on 2. Go to intranet messaging preferences 3. By default you should see checkboxes for all messages for SMS 4. Ensure columns are not misplaced (pushing one column too much to the right) 5. Delete sms method from one of the messages in message_transports table 6. Observe that "-" is displayed instead of checkbox for that message for SMS 7. Repeat same for TalkingTechItivaPhoneNotification system preference. By default it may not have transports in message_transports, so make sure to assign some in order to have the checkboxes visible. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 17.11, thanks to everybody involved! Pushed to 17.05.x, will be in 17.05.05. These patches have been pushed to 16.11.x and will be in 16.11.13. Pushed to 16.05.x, for 16.05.18 release |