From 0a1e76d866379ce6115aacd2cdee75db15bebab0 Mon Sep 17 00:00:00 2001
From: Kalev Lember <klember@redhat.com>
Date: Tue, 15 Jan 2019 23:48:33 +0100
Subject: [PATCH 1/3] shell: Improve the error message when whole update
 operation fails

In case the whole updates operation fails and we don't have the data
which app failed, show 'Unable to install updates' error instead of
'Unable to update "(null)"'.

Fixes: https://gitlab.gnome.org/GNOME/gnome-software/issues/480
---
 src/gs-shell.c | 120 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 87 insertions(+), 33 deletions(-)

diff --git a/src/gs-shell.c b/src/gs-shell.c
index 6f1cda816..7c2f86d81 100644
--- a/src/gs-shell.c
+++ b/src/gs-shell.c
@@ -1355,23 +1355,34 @@ gs_shell_show_event_update (GsShell *shell, GsPluginEvent *event)
 	g_autofree gchar *str_origin = NULL;
 	g_autoptr(GString) str = g_string_new (NULL);
 
-	str_app = gs_shell_get_title_from_app (app);
 	switch (error->code) {
 	case GS_PLUGIN_ERROR_DOWNLOAD_FAILED:
-		if (origin != NULL) {
+		if (app != NULL && origin != NULL) {
+			str_app = gs_shell_get_title_from_app (app);
 			str_origin = gs_shell_get_title_from_origin (origin);
 			/* TRANSLATORS: failure text for the in-app notification,
 			 * where the first %s is the app name (e.g. "GIMP") and
 			 * the second %s is the origin, e.g. "Fedora" or
 			 * "Fedora Project [fedoraproject.org]" */
-			g_string_append_printf (str, _("Unable to update %s from %s"),
+			g_string_append_printf (str, _("Unable to update %s from %s as download failed"),
 					       str_app, str_origin);
 			buttons = TRUE;
-		} else {
+		} else if (app != NULL) {
+			str_app = gs_shell_get_title_from_app (app);
 			/* TRANSLATORS: failure text for the in-app notification,
 			 * where the %s is the application name (e.g. "GIMP") */
 			g_string_append_printf (str, _("Unable to update %s as download failed"),
 						str_app);
+		} else if (origin != NULL) {
+			str_origin = gs_shell_get_title_from_origin (origin);
+			/* TRANSLATORS: failure text for the in-app notification,
+			 * where the %s is the origin, e.g. "Fedora" or
+			 * "Fedora Project [fedoraproject.org]" */
+			g_string_append_printf (str, _("Unable to install updates from %s as download failed"),
+						str_origin);
+		} else {
+			/* TRANSLATORS: failure text for the in-app notification */
+			g_string_append_printf (str, _("Unable to install updates as download failed"));
 		}
 		break;
 	case GS_PLUGIN_ERROR_NO_NETWORK:
@@ -1382,49 +1393,92 @@ gs_shell_show_event_update (GsShell *shell, GsPluginEvent *event)
 		buttons |= GS_SHELL_EVENT_BUTTON_NETWORK_SETTINGS;
 		break;
 	case GS_PLUGIN_ERROR_NO_SPACE:
-		/* TRANSLATORS: failure text for the in-app notification,
-		 * where the %s is the application name (e.g. "GIMP") */
-		g_string_append_printf (str, _("Unable to update %s: "
-					       "not enough disk space"),
-					str_app);
+		if (app != NULL) {
+			str_app = gs_shell_get_title_from_app (app);
+			/* TRANSLATORS: failure text for the in-app notification,
+			 * where the %s is the application name (e.g. "GIMP") */
+			g_string_append_printf (str, _("Unable to update %s: "
+						       "not enough disk space"),
+						str_app);
+		} else {
+			/* TRANSLATORS: failure text for the in-app notification */
+			g_string_append_printf (str, _("Unable to install updates: "
+						       "not enough disk space"));
+		}
 		buttons |= GS_SHELL_EVENT_BUTTON_NO_SPACE;
 		break;
 	case GS_PLUGIN_ERROR_AUTH_REQUIRED:
 	case GS_PLUGIN_ERROR_PIN_REQUIRED:
-		/* TRANSLATORS: failure text for the in-app notification,
-		 * where the %s is the application name (e.g. "GIMP") */
-		g_string_append_printf (str, _("Unable to update %s: "
-					       "authentication was required"),
-					str_app);
+		if (app != NULL) {
+			str_app = gs_shell_get_title_from_app (app);
+			/* TRANSLATORS: failure text for the in-app notification,
+			 * where the %s is the application name (e.g. "GIMP") */
+			g_string_append_printf (str, _("Unable to update %s: "
+						       "authentication was required"),
+						str_app);
+		} else {
+			/* TRANSLATORS: failure text for the in-app notification */
+			g_string_append_printf (str, _("Unable to install updates: "
+						       "authentication was required"));
+		}
 		break;
 	case GS_PLUGIN_ERROR_AUTH_INVALID:
-		/* TRANSLATORS: failure text for the in-app notification,
-		 * where the %s is the application name (e.g. "GIMP") */
-		g_string_append_printf (str, _("Unable to update %s: "
-					       "authentication was invalid"),
-					str_app);
+		if (app != NULL) {
+			str_app = gs_shell_get_title_from_app (app);
+			/* TRANSLATORS: failure text for the in-app notification,
+			 * where the %s is the application name (e.g. "GIMP") */
+			g_string_append_printf (str, _("Unable to update %s: "
+						       "authentication was invalid"),
+						str_app);
+		} else {
+			/* TRANSLATORS: failure text for the in-app notification */
+			g_string_append_printf (str, _("Unable to install updates: "
+						       "authentication was invalid"));
+		}
 		break;
 	case GS_PLUGIN_ERROR_NO_SECURITY:
-		/* TRANSLATORS: failure text for the in-app notification,
-		 * where the %s is the application name (e.g. "GIMP") */
-		g_string_append_printf (str, _("Unable to update %s: "
-					       "you do not have permission to "
-					       "update software"),
-					str_app);
+		if (app != NULL) {
+			str_app = gs_shell_get_title_from_app (app);
+			/* TRANSLATORS: failure text for the in-app notification,
+			 * where the %s is the application name (e.g. "GIMP") */
+			g_string_append_printf (str, _("Unable to update %s: "
+						       "you do not have permission to "
+						       "update software"),
+						str_app);
+		} else {
+			/* TRANSLATORS: failure text for the in-app notification */
+			g_string_append_printf (str, _("Unable to install updates: "
+						       "you do not have permission to "
+						       "update software"));
+		}
 		break;
 	case GS_PLUGIN_ERROR_AC_POWER_REQUIRED:
-		/* TRANSLATORS: failure text for the in-app notification,
-		 * where the %s is the application name (e.g. "Dell XPS 13") */
-		g_string_append_printf (str, _("Unable to update %s: "
-					       "AC power is required"),
-					str_app);
+		if (app != NULL) {
+			str_app = gs_shell_get_title_from_app (app);
+			/* TRANSLATORS: failure text for the in-app notification,
+			 * where the %s is the application name (e.g. "Dell XPS 13") */
+			g_string_append_printf (str, _("Unable to update %s: "
+						       "AC power is required"),
+						str_app);
+		} else {
+			/* TRANSLATORS: failure text for the in-app notification,
+			 * where the %s is the application name (e.g. "Dell XPS 13") */
+			g_string_append_printf (str, _("Unable to install updates: "
+						       "AC power is required"));
+		}
 		break;
 	case GS_PLUGIN_ERROR_CANCELLED:
 		break;
 	default:
-		/* TRANSLATORS: failure text for the in-app notification,
-		 * where the %s is the application name (e.g. "GIMP") */
-		g_string_append_printf (str, _("Unable to update %s"), str_app);
+		if (app != NULL) {
+			str_app = gs_shell_get_title_from_app (app);
+			/* TRANSLATORS: failure text for the in-app notification,
+			 * where the %s is the application name (e.g. "GIMP") */
+			g_string_append_printf (str, _("Unable to update %s"), str_app);
+		} else {
+			/* TRANSLATORS: failure text for the in-app notification */
+			g_string_append_printf (str, _("Unable to install updates"));
+		}
 		break;
 	}
 	if (str->len == 0)
-- 
2.21.0


From af764483533ac1afc42a5c0f1c6b76c43be1d24d Mon Sep 17 00:00:00 2001
From: Kalev Lember <klember@redhat.com>
Date: Tue, 15 Jan 2019 23:49:50 +0100
Subject: [PATCH 2/3] shell: Add details for a system upgrade error

Make the "internet access was required" error look the same as the rest
of the system upgrade errors, showing which distro version we were
trying to upgrade to.
---
 src/gs-shell.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/gs-shell.c b/src/gs-shell.c
index 7c2f86d81..7c97740fa 100644
--- a/src/gs-shell.c
+++ b/src/gs-shell.c
@@ -1534,10 +1534,12 @@ gs_shell_show_event_upgrade (GsShell *shell, GsPluginEvent *event)
 		}
 		break;
 	case GS_PLUGIN_ERROR_NO_NETWORK:
-		/* TRANSLATORS: failure text for the in-app notification */
-		g_string_append (str, _("Unable to upgrade: "
-					"internet access was required but "
-					"wasn’t available"));
+		/* TRANSLATORS: failure text for the in-app notification,
+		 * where the %s is the distro name (e.g. "Fedora 25") */
+		g_string_append_printf (str, _("Unable to upgrade to %s: "
+					       "internet access was required but "
+					       "wasn’t available"),
+					str_app);
 		buttons |= GS_SHELL_EVENT_BUTTON_NETWORK_SETTINGS;
 		break;
 	case GS_PLUGIN_ERROR_NO_SPACE:
-- 
2.21.0


From 0d3c47f69e7f6452412a0a3e6704106eefc0c658 Mon Sep 17 00:00:00 2001
From: Kalev Lember <klember@redhat.com>
Date: Tue, 15 Jan 2019 23:55:22 +0100
Subject: [PATCH 3/3] shell: Improve the logic when to show detailed error
 messages

Make sure we never have "Sorry, something went wrong" without actual
details what went wrong.
---
 src/gs-shell.c | 64 +++++++++++---------------------------------------
 1 file changed, 14 insertions(+), 50 deletions(-)

diff --git a/src/gs-shell.c b/src/gs-shell.c
index 7c97740fa..3eea95028 100644
--- a/src/gs-shell.c
+++ b/src/gs-shell.c
@@ -990,16 +990,6 @@ gs_shell_get_title_from_app (GsApp *app)
 	return g_strdup_printf (_("“%s”"), gs_app_get_id (app));
 }
 
-static gboolean
-gs_shell_show_detailed_error (GsShell *shell, const GError *error)
-{
-	if (error->code == GS_PLUGIN_ERROR_FAILED)
-		return TRUE;
-	if (error->code == GS_PLUGIN_ERROR_DOWNLOAD_FAILED)
-		return TRUE;
-	return FALSE;
-}
-
 static gchar *
 get_first_line (const gchar *str)
 {
@@ -1055,6 +1045,7 @@ gs_shell_show_event_refresh (GsShell *shell, GsPluginEvent *event)
 			/* TRANSLATORS: failure text for the in-app notification */
 			g_string_append (str, _("Unable to download updates"));
 		}
+		gs_shell_append_detailed_error (shell, str, error);
 		break;
 	case GS_PLUGIN_ERROR_NO_NETWORK:
 		/* TRANSLATORS: failure text for the in-app notification */
@@ -1106,6 +1097,7 @@ gs_shell_show_event_refresh (GsShell *shell, GsPluginEvent *event)
 			/* TRANSLATORS: failure text for the in-app notification */
 			g_string_append (str, _("Unable to get list of updates"));
 		}
+		gs_shell_append_detailed_error (shell, str, error);
 		break;
 	}
 	if (str->len == 0)
@@ -1121,10 +1113,6 @@ gs_shell_show_event_refresh (GsShell *shell, GsPluginEvent *event)
 		}
 	}
 
-	/* add extra debugging for debug builds */
-	if (gs_shell_show_detailed_error (shell, error))
-		gs_shell_append_detailed_error (shell, str, error);
-
 	/* show in-app notification */
 	gs_shell_show_event_app_notify (shell, str->str, buttons);
 	return TRUE;
@@ -1173,15 +1161,12 @@ gs_shell_show_event_purchase (GsShell *shell, GsPluginEvent *event)
 		/* TRANSLATORS: failure text for the in-app notification,
 		 * where the %s is the application name (e.g. "GIMP") */
 		g_string_append_printf (str, _("Unable to purchase %s"), str_app);
+		gs_shell_append_detailed_error (shell, str, error);
 		break;
 	}
 	if (str->len == 0)
 		return FALSE;
 
-	/* add extra debugging for debug builds */
-	if (gs_shell_show_detailed_error (shell, error))
-		gs_shell_append_detailed_error (shell, str, error);
-
 	/* show in-app notification */
 	gs_shell_show_event_app_notify (shell, str->str, GS_SHELL_EVENT_BUTTON_NONE);
 	return TRUE;
@@ -1218,6 +1203,7 @@ gs_shell_show_event_install (GsShell *shell, GsPluginEvent *event)
 						       "as download failed"),
 						str_app);
 		}
+		gs_shell_append_detailed_error (shell, str, error);
 		break;
 	case GS_PLUGIN_ERROR_NOT_SUPPORTED:
 		if (origin != NULL) {
@@ -1319,6 +1305,7 @@ gs_shell_show_event_install (GsShell *shell, GsPluginEvent *event)
 		/* TRANSLATORS: failure text for the in-app notification,
 		 * where the %s is the application name (e.g. "GIMP") */
 		g_string_append_printf (str, _("Unable to install %s"), str_app);
+		gs_shell_append_detailed_error (shell, str, error);
 		break;
 	}
 	if (str->len == 0)
@@ -1334,10 +1321,6 @@ gs_shell_show_event_install (GsShell *shell, GsPluginEvent *event)
 		}
 	}
 
-	/* add extra debugging for debug builds */
-	if (gs_shell_show_detailed_error (shell, error))
-		gs_shell_append_detailed_error (shell, str, error);
-
 	/* show in-app notification */
 	gs_shell_show_event_app_notify (shell, str->str, buttons);
 	return TRUE;
@@ -1384,6 +1367,7 @@ gs_shell_show_event_update (GsShell *shell, GsPluginEvent *event)
 			/* TRANSLATORS: failure text for the in-app notification */
 			g_string_append_printf (str, _("Unable to install updates as download failed"));
 		}
+		gs_shell_append_detailed_error (shell, str, error);
 		break;
 	case GS_PLUGIN_ERROR_NO_NETWORK:
 		/* TRANSLATORS: failure text for the in-app notification */
@@ -1479,6 +1463,7 @@ gs_shell_show_event_update (GsShell *shell, GsPluginEvent *event)
 			/* TRANSLATORS: failure text for the in-app notification */
 			g_string_append_printf (str, _("Unable to install updates"));
 		}
+		gs_shell_append_detailed_error (shell, str, error);
 		break;
 	}
 	if (str->len == 0)
@@ -1494,10 +1479,6 @@ gs_shell_show_event_update (GsShell *shell, GsPluginEvent *event)
 		}
 	}
 
-	/* add extra debugging for debug builds */
-	if (gs_shell_show_detailed_error (shell, error))
-		gs_shell_append_detailed_error (shell, str, error);
-
 	/* show in-app notification */
 	gs_shell_show_event_app_notify (shell, str->str, buttons);
 	return TRUE;
@@ -1532,6 +1513,7 @@ gs_shell_show_event_upgrade (GsShell *shell, GsPluginEvent *event)
 						       "as download failed"),
 						str_app);
 		}
+		gs_shell_append_detailed_error (shell, str, error);
 		break;
 	case GS_PLUGIN_ERROR_NO_NETWORK:
 		/* TRANSLATORS: failure text for the in-app notification,
@@ -1585,6 +1567,7 @@ gs_shell_show_event_upgrade (GsShell *shell, GsPluginEvent *event)
 		/* TRANSLATORS: failure text for the in-app notification,
 		 * where the %s is the distro name (e.g. "Fedora 25") */
 		g_string_append_printf (str, _("Unable to upgrade to %s"), str_app);
+		gs_shell_append_detailed_error (shell, str, error);
 		break;
 	}
 	if (str->len == 0)
@@ -1600,10 +1583,6 @@ gs_shell_show_event_upgrade (GsShell *shell, GsPluginEvent *event)
 		}
 	}
 
-	/* add extra debugging for debug builds */
-	if (gs_shell_show_detailed_error (shell, error))
-		gs_shell_append_detailed_error (shell, str, error);
-
 	/* show in-app notification */
 	gs_shell_show_event_app_notify (shell, str->str, buttons);
 	return TRUE;
@@ -1658,6 +1637,7 @@ gs_shell_show_event_remove (GsShell *shell, GsPluginEvent *event)
 		/* TRANSLATORS: failure text for the in-app notification,
 		 * where the %s is the application name (e.g. "GIMP") */
 		g_string_append_printf (str, _("Unable to remove %s"), str_app);
+		gs_shell_append_detailed_error (shell, str, error);
 		break;
 	}
 	if (str->len == 0)
@@ -1673,10 +1653,6 @@ gs_shell_show_event_remove (GsShell *shell, GsPluginEvent *event)
 		}
 	}
 
-	/* add extra debugging for debug builds */
-	if (gs_shell_show_detailed_error (shell, error))
-		gs_shell_append_detailed_error (shell, str, error);
-
 	/* show in-app notification */
 	gs_shell_show_event_app_notify (shell, str->str, buttons);
 	return TRUE;
@@ -1722,6 +1698,7 @@ gs_shell_show_event_launch (GsShell *shell, GsPluginEvent *event)
 			return FALSE;
 		/* TRANSLATORS: we failed to get a proper error code */
 		g_string_append (str, _("Sorry, something went wrong"));
+		gs_shell_append_detailed_error (shell, str, error);
 		break;
 	}
 	if (str->len == 0)
@@ -1737,10 +1714,6 @@ gs_shell_show_event_launch (GsShell *shell, GsPluginEvent *event)
 		}
 	}
 
-	/* add extra debugging for debug builds */
-	if (gs_shell_show_detailed_error (shell, error))
-		gs_shell_append_detailed_error (shell, str, error);
-
 	/* show in-app notification */
 	gs_shell_show_event_app_notify (shell, str->str, buttons);
 	return TRUE;
@@ -1776,15 +1749,12 @@ gs_shell_show_event_file_to_app (GsShell *shell, GsPluginEvent *event)
 			return FALSE;
 		/* TRANSLATORS: we failed to get a proper error code */
 		g_string_append (str, _("Sorry, something went wrong"));
+		gs_shell_append_detailed_error (shell, str, error);
 		break;
 	}
 	if (str->len == 0)
 		return FALSE;
 
-	/* add extra debugging for debug builds */
-	if (gs_shell_show_detailed_error (shell, error))
-		gs_shell_append_detailed_error (shell, str, error);
-
 	/* show in-app notification */
 	gs_shell_show_event_app_notify (shell, str->str, buttons);
 	return TRUE;
@@ -1820,15 +1790,12 @@ gs_shell_show_event_url_to_app (GsShell *shell, GsPluginEvent *event)
 			return FALSE;
 		/* TRANSLATORS: we failed to get a proper error code */
 		g_string_append (str, _("Sorry, something went wrong"));
+		gs_shell_append_detailed_error (shell, str, error);
 		break;
 	}
 	if (str->len == 0)
 		return FALSE;
 
-	/* add extra debugging for debug builds */
-	if (gs_shell_show_detailed_error (shell, error))
-		g_string_append_printf (str, "\n%s", error->message);
-
 	/* show in-app notification */
 	gs_shell_show_event_app_notify (shell, str->str, buttons);
 	return TRUE;
@@ -1890,6 +1857,7 @@ gs_shell_show_event_fallback (GsShell *shell, GsPluginEvent *event)
 			return FALSE;
 		/* TRANSLATORS: we failed to get a proper error code */
 		g_string_append (str, _("Sorry, something went wrong"));
+		gs_shell_append_detailed_error (shell, str, error);
 		break;
 	}
 	if (str->len == 0)
@@ -1905,10 +1873,6 @@ gs_shell_show_event_fallback (GsShell *shell, GsPluginEvent *event)
 		}
 	}
 
-	/* add extra debugging for debug builds */
-	if (gs_shell_show_detailed_error (shell, error))
-		gs_shell_append_detailed_error (shell, str, error);
-
 	/* show in-app notification */
 	gs_shell_show_event_app_notify (shell, str->str, buttons);
 	return TRUE;
-- 
2.21.0

