diff --git a/app/assets/javascripts/discourse/app/lib/notification-types-manager.js b/app/assets/javascripts/discourse/app/lib/notification-types-manager.js index c7a58bee040..7ee7878342b 100644 --- a/app/assets/javascripts/discourse/app/lib/notification-types-manager.js +++ b/app/assets/javascripts/discourse/app/lib/notification-types-manager.js @@ -10,6 +10,7 @@ import LikedConsolidated from "discourse/lib/notification-types/liked-consolidat import Liked from "discourse/lib/notification-types/liked"; import MembershipRequestAccepted from "discourse/lib/notification-types/membership-request-accepted"; import MembershipRequestConsolidated from "discourse/lib/notification-types/membership-request-consolidated"; +import NewFeatures from "discourse/lib/notification-types/new-features"; import MovedPost from "discourse/lib/notification-types/moved-post"; import WatchingFirstPost from "discourse/lib/notification-types/watching-first-post"; @@ -25,6 +26,7 @@ const CLASS_FOR_TYPE = { membership_request_accepted: MembershipRequestAccepted, membership_request_consolidated: MembershipRequestConsolidated, moved_post: MovedPost, + new_features: NewFeatures, watching_first_post: WatchingFirstPost, }; diff --git a/app/assets/javascripts/discourse/app/lib/notification-types/new-features.js b/app/assets/javascripts/discourse/app/lib/notification-types/new-features.js new file mode 100644 index 00000000000..4fe46db69c7 --- /dev/null +++ b/app/assets/javascripts/discourse/app/lib/notification-types/new-features.js @@ -0,0 +1,21 @@ +import NotificationTypeBase from "discourse/lib/notification-types/base"; +import getURL from "discourse-common/lib/get-url"; +import I18n from "I18n"; + +export default class extends NotificationTypeBase { + get label() { + return null; + } + + get description() { + return I18n.t("notifications.new_features"); + } + + get linkHref() { + return getURL("/admin"); + } + + get icon() { + return "gift"; + } +} diff --git a/app/assets/javascripts/discourse/app/widgets/new-features-notification-item.js b/app/assets/javascripts/discourse/app/widgets/new-features-notification-item.js new file mode 100644 index 00000000000..53d205e7d6e --- /dev/null +++ b/app/assets/javascripts/discourse/app/widgets/new-features-notification-item.js @@ -0,0 +1,19 @@ +import { DefaultNotificationItem } from "discourse/widgets/default-notification-item"; +import I18n from "I18n"; +import { createWidgetFrom } from "discourse/widgets/widget"; +import getURL from "discourse-common/lib/get-url"; +import { iconNode } from "discourse-common/lib/icon-library"; + +createWidgetFrom(DefaultNotificationItem, "new-features-notification-item", { + text() { + return I18n.t("notifications.new_features"); + }, + + url() { + return getURL("/admin"); + }, + + icon() { + return iconNode("gift"); + }, +}); diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index 0d524109d22..99f9fcff891 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -24,8 +24,14 @@ class Admin::DashboardController < Admin::StaffController end def new_features + new_features = DiscourseUpdates.new_features + + if current_user.admin? && most_recent = new_features&.first + DiscourseUpdates.bump_last_viewed_feature_date(current_user.id, most_recent["created_at"]) + end + data = { - new_features: DiscourseUpdates.new_features, + new_features: new_features, has_unseen_features: DiscourseUpdates.has_unseen_features?(current_user.id), release_notes_link: AdminDashboardGeneralData.fetch_cached_stats["release_notes_link"] } diff --git a/app/jobs/scheduled/check_new_features.rb b/app/jobs/scheduled/check_new_features.rb index cca5d9b7d70..c3f98bf4885 100644 --- a/app/jobs/scheduled/check_new_features.rb +++ b/app/jobs/scheduled/check_new_features.rb @@ -1,14 +1,50 @@ # frozen_string_literal: true module Jobs - class CheckNewFeatures < ::Jobs::Scheduled every 1.day def execute(args) + admin_ids = User.human_users.where(admin: true).pluck(:id) + + prev_most_recent = DiscourseUpdates.new_features&.first + if prev_most_recent + admin_ids.each do |admin_id| + if DiscourseUpdates.get_last_viewed_feature_date(admin_id).blank? + DiscourseUpdates.bump_last_viewed_feature_date( + admin_id, + prev_most_recent["created_at"] + ) + end + end + end + + # this memoization may seem pointless, but it actually avoids us hitting + # Meta repeatedly and getting rate-limited when this job is ran on a + # multisite cluster. + # in multisite, the `execute` method (of the same instance) is called for + # every site in the cluster. @new_features_json ||= DiscourseUpdates.new_features_payload DiscourseUpdates.update_new_features(@new_features_json) + + new_most_recent = DiscourseUpdates.new_features&.first + if new_most_recent + most_recent_feature_date = Time.zone.parse(new_most_recent["created_at"]) + admin_ids.each do |admin_id| + admin_last_viewed_feature_date = DiscourseUpdates.get_last_viewed_feature_date(admin_id) + if admin_last_viewed_feature_date.blank? || admin_last_viewed_feature_date < most_recent_feature_date + Notification.consolidate_or_create!( + user_id: admin_id, + notification_type: Notification.types[:new_features], + data: {} + ) + DiscourseUpdates.bump_last_viewed_feature_date( + admin_id, + new_most_recent["created_at"] + ) + end + end + end end end - end diff --git a/app/models/notification.rb b/app/models/notification.rb index 350de63720f..7da9513a99d 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -139,6 +139,7 @@ class Notification < ActiveRecord::Base assigned: 34, question_answer_user_commented: 35, # Used by https://github.com/discourse/discourse-question-answer watching_category_or_tag: 36, + new_features: 37, ) end diff --git a/app/services/notifications/consolidation_planner.rb b/app/services/notifications/consolidation_planner.rb index 6dba2faeff9..6d1f5cb44d0 100644 --- a/app/services/notifications/consolidation_planner.rb +++ b/app/services/notifications/consolidation_planner.rb @@ -12,7 +12,7 @@ module Notifications private def plan_for(notification) - consolidation_plans = [liked_by_two_users, liked, group_message_summary, group_membership] + consolidation_plans = [liked_by_two_users, liked, group_message_summary, group_membership, new_features_notification] consolidation_plans.concat(DiscoursePluginRegistry.notification_consolidation_plans) consolidation_plans.detect { |plan| plan.can_consolidate_data?(notification) } @@ -122,5 +122,9 @@ module Notifications end end end + + def new_features_notification + DeletePreviousNotifications.new(type: Notification.types[:new_features]) + end end end diff --git a/app/services/notifications/delete_previous_notifications.rb b/app/services/notifications/delete_previous_notifications.rb index 4c47fac9c26..955af73a012 100644 --- a/app/services/notifications/delete_previous_notifications.rb +++ b/app/services/notifications/delete_previous_notifications.rb @@ -17,7 +17,7 @@ module Notifications class DeletePreviousNotifications < ConsolidationPlan - def initialize(type:, previous_query_blk:) + def initialize(type:, previous_query_blk: nil) @type = type @previous_query_blk = previous_query_blk end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 8185d324d34..d4baf418ac9 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2539,6 +2539,7 @@ en: reaction: "%{username} %{description}" reaction_2: "%{username}, %{username2} %{description}" votes_released: "%{description} - completed" + new_features: "New features available!" dismiss_confirmation: body: default: @@ -2596,6 +2597,7 @@ en: membership_request_consolidated: "new membership requests" reaction: "new reaction" votes_released: "Vote was released" + new_features: "new Discourse features have been released!" upload_selector: uploading: "Uploading" diff --git a/lib/discourse_updates.rb b/lib/discourse_updates.rb index c7450feb5a4..0151855ffda 100644 --- a/lib/discourse_updates.rb +++ b/lib/discourse_updates.rb @@ -176,6 +176,16 @@ module DiscourseUpdates Discourse.redis.set(new_features_last_seen_key(user_id), last_seen["created_at"]) end + def get_last_viewed_feature_date(user_id) + date = Discourse.redis.hget(last_viewed_feature_dates_for_users_key, user_id.to_s) + return if date.blank? + Time.zone.parse(date) + end + + def bump_last_viewed_feature_date(user_id, feature_date) + Discourse.redis.hset(last_viewed_feature_dates_for_users_key, user_id.to_s, feature_date) + end + private def last_installed_version_key @@ -217,5 +227,9 @@ module DiscourseUpdates def new_features_last_seen_key(user_id) "new_features_last_seen_user_#{user_id}" end + + def last_viewed_feature_dates_for_users_key + "last_viewed_feature_dates_for_users_hash" + end end end diff --git a/lib/svg_sprite.rb b/lib/svg_sprite.rb index 3fd725dc747..b82e5451080 100644 --- a/lib/svg_sprite.rb +++ b/lib/svg_sprite.rb @@ -129,6 +129,7 @@ module SvgSprite "folder-open", "forward", "gavel", + "gift", "globe", "globe-americas", "hand-point-right", diff --git a/spec/jobs/check_new_features_spec.rb b/spec/jobs/check_new_features_spec.rb new file mode 100644 index 00000000000..0c1c9f2058d --- /dev/null +++ b/spec/jobs/check_new_features_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +RSpec.describe Jobs::CheckNewFeatures do + def build_feature_hash(id:, created_at:, discourse_version: "2.9.0.beta10") + { + id: id, + user_id: 89432, + emoji: "👤", + title: "New fancy feature!", + description: "", + link: "https://meta.discourse.org/t/-/238821", + created_at: created_at.iso8601, + updated_at: (created_at + 1.minutes).iso8601, + discourse_version: discourse_version + } + end + + def stub_meta_new_features_endpoint(*features) + stub_request(:get, "https://meta.discourse.org/new-features.json") + .to_return( + status: 200, + body: JSON.dump(features), + headers: { + "Content-Type" => "application/json" + } + ) + end + + fab!(:admin1) { Fabricate(:admin) } + fab!(:admin2) { Fabricate(:admin) } + + let(:feature1) do + build_feature_hash( + id: 35, + created_at: 3.days.ago, + discourse_version: "2.8.1.beta12" + ) + end + + let(:feature2) do + build_feature_hash( + id: 34, + created_at: 2.days.ago, + discourse_version: "2.8.1.beta13" + ) + end + + let(:pending_feature) do + build_feature_hash( + id: 37, + created_at: 1.day.ago, + discourse_version: "2.8.1.beta14" + ) + end + + before do + DiscourseUpdates.stubs(:current_version).returns("2.8.1.beta13") + freeze_time + stub_meta_new_features_endpoint(feature1, feature2, pending_feature) + end + + after do + Discourse.redis.del("new_features") + Discourse.redis.del("last_viewed_feature_dates_for_users_hash") + end + + it "backfills last viewed feature for admins who don't have last viewed feature" do + DiscourseUpdates.stubs(:current_version).returns("2.8.1.beta12") + DiscourseUpdates.update_new_features([feature1].to_json) + DiscourseUpdates.bump_last_viewed_feature_date(admin1.id, Time.zone.now.iso8601) + + described_class.new.execute({}) + + expect(DiscourseUpdates.get_last_viewed_feature_date(admin2.id).iso8601).to eq(feature1[:created_at]) + expect(DiscourseUpdates.get_last_viewed_feature_date(admin1.id).iso8601).to eq(Time.zone.now.iso8601) + end + + it "notifies admins about new features that are available in the site's version" do + Notification.destroy_all + + described_class.new.execute({}) + + expect(admin1.notifications.where( + notification_type: Notification.types[:new_features], + read: false + ).count).to eq(1) + expect(admin2.notifications.where( + notification_type: Notification.types[:new_features], + read: false + ).count).to eq(1) + end + + it "consolidates new features notifications" do + Notification.destroy_all + + described_class.new.execute({}) + + notification = admin1.notifications.where( + notification_type: Notification.types[:new_features], + read: false + ).first + expect(notification).to be_present + + DiscourseUpdates.stubs(:current_version).returns("2.8.1.beta14") + described_class.new.execute({}) + + # old notification is destroyed + expect(Notification.find_by(id: notification.id)).to eq(nil) + + notification = admin1.notifications.where( + notification_type: Notification.types[:new_features], + read: false + ).first + # new notification is created + expect(notification).to be_present + end + + it "doesn't notify admins about features they've already seen" do + Notification.destroy_all + DiscourseUpdates.bump_last_viewed_feature_date(admin1.id, feature2[:created_at]) + + described_class.new.execute({}) + + expect(admin1.notifications.count).to eq(0) + expect(admin2.notifications.where(notification_type: Notification.types[:new_features]).count).to eq(1) + end +end diff --git a/spec/lib/discourse_updates_spec.rb b/spec/lib/discourse_updates_spec.rb index e3ad55311e0..dfc65daffd0 100644 --- a/spec/lib/discourse_updates_spec.rb +++ b/spec/lib/discourse_updates_spec.rb @@ -213,4 +213,14 @@ RSpec.describe DiscourseUpdates do expect(result[2]["title"]).to eq("Bells") end end + + describe "#get_last_viewed_feature_date" do + fab!(:user) { Fabricate(:user) } + + it "returns an ActiveSupport::TimeWithZone object" do + time = Time.zone.parse("2022-12-13T21:33:59Z") + DiscourseUpdates.bump_last_viewed_feature_date(user.id, time) + expect(DiscourseUpdates.get_last_viewed_feature_date(user.id)).to eq(time) + end + end end diff --git a/spec/requests/admin/dashboard_controller_spec.rb b/spec/requests/admin/dashboard_controller_spec.rb index 219a2855eab..a59e3ea4aa3 100644 --- a/spec/requests/admin/dashboard_controller_spec.rb +++ b/spec/requests/admin/dashboard_controller_spec.rb @@ -10,21 +10,21 @@ RSpec.describe Admin::DashboardController do Jobs::VersionCheck.any_instance.stubs(:execute).returns(true) end - def populate_new_features + def populate_new_features(date1 = nil, date2 = nil) sample_features = [ { "id" => "1", "emoji" => "🤾", "title" => "Cool Beans", "description" => "Now beans are included", - "created_at" => Time.zone.now - 40.minutes + "created_at" => date1 || (Time.zone.now - 40.minutes) }, { "id" => "2", "emoji" => "🙈", "title" => "Fancy Legumes", "description" => "Legumes too!", - "created_at" => Time.zone.now - 20.minutes + "created_at" => date2 || (Time.zone.now - 20.minutes) } ] @@ -177,6 +177,7 @@ RSpec.describe Admin::DashboardController do sign_in(admin) Discourse.redis.del "new_features_last_seen_user_#{admin.id}" Discourse.redis.del "new_features" + Discourse.redis.del "last_viewed_feature_dates_for_users_hash" end it 'is empty by default' do @@ -217,6 +218,30 @@ RSpec.describe Admin::DashboardController do expect(json['has_unseen_features']).to eq(false) end + + it "sets/bumps the last viewed feature date for the admin" do + date1 = 30.minutes.ago + date2 = 20.minutes.ago + populate_new_features(date1, date2) + + expect(DiscourseUpdates.get_last_viewed_feature_date(admin.id)).to eq(nil) + + get "/admin/dashboard/new-features.json" + expect(response.status).to eq(200) + expect(DiscourseUpdates.get_last_viewed_feature_date(admin.id)).to be_within_one_second_of(date2) + + date2 = 10.minutes.ago + populate_new_features(date1, date2) + + get "/admin/dashboard/new-features.json" + expect(response.status).to eq(200) + expect(DiscourseUpdates.get_last_viewed_feature_date(admin.id)).to be_within_one_second_of(date2) + end + + it "doesn't error when there are no new features" do + get "/admin/dashboard/new-features.json" + expect(response.status).to eq(200) + end end context "when logged in as a moderator" do @@ -234,6 +259,16 @@ RSpec.describe Admin::DashboardController do expect(json['new_features'][0]["title"]).to eq("Fancy Legumes") expect(json['has_unseen_features']).to eq(true) end + + it "doesn't set last viewed feature date for moderators" do + populate_new_features + + expect(DiscourseUpdates.get_last_viewed_feature_date(moderator.id)).to eq(nil) + + get "/admin/dashboard/new-features.json" + expect(response.status).to eq(200) + expect(DiscourseUpdates.get_last_viewed_feature_date(moderator.id)).to eq(nil) + end end context "when logged in as a non-staff user" do diff --git a/spec/requests/api/schemas/json/site_response.json b/spec/requests/api/schemas/json/site_response.json index ebe85af1b0a..635000d874d 100644 --- a/spec/requests/api/schemas/json/site_response.json +++ b/spec/requests/api/schemas/json/site_response.json @@ -38,6 +38,9 @@ "watching_category_or_tag": { "type": "integer" }, + "new_features": { + "type": "integer" + }, "moved_post": { "type": "integer" },