bamaer commented on code in PR #8270: URL: https://github.com/apache/hop/pull/8270#discussion_r3943976156
########## ui/src/main/java/org/apache/hop/ui/hopgui/notifications/providers/NotificationHttp.java: ########## @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hop.ui.hopgui.notifications.providers; + +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hop.core.util.HttpClientManager; +import org.apache.hop.core.variables.Variables; + +/** Shared HTTP setup for the notification providers. */ +final class NotificationHttp { + + /** Give up if the remote host has not accepted the connection by then. */ + static final int CONNECT_TIMEOUT_MS = 10000; + + /** Give up if the remote host has not answered by then. */ + static final int RESPONSE_TIMEOUT_MS = 20000; + + private NotificationHttp() { + // Utility class + } + + /** + * A client with timeouts. {@link HttpClientManager#createDefaultClient()} sets none at all, and + * HttpClient 5 waits indefinitely for a response, so a single unresponsive feed would otherwise + * hold a polling thread forever. + * + * <p>The client is built on the process-wide shared connection manager, so do not close it: + * closing the response is what returns this request's connection to the pool. + * + * @return A client configured for polling a notification source + */ + static CloseableHttpClient newClient() { + return newClient(null, null); + } + + /** + * A client with timeouts, authenticating when credentials are given. + * + * @param username The user name, may be null or empty for anonymous access + * @param password The password or token, may be null or empty for anonymous access + * @return A client configured for polling a notification source + */ + static CloseableHttpClient newClient(String username, String password) { + HttpClientManager.HttpClientBuilderFacade builder = + HttpClientManager.getInstance() + .createBuilder() + .setConnectionTimeout(CONNECT_TIMEOUT_MS) + .setSocketTimeout(RESPONSE_TIMEOUT_MS); + String resolvedPassword = resolve(password); + if (resolvedPassword != null && !resolvedPassword.isEmpty()) { + // A token is often all a source wants; GitHub, for one, ignores the user name entirely. + builder.setCredentials(resolve(username), resolvedPassword); Review Comment: The `AuthScope` point is right and it's fixed. Credentials are scoped to the request host using the 3-arg `setCredentials` and `createHttpHost(URI)` that already existed on `HttpClientManager` — the 2-arg convenience overload was simply the wrong one to have called. GitHub now skips Basic entirely and sends `Authorization: Bearer`, which is what PATs want; confirmed against the live API that an invalid token under `Bearer` returns 401 "Bad credentials", i.e. the scheme is parsed and the token rejected. Your reachability argument doesn't hold as written, but it does hold against this branch. The old dialog stored `wUsername.getText().trim()`, so an empty field was `""`, not null, and `BasicUserPrincipal("")` is fine — the NPE only triggered on a hand-edited `hop-config.json` with no `auth.username` key. The dialog rewrite in this branch changes that: `NotificationSourceModel.toConfig` writes `trimmed(username)`, which is null for a field nobody touched, so the path becomes reachable from the UI. Coerced either way. One detail: it's an `IllegalArgumentException` at client construction rather than an NPE on fetch. ########## ui/src/main/java/org/apache/hop/ui/hopgui/notifications/NotificationPanel.java: ########## @@ -0,0 +1,1002 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hop.ui.hopgui.notifications; + +import java.text.SimpleDateFormat; +import java.util.Date; +import java.util.List; +import org.apache.hop.core.logging.LogChannel; +import org.apache.hop.core.notifications.Notification; +import org.apache.hop.i18n.BaseMessages; +import org.apache.hop.ui.core.PropsUi; +import org.apache.hop.ui.core.gui.GuiResource; +import org.apache.hop.ui.hopgui.HopGui; +import org.apache.hop.ui.hopgui.ISingletonProvider; +import org.apache.hop.ui.hopgui.ImplementationLoader; +import org.apache.hop.ui.hopgui.perspective.configuration.ConfigurationPerspective; +import org.apache.hop.ui.util.EnvironmentUtils; +import org.eclipse.swt.SWT; +import org.eclipse.swt.SWTException; +import org.eclipse.swt.custom.CLabel; +import org.eclipse.swt.custom.ScrolledComposite; +import org.eclipse.swt.events.SelectionAdapter; +import org.eclipse.swt.events.SelectionEvent; +import org.eclipse.swt.graphics.Color; +import org.eclipse.swt.graphics.Point; +import org.eclipse.swt.layout.FormAttachment; +import org.eclipse.swt.layout.FormData; +import org.eclipse.swt.layout.FormLayout; +import org.eclipse.swt.widgets.Button; +import org.eclipse.swt.widgets.Canvas; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Label; +import org.eclipse.swt.widgets.Listener; +import org.eclipse.swt.widgets.Shell; + +/** Dropdown panel for displaying notifications */ +public class NotificationPanel implements INotificationListener { + private static final Class<?> PKG = NotificationPanel.class; + + /** How many notifications the panel draws before it stops and says how many are left. */ + private static final int MAX_RENDERED_NOTIFICATIONS = 100; + + private static NotificationPanel fallback; + + private static final ISingletonProvider PROVIDER = loadProvider(); + + private static ISingletonProvider loadProvider() { + try { + return (ISingletonProvider) ImplementationLoader.newInstance(NotificationPanel.class); + } catch (Throwable e) { + // hop-ui unit tests have no rcp/rap *Impl on the classpath. Anywhere else this is a + // misconfiguration worth shouting about: one instance would then be shared by every Hop Web + // session, which is the very thing the per-session provider exists to prevent. + LogChannel.GENERAL.logBasic( + "No NotificationPanelImpl found; falling back to a single instance for this process. " + + "In Hop Web that means every session shares one."); + return () -> { + synchronized (NotificationPanel.class) { + if (fallback == null) { + fallback = new NotificationPanel(); + } + return fallback; + } + }; + } + } + + private Shell shell; + private Shell parentShell; + private ScrolledComposite scrolledComposite; + private Composite contentComposite; + private boolean isVisible = false; + + /** The configured sources, refreshed each time the list is drawn, for the source colours. */ + private java.util.List<org.apache.hop.ui.hopgui.notifications.config.NotificationSourceConfig> + sourcesForRender = new java.util.ArrayList<>(); + + /** Use {@link #getInstance()}. Public so RWT can create one per user session in Hop Web. */ + public NotificationPanel() { + this.parentShell = HopGui.getInstance().getShell(); + NotificationService.getInstance().addNotificationListener(this); + } + + /** + * @return The notification panel of this process, or of this user's session in Hop Web + */ + public static NotificationPanel getInstance() { + return (NotificationPanel) PROVIDER.getInstanceInternal(); + } + + /** Toggle the panel visibility */ + public void toggle() { + if (isVisible) { + hide(); + } else { + show(); + } + } + + /** Show the notification panel */ + public void show() { + if (shell != null && !shell.isDisposed()) { + // Panel already exists, refresh notifications and show + updateNotifications(); + shell.setVisible(true); + shell.setFocus(); + // Without this the panel stays "not visible" after the first open and close, which stops + // notificationsChanged() from refreshing it and leaves the bell unable to close it again. + isVisible = true; + return; + } + + createPanel(); + updateNotifications(); + positionPanel(); + shell.setVisible(true); + isVisible = true; + } + + /** Hide the notification panel */ + public void hide() { + if (shell != null && !shell.isDisposed()) { + shell.setVisible(false); + } + isVisible = false; + } + + /** Create the panel UI */ + private void createPanel() { + // Use DIALOG_TRIM instead of ON_TOP to keep it attached to parent + // Remove ON_TOP so it doesn't stay on top when switching applications + shell = new Shell(parentShell, SWT.DIALOG_TRIM | SWT.RESIZE); + shell.setLayout(new FormLayout()); + PropsUi.setLook(shell); + + // Header + Composite header = new Composite(shell, SWT.NONE); + header.setLayout(new FormLayout()); + PropsUi.setLook(header); + FormData fdHeader = new FormData(); + fdHeader.left = new FormAttachment(0, 0); + fdHeader.right = new FormAttachment(100, 0); + fdHeader.top = new FormAttachment(0, 0); + header.setLayoutData(fdHeader); + + // Settings button + Button settingsButton = new Button(header, SWT.PUSH); + settingsButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Settings")); + settingsButton.setToolTipText( + BaseMessages.getString(PKG, "NotificationPanel.Settings.Tooltip")); + PropsUi.setLook(settingsButton); + FormData fdSettings = new FormData(); + fdSettings.right = new FormAttachment(100, -10); + fdSettings.top = new FormAttachment(0, 5); + fdSettings.bottom = new FormAttachment(100, -5); + settingsButton.setLayoutData(fdSettings); + settingsButton.addSelectionListener( + new SelectionAdapter() { + @Override + public void widgetSelected(SelectionEvent e) { + org.apache.hop.ui.hopgui.perspective.configuration.ConfigurationPerspective + configPerspective = HopGui.getConfigurationPerspective(); + if (configPerspective != null) { + HopGui.getInstance().setActivePerspective(configPerspective); + + // Defer tab/tree selection until perspective is fully activated + Display.getCurrent() + .asyncExec( + () -> { + ConfigurationPerspective perspective = HopGui.getConfigurationPerspective(); + if (perspective != null) { + perspective.showNotificationsTab(); + } + }); + } + } + }); + + Button clearAll = new Button(header, SWT.PUSH); + clearAll.setText(BaseMessages.getString(PKG, "NotificationPanel.ClearAll")); + clearAll.setToolTipText(BaseMessages.getString(PKG, "NotificationPanel.ClearAll.Tooltip")); + PropsUi.setLook(clearAll); + FormData fdClearAll = new FormData(); + fdClearAll.right = new FormAttachment(settingsButton, -10); + fdClearAll.top = new FormAttachment(0, 5); + fdClearAll.bottom = new FormAttachment(100, -5); + clearAll.setLayoutData(fdClearAll); + clearAll.addSelectionListener( + new SelectionAdapter() { + @Override + public void widgetSelected(SelectionEvent e) { + NotificationService.getInstance().clearAll(); + updateNotifications(); + } + }); + + Button markAllRead = new Button(header, SWT.PUSH); + markAllRead.setText(BaseMessages.getString(PKG, "NotificationPanel.MarkAllRead")); + PropsUi.setLook(markAllRead); + FormData fdMarkAll = new FormData(); + fdMarkAll.right = new FormAttachment(clearAll, -10); + fdMarkAll.top = new FormAttachment(0, 5); + fdMarkAll.bottom = new FormAttachment(100, -5); + markAllRead.setLayoutData(fdMarkAll); + markAllRead.addSelectionListener( + new SelectionAdapter() { + @Override + public void widgetSelected(SelectionEvent e) { + NotificationService.getInstance().markAllAsRead(); + updateNotifications(); + } + }); + + // Scrolled content area + scrolledComposite = new ScrolledComposite(shell, SWT.V_SCROLL | SWT.BORDER); + PropsUi.setLook(scrolledComposite); + FormData fdScrolled = new FormData(); + fdScrolled.left = new FormAttachment(0, 0); + fdScrolled.right = new FormAttachment(100, 0); + fdScrolled.top = new FormAttachment(header, 0); + fdScrolled.bottom = new FormAttachment(100, -40); + scrolledComposite.setLayoutData(fdScrolled); + + contentComposite = new Composite(scrolledComposite, SWT.NONE); + contentComposite.setLayout(new FormLayout()); + PropsUi.setLook(contentComposite); + scrolledComposite.setContent(contentComposite); + scrolledComposite.setExpandHorizontal(true); + scrolledComposite.setExpandVertical(true); + + // Footer + Composite footer = new Composite(shell, SWT.NONE); + footer.setLayout(new FormLayout()); + PropsUi.setLook(footer); + FormData fdFooter = new FormData(); + fdFooter.left = new FormAttachment(0, 0); + fdFooter.right = new FormAttachment(100, 0); + fdFooter.bottom = new FormAttachment(100, 0); + footer.setLayoutData(fdFooter); + + Button closeButton = new Button(footer, SWT.PUSH); + closeButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Close")); + PropsUi.setLook(closeButton); + FormData fdClose = new FormData(); + fdClose.right = new FormAttachment(100, -10); + fdClose.top = new FormAttachment(0, 5); + fdClose.bottom = new FormAttachment(100, -5); + closeButton.setLayoutData(fdClose); + closeButton.addSelectionListener( + new SelectionAdapter() { + @Override + public void widgetSelected(SelectionEvent e) { + hide(); + } + }); + + // Close when clicking outside + shell.addListener( + SWT.Deactivate, + e -> { + if (!shell.isDisposed()) { + Display.getCurrent() + .asyncExec( + () -> { + if (!shell.isDisposed() && !shell.isFocusControl()) { + hide(); + } + }); + } + }); + + // The panel hangs off the bell in the main toolbar, so it has to follow the main window + // whenever that moves or is resized, not just when it is resized. + if (parentShell != null && !parentShell.isDisposed()) { + Listener repositionListener = + e -> { + if (shell != null && !shell.isDisposed() && isVisible) { + Display.getCurrent() + .asyncExec( + () -> { + if (shell != null && !shell.isDisposed() && isVisible) { + positionPanel(); + } + }); + } + }; + parentShell.addListener(SWT.Resize, repositionListener); + parentShell.addListener(SWT.Move, repositionListener); + } + + shell.setSize(400, 500); + + // Add listener to shell resize to ensure titles truncate properly + shell.addListener( + SWT.Resize, + e -> { + if (contentComposite != null && !contentComposite.isDisposed()) { + // Force layout update to ensure titles truncate correctly + contentComposite.layout(true, true); + } + }); + } + + /** Update the notifications display */ + private void updateNotifications() { + if (contentComposite == null || contentComposite.isDisposed()) { + return; + } + + // Clear existing notifications + for (Control control : contentComposite.getChildren()) { + control.dispose(); + } + + // Get configuration options + org.apache.hop.core.config.HopConfig hopConfig = + org.apache.hop.core.config.HopConfig.getInstance(); + boolean showReadNotifications = + org.apache.hop.core.config.HopConfig.readOptionString( + "notification.showReadNotifications", "true") + .equalsIgnoreCase("true"); + String daysToGoBackStr = + org.apache.hop.core.config.HopConfig.readOptionString( + "notification.global.daysToGoBack", "30"); + int daysToGoBack = 0; + try { + daysToGoBack = Integer.parseInt(daysToGoBackStr); + } catch (NumberFormatException e) { + daysToGoBack = 30; // Default to 30 days + } + + // Get provider errors and notifications + List<org.apache.hop.ui.hopgui.notifications.ProviderErrorInfo> providerErrors = + NotificationService.getInstance().getProviderErrors(); + List<Notification> notifications = + NotificationService.getInstance().getNotifications(!showReadNotifications, daysToGoBack); Review Comment: Confirmed and wired through. `NotificationService` builds a source id → window map when the configuration loads and resolves per notification, with 0 and blank meaning "use the global setting", as the dialog label promises. Read once per session rather than per refresh, since drawing the panel asks for the list once per notification on screen. Four tests in `NotificationServiceTest`. ########## ui/src/main/java/org/apache/hop/ui/hopgui/notifications/NotificationProviderPlugins.java: ########## @@ -0,0 +1,171 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hop.ui.hopgui.notifications; + +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import org.apache.hop.core.logging.ILogChannel; +import org.apache.hop.core.notifications.INotificationProvider; +import org.apache.hop.core.notifications.NotificationProviderPluginType; +import org.apache.hop.core.plugins.IPlugin; +import org.apache.hop.core.plugins.PluginRegistry; +import org.apache.hop.ui.hopgui.notifications.config.NotificationSourceConfig; + +/** + * The notification providers contributed by plugins, as found in the plugin registry. + * + * <p>A plugin declares a provider with {@link + * org.apache.hop.core.notifications.NotificationProviderPlugin} and is discovered from its jar. It + * therefore needs no entry in the configuration to work: a stored source only records what the user + * changed about it, and a provider whose plugin has been uninstalled simply stops being found. + */ +public final class NotificationProviderPlugins { + + private NotificationProviderPlugins() { + // Utility class + } + + /** + * @return The registry entries of every declared notification provider + */ + public static List<IPlugin> plugins() { + List<IPlugin> plugins = + PluginRegistry.getInstance().getPlugins(NotificationProviderPluginType.class); + return plugins == null ? new ArrayList<>() : plugins; + } + + /** + * @return The identifiers of every declared notification provider + */ + public static Set<String> ids() { + Set<String> ids = new LinkedHashSet<>(); + for (IPlugin plugin : plugins()) { + String id = idOf(plugin); + if (id != null) { + ids.add(id); + } + } + return ids; + } + + /** + * Instantiate a declared provider. + * + * @param plugin The registry entry + * @param log Where to report a provider that cannot be loaded + * @return The provider, or null when its class could not be loaded + */ + public static INotificationProvider load(IPlugin plugin, ILogChannel log) { + try { + return PluginRegistry.getInstance().loadClass(plugin, INotificationProvider.class); + } catch (Exception e) { + log.logError("Unable to load the notification provider of plugin " + plugin.getName(), e); + return null; + } + } + + /** + * The identifier a plugin's provider is known by. This is the source id its notifications are + * qualified with, so it has to be the plugin id and nothing derived from the instance. + * + * @param plugin The registry entry + * @return The identifier, or null when the entry declares none + */ + public static String idOf(IPlugin plugin) { + String[] ids = plugin.getIds(); + return ids == null || ids.length == 0 ? null : ids[0]; + } + + /** + * Describe the declared providers as configuration sources, so the Notifications settings can + * list a plugin that has never been configured alongside the sources the user added. + * + * @return One source per declared provider, in registry order + */ + public static List<NotificationSourceConfig> describeAsSources() { + List<NotificationSourceConfig> described = new ArrayList<>(); + for (IPlugin plugin : plugins()) { + String id = idOf(plugin); + if (id == null) { + continue; + } + NotificationSourceConfig source = new NotificationSourceConfig(); + source.setId(id); + source.setPluginId(id); + source.setName( + plugin.getName() == null || plugin.getName().isEmpty() ? id : plugin.getName()); + source.setType(NotificationSourceConfig.SourceType.CUSTOM_PLUGIN); + source.setEnabled(true); + source.setPollIntervalMinutes("60"); Review Comment: Fixed as suggested — `describeAsSources()` no longer writes an interval, so `applyPollInterval`'s empty check leaves the provider's own default alone and the marketplace one keeps its six hours. The settings table now shows the running provider's actual interval instead of falling back to the global, which was misleading. Pulling on this surfaced something larger: `loadSources()` calls `addDiscovered()` and *any* save writes the whole list, so pressing Save baked every discovered plugin into `hop-config.json` permanently. That contradicts the design stated in `NotificationProviderPlugins`' own javadoc — "a provider whose plugin has been uninstalled simply stops being found" — because once persisted it doesn't. Added `toPersist()`, which drops discovered rows still identical to their synthesized form. ########## ui/src/main/java/org/apache/hop/ui/hopgui/notifications/config/NotificationSourceDialog.java: ########## @@ -0,0 +1,976 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hop.ui.hopgui.notifications.config; + +import java.util.UUID; +import org.apache.hop.core.util.Utils; +import org.apache.hop.i18n.BaseMessages; +import org.apache.hop.ui.core.PropsUi; +import org.apache.hop.ui.core.dialog.ErrorDialog; +import org.apache.hop.ui.core.widget.PasswordTextVar; +import org.apache.hop.ui.core.widget.TextVar; +import org.apache.hop.ui.hopgui.HopGui; +import org.apache.hop.ui.pipeline.transform.BaseTransformDialog; +import org.eclipse.swt.SWT; +import org.eclipse.swt.events.SelectionAdapter; +import org.eclipse.swt.events.SelectionEvent; +import org.eclipse.swt.layout.FormAttachment; +import org.eclipse.swt.layout.FormData; +import org.eclipse.swt.layout.FormLayout; +import org.eclipse.swt.widgets.Button; +import org.eclipse.swt.widgets.ColorDialog; +import org.eclipse.swt.widgets.Combo; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Label; +import org.eclipse.swt.widgets.Shell; + +/** Dialog for adding or editing a notification source configuration. */ +public class NotificationSourceDialog { + + private static final Class<?> PKG = NotificationSourceDialog.class; + + private Shell shell; + private Shell parentShell; + private NotificationSourceConfig sourceConfig; + + /** + * Whether the dialog was closed without confirming. Only OK clears it: closing the window any + * other way - the title bar, Escape - leaves the edits unsaved, which is what closing a dialog + * means everywhere else. + */ + private boolean cancelled = true; + + private PropsUi props = PropsUi.getInstance(); + + // UI widgets + private TextVar wName; + private Combo wType; + private Button wEnabled; + private Button wColorButton; + private Label wColorPreview; + private Composite wTypeSpecificComposite; + private TextVar wGithubUrl; // For URL input + private TextVar wGithubOwner; + private TextVar wGithubRepo; + private Button wGithubIncludePrereleases; + private TextVar wRssUrl; + private TextVar wPluginId; + private TextVar wPollInterval; + private TextVar wDaysToGoBack; + private TextVar wUsername; + private PasswordTextVar wPassword; + private TextVar wMinimumVersion; + + /** Set while one GitHub field is updating another, so the two directions do not loop. */ + private boolean syncingGithubFields; + + public NotificationSourceDialog(Shell parent, NotificationSourceConfig sourceConfig) { + this.parentShell = parent; + this.shell = new Shell(parent, SWT.DIALOG_TRIM | SWT.RESIZE | SWT.MAX | SWT.MIN); + this.sourceConfig = sourceConfig != null ? sourceConfig : new NotificationSourceConfig(); + props.setLook(this.shell); + } + + public String open() { + Display display = parentShell.getDisplay(); + + shell.setText(BaseMessages.getString(PKG, "NotificationSourceDialog.Title")); + + FormLayout formLayout = new FormLayout(); Review Comment: Done. `NotificationSourceDialog` is 976 → 372 lines with no `FormAttachment` left in it. The form moved to a new `NotificationSourceModel` carrying 16 `@GuiWidgetElement` fields, built through `GuiCompositeWidgets.addScrolledComposite`, so the form scrolls and the button bar stays pinned. The colour picker and URL parser are annotated `BUTTON` methods — the framework passes the model in and re-binds afterwards, so no manual refresh plumbing. One deviation: type-specific fields aren't extra groups. Groups render as tabs (`GuiWidgetGroups` logs "not implemented yet" for LIST/BOXES), which would put a GitHub, an RSS and a Plugin tab on every source regardless of type. Instead every field is declared once and `setWidgetsHidden` collapses the irrelevant rows when the type changes — same visible behaviour as before, without the dispose-and-rebuild. Happy to switch to tabs if you'd prefer. Two behaviour fixes fell out: saving a GitHub source now accepts a URL alone instead of erroring, and `toConfig` writes only the chosen type's fields, so switching a source to RSS and back no longer leaves a stale owner/repo behind. Tested on the desktop and in Hop Web. ########## ui/src/main/java/org/apache/hop/ui/hopgui/notifications/NotificationPanel.java: ########## @@ -0,0 +1,1002 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hop.ui.hopgui.notifications; + +import java.text.SimpleDateFormat; +import java.util.Date; +import java.util.List; +import org.apache.hop.core.logging.LogChannel; +import org.apache.hop.core.notifications.Notification; +import org.apache.hop.i18n.BaseMessages; +import org.apache.hop.ui.core.PropsUi; +import org.apache.hop.ui.core.gui.GuiResource; +import org.apache.hop.ui.hopgui.HopGui; +import org.apache.hop.ui.hopgui.ISingletonProvider; +import org.apache.hop.ui.hopgui.ImplementationLoader; +import org.apache.hop.ui.hopgui.perspective.configuration.ConfigurationPerspective; +import org.apache.hop.ui.util.EnvironmentUtils; +import org.eclipse.swt.SWT; +import org.eclipse.swt.SWTException; +import org.eclipse.swt.custom.CLabel; +import org.eclipse.swt.custom.ScrolledComposite; +import org.eclipse.swt.events.SelectionAdapter; +import org.eclipse.swt.events.SelectionEvent; +import org.eclipse.swt.graphics.Color; +import org.eclipse.swt.graphics.Point; +import org.eclipse.swt.layout.FormAttachment; +import org.eclipse.swt.layout.FormData; +import org.eclipse.swt.layout.FormLayout; +import org.eclipse.swt.widgets.Button; +import org.eclipse.swt.widgets.Canvas; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Label; +import org.eclipse.swt.widgets.Listener; +import org.eclipse.swt.widgets.Shell; + +/** Dropdown panel for displaying notifications */ +public class NotificationPanel implements INotificationListener { + private static final Class<?> PKG = NotificationPanel.class; + + /** How many notifications the panel draws before it stops and says how many are left. */ + private static final int MAX_RENDERED_NOTIFICATIONS = 100; + + private static NotificationPanel fallback; + + private static final ISingletonProvider PROVIDER = loadProvider(); + + private static ISingletonProvider loadProvider() { + try { + return (ISingletonProvider) ImplementationLoader.newInstance(NotificationPanel.class); + } catch (Throwable e) { + // hop-ui unit tests have no rcp/rap *Impl on the classpath. Anywhere else this is a + // misconfiguration worth shouting about: one instance would then be shared by every Hop Web + // session, which is the very thing the per-session provider exists to prevent. + LogChannel.GENERAL.logBasic( + "No NotificationPanelImpl found; falling back to a single instance for this process. " + + "In Hop Web that means every session shares one."); + return () -> { + synchronized (NotificationPanel.class) { + if (fallback == null) { + fallback = new NotificationPanel(); + } + return fallback; + } + }; + } + } + + private Shell shell; + private Shell parentShell; + private ScrolledComposite scrolledComposite; + private Composite contentComposite; + private boolean isVisible = false; + + /** The configured sources, refreshed each time the list is drawn, for the source colours. */ + private java.util.List<org.apache.hop.ui.hopgui.notifications.config.NotificationSourceConfig> + sourcesForRender = new java.util.ArrayList<>(); + + /** Use {@link #getInstance()}. Public so RWT can create one per user session in Hop Web. */ + public NotificationPanel() { + this.parentShell = HopGui.getInstance().getShell(); + NotificationService.getInstance().addNotificationListener(this); + } + + /** + * @return The notification panel of this process, or of this user's session in Hop Web + */ + public static NotificationPanel getInstance() { + return (NotificationPanel) PROVIDER.getInstanceInternal(); + } + + /** Toggle the panel visibility */ + public void toggle() { + if (isVisible) { + hide(); + } else { + show(); + } + } + + /** Show the notification panel */ + public void show() { + if (shell != null && !shell.isDisposed()) { + // Panel already exists, refresh notifications and show + updateNotifications(); + shell.setVisible(true); + shell.setFocus(); + // Without this the panel stays "not visible" after the first open and close, which stops + // notificationsChanged() from refreshing it and leaves the bell unable to close it again. + isVisible = true; + return; + } + + createPanel(); + updateNotifications(); + positionPanel(); + shell.setVisible(true); + isVisible = true; + } + + /** Hide the notification panel */ + public void hide() { + if (shell != null && !shell.isDisposed()) { + shell.setVisible(false); + } + isVisible = false; + } + + /** Create the panel UI */ + private void createPanel() { + // Use DIALOG_TRIM instead of ON_TOP to keep it attached to parent + // Remove ON_TOP so it doesn't stay on top when switching applications + shell = new Shell(parentShell, SWT.DIALOG_TRIM | SWT.RESIZE); + shell.setLayout(new FormLayout()); + PropsUi.setLook(shell); + + // Header + Composite header = new Composite(shell, SWT.NONE); + header.setLayout(new FormLayout()); + PropsUi.setLook(header); + FormData fdHeader = new FormData(); + fdHeader.left = new FormAttachment(0, 0); + fdHeader.right = new FormAttachment(100, 0); + fdHeader.top = new FormAttachment(0, 0); + header.setLayoutData(fdHeader); + + // Settings button + Button settingsButton = new Button(header, SWT.PUSH); + settingsButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Settings")); + settingsButton.setToolTipText( + BaseMessages.getString(PKG, "NotificationPanel.Settings.Tooltip")); + PropsUi.setLook(settingsButton); + FormData fdSettings = new FormData(); + fdSettings.right = new FormAttachment(100, -10); + fdSettings.top = new FormAttachment(0, 5); + fdSettings.bottom = new FormAttachment(100, -5); + settingsButton.setLayoutData(fdSettings); + settingsButton.addSelectionListener( + new SelectionAdapter() { + @Override + public void widgetSelected(SelectionEvent e) { + org.apache.hop.ui.hopgui.perspective.configuration.ConfigurationPerspective + configPerspective = HopGui.getConfigurationPerspective(); + if (configPerspective != null) { + HopGui.getInstance().setActivePerspective(configPerspective); + + // Defer tab/tree selection until perspective is fully activated + Display.getCurrent() + .asyncExec( + () -> { + ConfigurationPerspective perspective = HopGui.getConfigurationPerspective(); + if (perspective != null) { + perspective.showNotificationsTab(); + } + }); + } + } + }); + + Button clearAll = new Button(header, SWT.PUSH); + clearAll.setText(BaseMessages.getString(PKG, "NotificationPanel.ClearAll")); + clearAll.setToolTipText(BaseMessages.getString(PKG, "NotificationPanel.ClearAll.Tooltip")); + PropsUi.setLook(clearAll); + FormData fdClearAll = new FormData(); + fdClearAll.right = new FormAttachment(settingsButton, -10); + fdClearAll.top = new FormAttachment(0, 5); + fdClearAll.bottom = new FormAttachment(100, -5); + clearAll.setLayoutData(fdClearAll); + clearAll.addSelectionListener( + new SelectionAdapter() { + @Override + public void widgetSelected(SelectionEvent e) { + NotificationService.getInstance().clearAll(); + updateNotifications(); + } + }); + + Button markAllRead = new Button(header, SWT.PUSH); + markAllRead.setText(BaseMessages.getString(PKG, "NotificationPanel.MarkAllRead")); + PropsUi.setLook(markAllRead); + FormData fdMarkAll = new FormData(); + fdMarkAll.right = new FormAttachment(clearAll, -10); + fdMarkAll.top = new FormAttachment(0, 5); + fdMarkAll.bottom = new FormAttachment(100, -5); + markAllRead.setLayoutData(fdMarkAll); + markAllRead.addSelectionListener( + new SelectionAdapter() { + @Override + public void widgetSelected(SelectionEvent e) { + NotificationService.getInstance().markAllAsRead(); + updateNotifications(); + } + }); + + // Scrolled content area + scrolledComposite = new ScrolledComposite(shell, SWT.V_SCROLL | SWT.BORDER); + PropsUi.setLook(scrolledComposite); + FormData fdScrolled = new FormData(); + fdScrolled.left = new FormAttachment(0, 0); + fdScrolled.right = new FormAttachment(100, 0); + fdScrolled.top = new FormAttachment(header, 0); + fdScrolled.bottom = new FormAttachment(100, -40); + scrolledComposite.setLayoutData(fdScrolled); + + contentComposite = new Composite(scrolledComposite, SWT.NONE); + contentComposite.setLayout(new FormLayout()); + PropsUi.setLook(contentComposite); + scrolledComposite.setContent(contentComposite); + scrolledComposite.setExpandHorizontal(true); + scrolledComposite.setExpandVertical(true); + + // Footer + Composite footer = new Composite(shell, SWT.NONE); + footer.setLayout(new FormLayout()); + PropsUi.setLook(footer); + FormData fdFooter = new FormData(); + fdFooter.left = new FormAttachment(0, 0); + fdFooter.right = new FormAttachment(100, 0); + fdFooter.bottom = new FormAttachment(100, 0); + footer.setLayoutData(fdFooter); + + Button closeButton = new Button(footer, SWT.PUSH); + closeButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Close")); + PropsUi.setLook(closeButton); + FormData fdClose = new FormData(); + fdClose.right = new FormAttachment(100, -10); + fdClose.top = new FormAttachment(0, 5); + fdClose.bottom = new FormAttachment(100, -5); + closeButton.setLayoutData(fdClose); + closeButton.addSelectionListener( + new SelectionAdapter() { + @Override + public void widgetSelected(SelectionEvent e) { + hide(); + } + }); + + // Close when clicking outside + shell.addListener( + SWT.Deactivate, + e -> { + if (!shell.isDisposed()) { + Display.getCurrent() + .asyncExec( + () -> { + if (!shell.isDisposed() && !shell.isFocusControl()) { + hide(); + } + }); + } + }); + + // The panel hangs off the bell in the main toolbar, so it has to follow the main window + // whenever that moves or is resized, not just when it is resized. + if (parentShell != null && !parentShell.isDisposed()) { + Listener repositionListener = + e -> { + if (shell != null && !shell.isDisposed() && isVisible) { + Display.getCurrent() + .asyncExec( + () -> { + if (shell != null && !shell.isDisposed() && isVisible) { + positionPanel(); + } + }); + } + }; + parentShell.addListener(SWT.Resize, repositionListener); + parentShell.addListener(SWT.Move, repositionListener); + } + + shell.setSize(400, 500); + + // Add listener to shell resize to ensure titles truncate properly + shell.addListener( + SWT.Resize, + e -> { + if (contentComposite != null && !contentComposite.isDisposed()) { + // Force layout update to ensure titles truncate correctly + contentComposite.layout(true, true); + } + }); + } + + /** Update the notifications display */ + private void updateNotifications() { + if (contentComposite == null || contentComposite.isDisposed()) { + return; + } + + // Clear existing notifications + for (Control control : contentComposite.getChildren()) { + control.dispose(); + } + + // Get configuration options + org.apache.hop.core.config.HopConfig hopConfig = + org.apache.hop.core.config.HopConfig.getInstance(); + boolean showReadNotifications = + org.apache.hop.core.config.HopConfig.readOptionString( + "notification.showReadNotifications", "true") + .equalsIgnoreCase("true"); + String daysToGoBackStr = + org.apache.hop.core.config.HopConfig.readOptionString( + "notification.global.daysToGoBack", "30"); + int daysToGoBack = 0; + try { + daysToGoBack = Integer.parseInt(daysToGoBackStr); + } catch (NumberFormatException e) { + daysToGoBack = 30; // Default to 30 days + } + + // Get provider errors and notifications + List<org.apache.hop.ui.hopgui.notifications.ProviderErrorInfo> providerErrors = + NotificationService.getInstance().getProviderErrors(); + List<Notification> notifications = + NotificationService.getInstance().getNotifications(!showReadNotifications, daysToGoBack); + // Read once per repaint: getSourceColor runs twice for every notification on screen. + sourcesForRender = org.apache.hop.ui.hopgui.notifications.config.NotificationSources.load(); + + Control lastControl = null; + + // Provider error banner + if (!providerErrors.isEmpty()) { + Composite errorBanner = createProviderErrorBanner(providerErrors, lastControl); + lastControl = errorBanner; + } + + if (notifications.isEmpty() && lastControl == null) { + Label emptyLabel = new Label(contentComposite, SWT.CENTER | SWT.WRAP); + emptyLabel.setText(BaseMessages.getString(PKG, "NotificationPanel.NoNotifications")); + PropsUi.setLook(emptyLabel); + FormData fdEmpty = new FormData(); + fdEmpty.left = new FormAttachment(0, 10); + fdEmpty.right = new FormAttachment(100, -10); + fdEmpty.top = new FormAttachment(0, 20); + emptyLabel.setLayoutData(fdEmpty); + } else if (!notifications.isEmpty()) { + // Every notification becomes a small stack of widgets. Beyond a screenful or two nobody + // scrolls anyway, and building hundreds of them is what makes opening the panel feel slow. + int shown = Math.min(notifications.size(), MAX_RENDERED_NOTIFICATIONS); + for (Notification notification : notifications.subList(0, shown)) { + try { + Composite notifComposite = createNotificationItem(notification, lastControl); + lastControl = notifComposite; + } catch (Exception e) { + // Log error but continue with other notifications + LogChannel.UI.logError("Error creating notification item: " + notification.getTitle(), e); + } + } + if (notifications.size() > shown) { + Label moreLabel = new Label(contentComposite, SWT.CENTER | SWT.WRAP); + moreLabel.setText( + BaseMessages.getString( + PKG, + "NotificationPanel.MoreNotifications", + Integer.toString(notifications.size() - shown))); + PropsUi.setLook(moreLabel); + FormData fdMore = new FormData(); + fdMore.left = new FormAttachment(0, 10); + fdMore.right = new FormAttachment(100, -10); + fdMore.top = new FormAttachment(lastControl, 10); + moreLabel.setLayoutData(fdMore); + lastControl = moreLabel; + } + } else if (lastControl != null && notifications.isEmpty()) { + // Errors only, no notifications + Label emptyLabel = new Label(contentComposite, SWT.CENTER | SWT.WRAP); + emptyLabel.setText(BaseMessages.getString(PKG, "NotificationPanel.NoNotifications")); + PropsUi.setLook(emptyLabel); + FormData fdEmpty = new FormData(); + fdEmpty.left = new FormAttachment(0, 10); + fdEmpty.right = new FormAttachment(100, -10); + fdEmpty.top = new FormAttachment(lastControl, 10); + emptyLabel.setLayoutData(fdEmpty); + } + + // Force layout of content composite and scrolled composite + if (contentComposite != null && !contentComposite.isDisposed()) { + // Get the scrolled composite width first to ensure proper sizing + int availableWidth = SWT.DEFAULT; + if (scrolledComposite != null && !scrolledComposite.isDisposed()) { + org.eclipse.swt.graphics.Rectangle scrolledBounds = scrolledComposite.getBounds(); + if (scrolledBounds.width > 0) { + availableWidth = scrolledBounds.width - 20; // Account for margins + } + } + + // Layout scrolled composite first to get its actual width + if (scrolledComposite != null && !scrolledComposite.isDisposed()) { + scrolledComposite.layout(true, false); + org.eclipse.swt.graphics.Rectangle scrolledBounds = scrolledComposite.getBounds(); + if (scrolledBounds.width > 0) { + availableWidth = scrolledBounds.width - 20; // Account for margins + } else { + // Fallback: use shell width if scrolled composite not sized yet + if (shell != null && !shell.isDisposed()) { + availableWidth = shell.getSize().x > 0 ? shell.getSize().x - 40 : 380; + } else { + availableWidth = 380; // Default width + } + } + } + + // Layout content composite with proper width constraint + // This is critical for SWT.WRAP labels to calculate their height + contentComposite.layout(true, true); + + // Compute size with width constraint for proper wrapping + org.eclipse.swt.graphics.Point contentSize = + contentComposite.computeSize(availableWidth, SWT.DEFAULT); + + if (scrolledComposite != null && !scrolledComposite.isDisposed()) { + scrolledComposite.setMinSize(contentSize); + scrolledComposite.layout(true, true); + } + } + } + + /** Create the provider error banner with Retry button */ + private Composite createProviderErrorBanner( + List<org.apache.hop.ui.hopgui.notifications.ProviderErrorInfo> errors, Control above) { + Composite banner = new Composite(contentComposite, SWT.BORDER); + FormLayout bannerLayout = new FormLayout(); + // The error text wraps to several lines; without a bottom margin the last one sits on the + // border, because nothing attaches the final label to the bottom of the banner. + bannerLayout.marginBottom = 10; + banner.setLayout(bannerLayout); + // Look first, then the banner's own colours: setLook applies the theme's foreground, which on + // a dark theme is near white and left this text unreadable on the light background. Both + // colours are set explicitly so the banner reads the same whichever theme is in use. + PropsUi.setLook(banner); + Color bannerBackground = GuiResource.getInstance().getColor(255, 248, 220); // Light yellow + Color bannerForeground = GuiResource.getInstance().getColor(60, 50, 20); // Dark brown + banner.setBackground(bannerBackground); + banner.setForeground(bannerForeground); + + FormData fdBanner = new FormData(); + fdBanner.left = new FormAttachment(0, 0); + fdBanner.right = new FormAttachment(100, 0); + fdBanner.top = above != null ? new FormAttachment(above, 10) : new FormAttachment(0, 10); + banner.setLayoutData(fdBanner); + + Label headerLabel = new Label(banner, SWT.WRAP); + headerLabel.setText(BaseMessages.getString(PKG, "NotificationPanel.ProviderErrors")); + PropsUi.setLook(headerLabel); + headerLabel.setBackground(bannerBackground); + headerLabel.setForeground(bannerForeground); + FormData fdHeader = new FormData(); + fdHeader.left = new FormAttachment(0, 10); + fdHeader.right = new FormAttachment(100, -80); + fdHeader.top = new FormAttachment(0, 10); + headerLabel.setLayoutData(fdHeader); + + Button retryButton = new Button(banner, SWT.PUSH); + retryButton.setText(BaseMessages.getString(PKG, "NotificationPanel.Retry")); + retryButton.setToolTipText(BaseMessages.getString(PKG, "NotificationPanel.Retry.Tooltip")); + PropsUi.setLook(retryButton); + FormData fdRetry = new FormData(); + fdRetry.right = new FormAttachment(100, -10); + fdRetry.top = new FormAttachment(0, 5); + retryButton.setLayoutData(fdRetry); + retryButton.addSelectionListener( + new SelectionAdapter() { + @Override + public void widgetSelected(SelectionEvent e) { + // Fetches in the background; the panel refreshes through notificationsChanged(). + NotificationService.getInstance().retryNow(); + } + }); + + Control lastLine = headerLabel; + for (org.apache.hop.ui.hopgui.notifications.ProviderErrorInfo err : errors) { + String text = + BaseMessages.getString( + PKG, "NotificationPanel.ProviderErrorItem", err.getProviderName(), err.getMessage()); + Label line = new Label(banner, SWT.WRAP); + line.setText(text); + PropsUi.setLook(line); + line.setBackground(bannerBackground); + line.setForeground(bannerForeground); + FormData fdLine = new FormData(); + fdLine.left = new FormAttachment(0, 10); + fdLine.right = new FormAttachment(100, -10); + fdLine.top = new FormAttachment(lastLine, 5); + line.setLayoutData(fdLine); + lastLine = line; + } + + return banner; + } + + /** Create a notification item UI */ + private Composite createNotificationItem(Notification notification, Control above) { + Composite composite = new Composite(contentComposite, SWT.BORDER); + composite.setLayout(new FormLayout()); + PropsUi.setLook(composite); + + GuiResource guiResource = GuiResource.getInstance(); + + // Set background color based on read state and priority + // Note: We'll update this dynamically when notification is marked as read + updateNotificationBackground(composite, notification, guiResource); + + // Set FormData for positioning in parent + FormData fdComposite = new FormData(); + fdComposite.left = new FormAttachment(0, 0); + fdComposite.right = new FormAttachment(100, 0); + if (above != null) { + fdComposite.top = new FormAttachment(above, 5); + } else { + fdComposite.top = new FormAttachment(0, 5); + } + // Don't set bottom - composite will size to its children + composite.setLayoutData(fdComposite); + + // Priority indicator (colored bar on the left) + Composite priorityBar = new Composite(composite, SWT.NONE); + priorityBar.setLayout(null); + priorityBar.setData("type", "priorityBar"); // Mark for later updates + FormData fdPriorityBar = new FormData(); + fdPriorityBar.left = new FormAttachment(0, 0); + fdPriorityBar.top = new FormAttachment(0, 0); + fdPriorityBar.bottom = new FormAttachment(100, 0); + fdPriorityBar.width = 4; + priorityBar.setLayoutData(fdPriorityBar); + + // Set initial priority bar color (will be updated when read state changes) + updatePriorityBar(priorityBar, notification, guiResource); + + // Source color indicator (small colored square) - positioned on the left, after priority bar + // A Canvas, not a plain Composite: the border below is drawn in a paint listener, and RAP + // only offers one on Canvas. On a Composite this compiles against desktop SWT and fails in + // Hop Web with NoSuchMethodError, taking the whole notification list down with it. + Canvas sourceIndicator = new Canvas(composite, SWT.NONE); + sourceIndicator.setLayout(null); + sourceIndicator.setData("type", "sourceIndicator"); // Mark to exclude from click handling + PropsUi.setLook(sourceIndicator); + // Get color for this source from configuration + org.eclipse.swt.graphics.Color sourceColor = getSourceColor(notification, guiResource); + sourceIndicator.setBackground(sourceColor); + FormData fdSourceIndicator = new FormData(); + // Position after priority bar, 8px gap + fdSourceIndicator.left = new FormAttachment(priorityBar, 8); + fdSourceIndicator.top = new FormAttachment(0, 10); + fdSourceIndicator.width = 12; // Fixed width + fdSourceIndicator.height = 12; // Fixed height + sourceIndicator.setLayoutData(fdSourceIndicator); + + // Add a PaintListener to draw a border for better visibility + sourceIndicator.addPaintListener( + e -> { + org.eclipse.swt.graphics.Rectangle bounds = sourceIndicator.getBounds(); + e.gc.setForeground(guiResource.getColorDarkGray()); + e.gc.setLineWidth(1); + e.gc.drawRectangle(0, 0, bounds.width - 1, bounds.height - 1); + }); + + // Tooltip with source name and URL (source name shown in tooltip, not as text) + String tooltipText = buildSourceTooltip(notification); + sourceIndicator.setToolTipText(tooltipText); + + // Title - use CLabel for automatic ellipsis truncation + // Title starts after source indicator + CLabel titleLabel = new CLabel(composite, SWT.LEFT); + String fullTitle = notification.getTitle() != null ? notification.getTitle() : ""; + titleLabel.setText(fullTitle); + titleLabel.setData("type", "title"); // Mark for later updates + titleLabel.setData("fullTitle", fullTitle); // Store full title for tooltip + PropsUi.setLook(titleLabel); + if (!notification.isRead()) { + titleLabel.setFont(guiResource.getFontBold()); + } + // Set tooltip to show full title if truncated + titleLabel.setToolTipText(fullTitle); + FormData fdTitle = new FormData(); + // Title starts after source indicator with 8px gap + fdTitle.left = new FormAttachment(sourceIndicator, 8); + // Title extends to right edge with margin + fdTitle.right = new FormAttachment(100, -10); + fdTitle.top = new FormAttachment(0, 10); + // Don't attach bottom - CLabel will size to its preferred height + titleLabel.setLayoutData(fdTitle); + + // Timestamp between title and body (always visible) + Label timeLabel = new Label(composite, SWT.NONE); + String timeText = formatTimestamp(notification.getTimestamp()); + if (timeText == null || timeText.isEmpty()) { + timeText = "Unknown date"; + } + timeLabel.setText(timeText); + PropsUi.setLook(timeLabel); + timeLabel.setForeground(guiResource.getColorDarkGray()); + FormData fdTime = new FormData(); + fdTime.left = new FormAttachment(priorityBar, 10); + // Timestamp extends to right edge of composite + fdTime.right = new FormAttachment(100, -10); + fdTime.top = new FormAttachment(titleLabel, 5); + // Don't attach bottom - label will size to its preferred height + timeLabel.setLayoutData(fdTime); + + // Message/Description - simplified, truncated to max 3-5 lines + Label messageLabel = null; + String message = notification.getMessage(); + + if (message != null && !message.isEmpty()) { + // Limit to approximately 3-5 lines (roughly 200-300 characters) + // Simple truncation - just show start of message + int maxLength = 250; + String displayMessage = message; + if (displayMessage.length() > maxLength) { + displayMessage = displayMessage.substring(0, maxLength).trim() + "..."; + } + messageLabel = new Label(composite, SWT.WRAP); + messageLabel.setText(displayMessage); + PropsUi.setLook(messageLabel); + FormData fdMessage = new FormData(); + fdMessage.left = new FormAttachment(priorityBar, 10); + // Message extends to right edge of composite (sourceIndicator is on left, so don't constrain + // by it) + fdMessage.right = new FormAttachment(100, -10); + fdMessage.top = new FormAttachment(timeLabel, 5); + // Don't attach bottom - label will wrap and size to its content + messageLabel.setLayoutData(fdMessage); + + // Set default cursor (not clickable) + messageLabel.setCursor( + composite.getDisplay().getSystemCursor(org.eclipse.swt.SWT.CURSOR_ARROW)); + } + + // Force layout of this composite to ensure all children are properly sized + composite.layout(true, true); + + // Set cursor to pointer to indicate clickability for entire notification area + // Store reference to notification ID and guiResource for updates + composite.setData("notificationId", notification.getId()); + composite.setData("guiResource", guiResource); + + // Set cursor behavior: title and composite are clickable, body and timestamp are not + org.eclipse.swt.graphics.Cursor handCursor = + composite.getDisplay().getSystemCursor(org.eclipse.swt.SWT.CURSOR_HAND); + org.eclipse.swt.graphics.Cursor defaultCursor = + composite.getDisplay().getSystemCursor(org.eclipse.swt.SWT.CURSOR_ARROW); + + // Title is clickable - use hand cursor + titleLabel.setCursor(handCursor); + + // Body (message) and timestamp are NOT clickable - use default cursor + if (messageLabel != null) { + messageLabel.setCursor(defaultCursor); + } + timeLabel.setCursor(defaultCursor); + + // Composite itself is clickable (for clicking outside title but still on notification) + composite.setCursor(handCursor); + + // Click handler - attach to composite and all child controls + org.eclipse.swt.widgets.Listener clickListener = + e -> { + // Mark as read first - this updates the notification in the service + NotificationService.getInstance().markAsRead(notification.getId()); + + // Get fresh notification from service to ensure we have updated state + List<Notification> allNotifications = + NotificationService.getInstance().getNotifications(false); + Notification updatedNotification = + allNotifications.stream() + .filter(n -> notification.getId().equals(n.getId())) + .findFirst() + .orElse(notification); + + // Ensure it's marked as read (should already be, but be safe) + updatedNotification.setRead(true); + + // Update visual state immediately + updateNotificationBackground(composite, updatedNotification, guiResource); + + // Force redraw to ensure visual changes are visible + composite.redraw(); + + // Open link if available. NotificationService drops links it will not open, but the + // link is handed to the operating system here, so it is checked again at the click. + String link = updatedNotification.getLink(); + if (link != null && !link.isEmpty()) { + if (NotificationLinks.isSafe(link)) { + try { + EnvironmentUtils.getInstance().openUrl(link); + } catch (Exception ex) { + LogChannel.UI.logError("Error opening notification link " + link, ex); + } + } else { + LogChannel.UI.logBasic( + "Refusing to open notification link " + + link + + ": only http and https are opened"); + } + } + }; + + // Attach click handler to composite + composite.addListener(SWT.MouseDown, clickListener); + + // Also attach to all child controls to make entire area clickable + attachClickListenerRecursive(composite, clickListener); + + return composite; + } + + /** Update notification background based on read state and priority */ + private void updateNotificationBackground( + Composite composite, Notification notification, GuiResource guiResource) { + if (notification.isRead()) { + // Read notifications have default background + composite.setBackground(null); + } else { + // Unread notifications have colored background based on priority + if (notification.getPriority() != null) { + switch (notification.getPriority()) { + case ERROR: + composite.setBackground(guiResource.getColor(255, 240, 240)); // Light red tint + break; + case WARNING: + composite.setBackground(guiResource.getColor(255, 250, 240)); // Light yellow tint + break; + case INFO: + default: + composite.setBackground(guiResource.getColorLightGray()); + break; + } + } else { + composite.setBackground(guiResource.getColorLightGray()); + } + } + + // Update priority bar color based on read state + Control[] children = composite.getChildren(); + for (Control child : children) { + if (child instanceof Composite) { + Object type = child.getData("type"); + if ("priorityBar".equals(type)) { + updatePriorityBar((Composite) child, notification, guiResource); + } + } + // Check for title label (can be CLabel or Label) + Object type = child.getData("type"); + if ("title".equals(type)) { + if (child instanceof CLabel) { + CLabel titleLabel = (CLabel) child; + if (notification.isRead()) { + // Use default font (remove bold) + titleLabel.setFont(null); + } else { + titleLabel.setFont(guiResource.getFontBold()); + } + } else if (child instanceof Label) { + Label titleLabel = (Label) child; + if (notification.isRead()) { + // Use default font (remove bold) + titleLabel.setFont(null); + } else { + titleLabel.setFont(guiResource.getFontBold()); + } + } + } + } + } + + /** Update priority bar color based on read state */ + private void updatePriorityBar( + Composite priorityBar, Notification notification, GuiResource guiResource) { + if (notification.isRead()) { + // Read notifications have gray priority bar + priorityBar.setBackground(guiResource.getColorGray()); + } else { + // Unread notifications use the source color from configuration + org.eclipse.swt.graphics.Color sourceColor = getSourceColor(notification, guiResource); + priorityBar.setBackground(sourceColor); + } + } + + /** Recursively attach click listener to composite and all its children */ + private void attachClickListenerRecursive( + Control control, org.eclipse.swt.widgets.Listener listener) { + if (control == null || control.isDisposed()) { + return; + } + // Don't attach to the priority bar or source indicator (they're just visual indicators) + Object type = control.getData("type"); + if (!"priorityBar".equals(type) && !"sourceIndicator".equals(type)) { + // Only attach to leaf controls (Labels, etc.) to avoid duplicate events + // The composite already has the listener, so we don't need to attach to child composites + if (!(control instanceof Composite)) { + control.addListener(SWT.MouseDown, listener); + } + } + if (control instanceof Composite) { + Composite composite = (Composite) control; + for (Control child : composite.getChildren()) { + attachClickListenerRecursive(child, listener); + } + } + } + + /** + * Get color for a notification source. This will be configurable via ConfigOption later. For now, + * uses a simple hash-based color scheme. + */ + private org.eclipse.swt.graphics.Color getSourceColor( + Notification notification, GuiResource guiResource) { + // Try to get color from notification source configuration + String sourceId = notification.getSourceId(); + if (sourceId != null && !sourceId.isEmpty()) { + for (org.apache.hop.ui.hopgui.notifications.config.NotificationSourceConfig source : + sourcesForRender) { + if (sourceId.equals(source.getId())) { + String colorHex = source.getColor(); + if (colorHex != null && !colorHex.isEmpty()) { + try { + String hex = colorHex.startsWith("#") ? colorHex.substring(1) : colorHex; + int colorValue = Integer.parseInt(hex, 16); + return guiResource.getColor( + (colorValue >> 16) & 0xFF, (colorValue >> 8) & 0xFF, colorValue & 0xFF); + } catch (NumberFormatException e) { + // Not a colour we can read; fall through to one derived from the source name. + } + } + break; + } + } + } + + // Fallback: hash-based color generation for consistent colors per source + String source = notification.getSource(); + if (source == null || source.isEmpty()) { + return guiResource.getColorGray(); + } + + int hash = source.hashCode(); + int r = Math.abs(hash % 200) + 50; // 50-250 range + int g = Math.abs((hash >> 8) % 200) + 50; + int b = Math.abs((hash >> 16) % 200) + 50; + + return guiResource.getColor(r, g, b); + } + + /** Build tooltip text for source indicator showing source name and URL */ + private String buildSourceTooltip(Notification notification) { + StringBuilder tooltip = new StringBuilder(); + if (notification.getSource() != null && !notification.getSource().isEmpty()) { + tooltip.append("Source: ").append(notification.getSource()); + } + if (notification.getLink() != null && !notification.getLink().isEmpty()) { + if (tooltip.length() > 0) { + tooltip.append("\n"); + } + tooltip.append("URL: ").append(notification.getLink()); + } + return tooltip.length() > 0 ? tooltip.toString() : "Unknown source"; + } + + /** Format timestamp for display */ + private String formatTimestamp(Date timestamp) { + if (timestamp == null) { + return ""; + } + long diff = System.currentTimeMillis() - timestamp.getTime(); + long minutes = diff / 60000; + long hours = diff / 3600000; + long days = diff / 86400000; + + if (minutes < 1) { + return "Just now"; Review Comment: Moved to the bundle with separate singular/plural keys, since `BaseMessages` has no plural rules. Also caught one you didn't mention: `new SimpleDateFormat("MMM d, yyyy")` had no `Locale`, so month names came from the platform default while everything around them was English — now `LanguageChoice.getInstance().getDefaultLocale()`. `"Unknown source"` was hardcoded too. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
