enh(updatenotification): Try to make legacy-notification smart(er)#43742
Draft
joshtrichards wants to merge 1 commit intomasterfrom
Draft
enh(updatenotification): Try to make legacy-notification smart(er)#43742joshtrichards wants to merge 1 commit intomasterfrom
joshtrichards wants to merge 1 commit intomasterfrom
Conversation
Fixes #43645 Signed-off-by: Josh <josh.t.richards@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
If
updatenotificationapp is enabled (the default), but thenotifications(push notifications) app is disabled, we load a so-called legacy-notification as a toast message as a fallback to inform admins about new available updates. If there is a new version of Server available, a new toast is generated upon every DOM reload. An admin (effectively) can't dismiss it:If a user that disables push notifications still wishes to periodically be notified in the Web UI about new updates they must tolerate the constant toasts all day long every day without the ability to dismiss them beyond the next page load.
Example: Joe User is happily running Server v27, but doesn't plan to update to v28 soon. They don't want to be told about the latest v28 release all day long every day long in a new toast.
Alternatives from the user's perspective:
notificationsapp even if they won't really want toupdatenotificationapp outright or settingupdatechecker: falseinconfig.php) [likely not an action we want to encourage if it's merely out of frustration with the constant toasts]This PR tries to make this legacy code a tiny bit smart, by only sending the notification one day a week and on the half hour only.
Currently it's set to every 7th day, but maybe every 4th or something makes more sense (in order to be less likely to skip a weekday/etc.).
Keep in mind this doesn't have to be perfect. It's already only a fallback that is only utilized when the
notificationsapp is disabled.TODO
Checklist