1176483 - Don't allow to set SMS notifications as reminder action because Google Calendar no longer supports it

Status: RESOLVED FIXED

(Calendar :: Provider: GData, defect)

Description

[Stefan Sitter [:ssitter]]

Provider for Google Calendar adds the possibility to select SMS notification as reminder action for its calendars. Google Calendar no longer supports this:

<https://support.google.com/calendar/answer/45351?hl=en>
> SMS notifications are no longer available
> 
> Starting June 27th, SMS notifications from Google Calendar will no
> longer be available.
> 
> SMS notifications launched before smartphones were available. With
> smartphones, you can get notifications from the calendar app on your
> device, or the Google Calendar app for Android or iPhone.
> 
> If you had SMS notifications turned on, you'll see pop-up
> notifications instead. Learn more about notifications.
> 
> Note: Google Apps for Work, Education, and Government customers can
> still get SMS notifications.

Therefore it might be recommended to not allow to set SMS notifications as reminder action anymore. Or maybe it depends on how Google Calendar behaves after this date when you set this type of reminder. Will they throw an error? Will they ignore it and the user might miss the event? Will they downgrade it to pop-up notification that works in Lightning too or only in the web interface?

Comment 1

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix - v1

We can't quite kill the SMS feature yet, since Google Apps for Work/Education/Government still support it. Unfortunately there is no sure-fire way to identify this case. I'm putting it behind a pref for now and will add a FAQ item for apps folks to re-enable it.

Comment 2

[[:MakeMyDay]]

Comment on attachment 8650732 [details] [diff] [review]
Fix - v1

Review of attachment 8650732 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with the comments below considered.

::: calendar/providers/gdata/components/calGoogleCalendar.js
@@ +276,5 @@
>              case "capabilities.alarms.maxCount":
>                  return 5;
>              case "capabilities.alarms.actionValues":
> +                let actionValues = ["DISPLAY", "EMAIL"];
> +                if (Preferences.get("calendar.google.enableSMSReminders")) {

You should add the default value for the pref here:
if (Preferences.get("calendar.google.enableSMSReminders", false)) {

::: calendar/providers/gdata/content/gdata-event-dialog-reminder.js
@@ +75,5 @@
> +     * SMS Reminders are only supported for Google Apps for Work, Education,
> +     * and Government. hide the menuitem if SMS reminders are not supported
> +     */
> +    function hideSMSReminders() {
> +        if (!Preferences.get("calendar.google.enableSMSReminders")) {

Here's also missing the default value.

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

In this case it is fine, because the default value is false and if not passed then undefined will be returned.

Comment 4

[aleth [:aleth]]

Bitrotted :-/

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Just patch order. Should apply fine now, I'll push in a moment.

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Pushed to comm-central changeset b5f568bee812