After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 777492 - Notify when an online account needs attention
Notify when an online account needs attention
Status: RESOLVED FIXED
Product: gnome-photos
Classification: Applications
Component: general
unspecified
Other All
: Normal enhancement
: ---
Assigned To: GNOME photos maintainer(s)
GNOME photos maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2017-01-19 13:47 UTC by Debarshi Ray
Modified: 2017-02-16 16:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add PhotosSourceNotification (10.24 KB, patch)
2017-01-21 11:23 UTC, Umang Jain
none Details | Review
Notify when an online account needs attention (1.26 KB, patch)
2017-01-21 11:24 UTC, Umang Jain
none Details | Review
Add PhotosSourceNotification (11.02 KB, patch)
2017-02-16 16:10 UTC, Debarshi Ray
committed Details | Review
source-manager: Keep track of sources that need attention (6.47 KB, patch)
2017-02-16 16:10 UTC, Debarshi Ray
committed Details | Review
embed: Notify when a source needs attention (4.64 KB, patch)
2017-02-16 16:10 UTC, Debarshi Ray
none Details | Review
embed: Notify when a source needs attention (4.83 KB, patch)
2017-02-16 16:28 UTC, Debarshi Ray
committed Details | Review
Screenshot (2.41 MB, image/png)
2017-02-16 16:33 UTC, Debarshi Ray
  Details

Description Debarshi Ray 2017-01-19 13:47:50 UTC
We should show a notification when an online account's credentials don't work anymore. It could be due to the expiry of an OAuth2 token having expired, or due to the password having changed. Currently, you can only check if an account needs attention by going to Settings -> Online Accounts.

The notification should say something like:
  "Your google credentials have expired. [open settings]"

Look at the existing code in gnome-photos to launch the Online Accounts panel. Also the following gnome-online-accounts API will be useful:
  (a) GoaClient::account-changed signal: https://developer.gnome.org/goa/stable/GoaClient.html#GoaClient-account-changed
  (b) GoaAccount:attention-needed property: https://developer.gnome.org/goa/stable/GoaAccount.html#GoaAccount--attention-needed

To force an account to expire for testing, you can use seahorse to delete the credential. Search for 'goa' and look for the account. Then invoke the EnsureCredentials D-Bus method. See: https://wiki.gnome.org/Projects/GnomeOnlineAccounts/Debugging
Comment 1 Umang Jain 2017-01-21 11:23:39 UTC
Created attachment 343947 [details] [review]
Add PhotosSourceNotification
Comment 2 Umang Jain 2017-01-21 11:24:07 UTC
Created attachment 343948 [details] [review]
Notify when an online account needs attention
Comment 3 Debarshi Ray 2017-01-26 16:04:32 UTC
Review of attachment 343948 [details] [review]:

Thanks for working on this Umang. The patches look quite good as a first attempt. Some comments below:

::: src/photos-source-manager.c
@@ +104,3 @@
 
+      if (goa_account_get_attention_needed (account))
+	{

The problem with doing it this way is that if two different online accounts have their credentials expired, then the notifications will get duplicated. eg., you are already showing a notification for Google, when the credentials for Facebook expires - you will end up with two for Google.

We need a way to track which notifications are being shown, etc.. One option is to use a GHashTable that maps the ID of the PhotosSource to the PhotosSourceNotification.
Comment 4 Debarshi Ray 2017-01-26 16:06:57 UTC
Review of attachment 343948 [details] [review]:

::: src/photos-source-manager.c
@@ +104,3 @@
 
+      if (goa_account_get_attention_needed (account))
+	{

Once the user takes care of the problem, we should withdraw the notification if it hasn't already timed out or been closed. That's another reason why we need the tracking.
Comment 5 Debarshi Ray 2017-01-26 16:51:25 UTC
Review of attachment 343947 [details] [review]:

::: src/photos-source-notification.c
@@ +101,3 @@
+
+  error = NULL;
+  app = g_app_info_create_from_commandline ("gnome-control-center online-accounts",

It would be even better if we selected the affected account:
  gnome-control-center online-accounts account_1483638297_0

The third argument is the ID of the GoaAccount.

@@ +181,3 @@
+  g_signal_connect_swapped (close, "clicked", G_CALLBACK (photos_source_notification_close), self);
+
+  photos_notification_manager_add_notification (PHOTOS_NOTIFICATION_MANAGER (self->ntfctn_mngr), GTK_WIDGET (self));

I think it will be easier if we implement this notification slightly differently from the others. All our existing notifications (except IndexingNotification) are self-managing. ie., they add themselves to NotificationManager, and at some point call gtk_widget_destroy on themselves.

However, given that we need to track these notifications in SourceManager, it will be much easier if we let SourceManager manage the addition, timeout, etc.. See below.

@@ +256,3 @@
+
+
+void

Let's return 'GtkWidget *' instead.
Comment 6 Debarshi Ray 2017-02-16 16:10:19 UTC
Created attachment 345956 [details] [review]
Add PhotosSourceNotification
Comment 7 Debarshi Ray 2017-02-16 16:10:42 UTC
Created attachment 345957 [details] [review]
source-manager: Keep track of sources that need attention
Comment 8 Debarshi Ray 2017-02-16 16:10:57 UTC
Created attachment 345958 [details] [review]
embed: Notify when a source needs attention
Comment 9 Debarshi Ray 2017-02-16 16:11:57 UTC
There is an added complication when using GTK+ from inside PhotosSourceManager. It is created before gtk_init, and it is used by the gnome-shell search provider which doesn't have a GtkWindow.
Comment 10 Debarshi Ray 2017-02-16 16:28:22 UTC
Created attachment 345960 [details] [review]
embed: Notify when a source needs attention
Comment 11 Debarshi Ray 2017-02-16 16:33:10 UTC
Created attachment 345963 [details]
Screenshot