[pve-devel] [PATCH widget-toolkit] ui: repo status: Simplify logic for adding warnings

[pve-devel] [PATCH widget-toolkit] ui: repo status: Simplify logic for adding warnings

[Maximiliano Sandoval]

The strings were hard to translate and required some tricks like passing
'Ceph ' as an argument. The `/main` part was also removed to simplify
the flow and UX.

Suggested-by: Dietmar Maurer <dietmar@proxmox.com>
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/node/APTRepositories.js | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
index 4e74da2..9dfd588 100644
--- a/src/node/APTRepositories.js
+++ b/src/node/APTRepositories.js
@@ -489,33 +489,31 @@ Ext.define('Proxmox.node.APTRepositories', {
 		addWarn(gettext('Detected mixed suites before upgrade'));
 	    }
 
-	    let productionReadyCheck = (repos, type, noSubAlternateName) => {
+	    let productionReadyCheck = (repos, name) => {
 		if (!activeSubscription && repos.enterprise) {
 		    addWarn(Ext.String.format(
-			gettext('The {0}enterprise repository is enabled, but there is no active subscription!'),
-			type,
+			gettext('The {0} enterprise repository is enabled, but there is no active subscription!'),
+			name,
 		    ));
 		}
 
 		if (repos.nosubscription) {
 		    addWarn(Ext.String.format(
-			gettext('The {0}no-subscription{1} repository is not recommended for production use!'),
-			type,
-			noSubAlternateName,
+			gettext('The {0} no-subscription repository is not recommended for production use!'),
+			name,
 		    ));
 		}
 
 		if (repos.test) {
 		    addWarn(Ext.String.format(
-			gettext('The {0}test repository may pull in unstable updates and is not recommended for production use!'),
-			type,
+			gettext('The {0} test repository may pull in unstable updates and is not recommended for production use!'),
+			name,
 		    ));
 		}
 	    };
 
-	    productionReadyCheck({ enterprise, nosubscription, test }, '', '');
-	    // TODO drop alternate 'main' name when no longer relevant
-	    productionReadyCheck(cephRepos, 'Ceph ', '/main');
+	    productionReadyCheck({ enterprise, nosubscription, test }, vm.get('product'));
+	    productionReadyCheck(cephRepos, 'Ceph');
 
 	    if (errors.length > 0) {
 		text = gettext('Fatal parsing error for at least one repository');
-- 
2.39.2

Re: [pve-devel] [PATCH widget-toolkit] ui: repo status: Simplify logic for adding warnings

[Fiona Ebner]

Am 07.12.23 um 14:11 schrieb Maximiliano Sandoval:
> The strings were hard to translate and required some tricks like passing
> 'Ceph ' as an argument. The `/main` part was also removed to simplify
> the flow and UX.
> 

But people who have a Ceph repository with 'main' component configured
might be confused when getting a warning that only mentions
'no-subscription'.

It's only relevant for Ceph Quincy and it turns out that the warnings
are currently only displayed for Ceph Reef repositories, because the UI
doesn't expect multiple Ceph versions. So I'd be fine with dropping the
'/main' after all.

Could you take a stab at fixing handling for multiple Ceph versions in a
follow-up? I.e. fix updateStandardRepos() to detect which version is
actually configured and only consider the statuses from those, as well
as displaying a warning if there are mixed Ceph versions configured.

> Suggested-by: Dietmar Maurer <dietmar@proxmox.com>
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>