forked from TryGhost/Ghost
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Update Notification improvements (TryGhost#9123)
closes TryGhost#5071 - Remove hardcoded notification in admin controller - NOTE: update check notifications are no longer blocking the admin rendering - this is one of the most import changes - we remove the hardcoded release message - we also remove adding a notification manually in here, because this will work differently from now on -> you receive a notification (release or custom) in the update check module and this module adds the notification as is to our database - Change default core settings keys - remove displayUpdateNotification -> this was used to store the release version number send from the UCS -> based on this value, Ghost creates a notification container with self defined values -> not needed anymore - rename seenNotifications to notifications -> the new notifications key will hold both 1. the notification from the USC 2. the information about if a notification was seen or not - this key hold only one release notification - and n custom notifications - Update Check Module: Request to the USC depends on the privacy configuration - useUpdateCheck: true -> does a checkin in the USC (exposes data) - useUpdateCheck: false -> does only a GET query to the USC (does not expose any data) - make the request handling dynamic, so it depends on the flag - add an extra logic to be able to define a custom USC endpoint (helpful for testing) - add an extra logic to be able to force the request to the service (helpful for testing) - Update check module: re-work condition when a check should happen - only if the env is not correct - remove deprecated config.updateCheck - remove isPrivacyDisabled check (handled differently now, explained in last commit) - Update check module: remove `showUpdateNotification` and readability - showUpdateNotification was used in the admin controller to fetch the latest release version number from the db - no need to check against semver in general, the USC takes care of that (no need to double check) - improve readability of `nextUpdateCheck` condition - Update check module: refactor `updateCheckResponse` - remove db call to displayUpdateNotification, not used anymore - support receiving multiple custom notifications - support custom notification groups - the default group is `all` - this will always be consumed - groups can be extended via config e.g. `notificationGroups: ['migration']` - Update check module: refactor createCustomNotification helper - get rid of taking over notification duplication handling (this is not the task of the update check module) - ensure we have good fallback values for non present attributes in a notification - get rid of semver check (happens in the USC) - could be reconsidered later if LTS is gone - Refactor notification API - reason: get rid of in process notification store -> this was an object hold in process -> everything get's lost after restart -> not helpful anymore, because imagine the following case -> you get a notification -> you store it in process -> you mark this notification as seen -> you restart Ghost, you will receive the same notification on the next check again -> because we are no longer have a separate seen notifications object - use database settings key `notification` instead - refactor all api endpoints to support reading and storing into the `notifications` object - most important: notification deletion happens via a `seen` property (the notification get's physically deleted 3 month automatically) -> we have to remember a seen property, because otherwise you don't know which notification was already received/seen - Add listener to remove seen notifications automatically after 3 month - i just decided for 3 month (we can decrease?) - at the end it doesn't really matter, as long as the windows is not tooooo short - listen on updates for the notifications settings - check if notification was seen and is older than 3 month - ignore release notification - Updated our privacy document - Updated docs.ghost.org for privacy config behaviour - contains a migration script to remove old settings keys
- Loading branch information
Showing
14 changed files
with
1,165 additions
and
370 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
67 changes: 67 additions & 0 deletions
67
core/server/data/migrations/versions/1.20/1-remove-settings-keys.js
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| 'use strict'; | ||
|
|
||
| const _ = require('lodash'), | ||
| models = require('../../../../models'), | ||
| common = require('../../../../lib/common'); | ||
|
|
||
| module.exports.config = { | ||
| transaction: true | ||
| }; | ||
|
|
||
| module.exports.up = function removeSettingKeys(options) { | ||
| let localOptions = _.merge({ | ||
| context: {internal: true} | ||
| }, options); | ||
|
|
||
| return models.Settings.findOne({key: 'display_update_notification'}, localOptions) | ||
| .then(function (settingsModel) { | ||
| if (!settingsModel) { | ||
| common.logging.warn('Deleted Settings Key `display_update_notification`.'); | ||
| return; | ||
| } | ||
|
|
||
| common.logging.info('Deleted Settings Key `display_update_notification`.'); | ||
| return models.Settings.destroy({id: settingsModel.id}, localOptions); | ||
| }) | ||
| .then(function () { | ||
| return models.Settings.findOne({key: 'seen_notifications'}, localOptions); | ||
| }) | ||
| .then(function (settingsModel) { | ||
| if (!settingsModel) { | ||
| common.logging.warn('Deleted Settings Key `seen_notifications`.'); | ||
| return; | ||
| } | ||
|
|
||
| common.logging.info('Deleted Settings Key `seen_notifications`.'); | ||
| return models.Settings.destroy({id: settingsModel.id}, localOptions); | ||
| }); | ||
| }; | ||
|
|
||
| module.exports.down = function addSettingsKeys(options) { | ||
| let localOptions = _.merge({ | ||
| context: {internal: true} | ||
| }, options); | ||
|
|
||
| return models.Settings.findOne({key: 'display_update_notification'}, localOptions) | ||
| .then(function (settingsModel) { | ||
| if (settingsModel) { | ||
| common.logging.warn('Added Settings Key `display_update_notification`.'); | ||
| return; | ||
| } | ||
|
|
||
| common.logging.info('Added Settings Key `display_update_notification`.'); | ||
| return models.Settings.forge({key: 'display_update_notification'}).save(null, localOptions); | ||
| }) | ||
| .then(function () { | ||
| return models.Settings.findOne({key: 'seen_notifications'}, localOptions); | ||
| }) | ||
| .then(function (settingsModel) { | ||
| if (settingsModel) { | ||
| common.logging.warn('Added Settings Key `seen_notifications`.'); | ||
| return; | ||
| } | ||
|
|
||
| common.logging.info('Added Settings Key `seen_notifications`.'); | ||
| return models.Settings.forge({key: 'seen_notifications', value: '[]'}).save([], localOptions); | ||
| }); | ||
| }; |
Oops, something went wrong.