diff --git a/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js b/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js index 8f24468812..8caaa0fd30 100644 --- a/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js +++ b/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js @@ -27,16 +27,22 @@ createWidget("priority-faq-link", { }, click(e) { - if (this.siteSettings.faq_url === this.attrs.href) { + const { + attrs: { href }, + currentUser, + siteSettings + } = this; + + if (siteSettings.faq_url === href) { ajax(userPath("read-faq"), { type: "POST" }).then(() => { - this.currentUser.set("read_faq", true); + currentUser.set("read_faq", true); if (wantsNewWindow(e)) { return; } e.preventDefault(); - DiscourseURL.routeTo(this.attrs.href); + DiscourseURL.routeTo(href); }); } else { if (wantsNewWindow(e)) { @@ -44,12 +50,14 @@ createWidget("priority-faq-link", { } e.preventDefault(); - DiscourseURL.routeTo(this.attrs.href); + DiscourseURL.routeTo(href); } } }); export default createWidget("hamburger-menu", { + buildKey: () => "hamburger-menu", + tagName: "div.hamburger-panel", settings: { @@ -59,6 +67,10 @@ export default createWidget("hamburger-menu", { showAbout: true }, + defaultState() { + return { loaded: false, loading: false }; + }, + adminLinks() { const { currentUser } = this; @@ -88,15 +100,8 @@ export default createWidget("hamburger-menu", { return tts ? tts.lookupCount(type) : 0; }, - showUserDirectory() { - if (!this.siteSettings.enable_user_directory) return false; - if (this.siteSettings.hide_user_profiles_from_public && !this.currentUser) - return false; - return true; - }, - generalLinks() { - const { siteSettings } = this; + const { attrs, currentUser, siteSettings, state } = this; const links = []; links.push({ @@ -106,7 +111,7 @@ export default createWidget("hamburger-menu", { title: "filters.latest.help" }); - if (this.currentUser) { + if (currentUser) { links.push({ route: "discovery.new", className: "new-topics-link", @@ -124,22 +129,20 @@ export default createWidget("hamburger-menu", { title: "filters.unread.help", count: this.lookupCount("unread") }); - } - // Staff always see the review link. Non-staff will see it if there are items to review - if ( - this.currentUser && - (this.currentUser.staff || this.currentUser.reviewable_count) - ) { - links.push({ - route: siteSettings.reviewable_default_topics - ? "review.topics" - : "review", - className: "review", - label: "review.title", - badgeCount: "reviewable_count", - badgeClass: "reviewables" - }); + // Staff always see the review link. + // Non-staff will see it if there are items to review + if (currentUser.staff || currentUser.reviewable_count) { + links.push({ + route: siteSettings.reviewable_default_topics + ? "review.topics" + : "review", + className: "review", + label: "review.title", + badgeCount: "reviewable_count", + badgeClass: "reviewables" + }); + } } links.push({ @@ -157,7 +160,9 @@ export default createWidget("hamburger-menu", { }); } - if (this.showUserDirectory()) { + const canSeeUserProfiles = + currentUser || !siteSettings.hide_user_profiles_from_public; + if (siteSettings.enable_user_directory && canSeeUserProfiles) { links.push({ route: "users", className: "user-directory-link", @@ -165,7 +170,7 @@ export default createWidget("hamburger-menu", { }); } - if (this.siteSettings.enable_group_directory) { + if (siteSettings.enable_group_directory) { links.push({ route: "groups", className: "groups-link", @@ -173,23 +178,25 @@ export default createWidget("hamburger-menu", { }); } - if (this.siteSettings.tagging_enabled) { + if (siteSettings.tagging_enabled) { links.push({ route: "tags", label: "tagging.tags" }); } const extraLinks = flatten( - applyDecorators(this, "generalLinks", this.attrs, this.state) + applyDecorators(this, "generalLinks", attrs, state) ); + return links.concat(extraLinks).map(l => this.attach("link", l)); }, listCategories() { - const maxCategoriesToDisplay = this.siteSettings - .header_dropdown_category_count; + const { currentUser, site, siteSettings } = this; + const maxCategoriesToDisplay = siteSettings.header_dropdown_category_count; + let categories = []; - if (this.currentUser) { - const allCategories = this.site + if (currentUser) { + const allCategories = site .get("categories") .filter(c => c.notification_level !== NotificationLevels.MUTED); @@ -203,7 +210,8 @@ export default createWidget("hamburger-menu", { ); }); - const topCategoryIds = this.currentUser.get("top_category_ids") || []; + const topCategoryIds = currentUser.get("top_category_ids") || []; + topCategoryIds.forEach(id => { const category = allCategories.find(c => c.id === id); if (category && !categories.includes(category)) { @@ -217,14 +225,14 @@ export default createWidget("hamburger-menu", { .sort((a, b) => b.topic_count - a.topic_count) ); } else { - categories = this.site + categories = site .get("categoriesByCount") .filter(c => c.notification_level !== NotificationLevels.MUTED); } - if (!this.siteSettings.allow_uncategorized_topics) { + if (!siteSettings.allow_uncategorized_topics) { categories = categories.filter( - c => c.id !== this.site.uncategorized_category_id + c => c.id !== site.uncategorized_category_id ); } @@ -235,8 +243,10 @@ export default createWidget("hamburger-menu", { }, footerLinks(prioritizeFaq, faqUrl) { + const { attrs, capabilities, settings, site, siteSettings, state } = this; const links = []; - if (this.settings.showAbout) { + + if (settings.showAbout) { links.push({ route: "about", className: "about-link", @@ -244,12 +254,11 @@ export default createWidget("hamburger-menu", { }); } - if (this.settings.showFAQ && !prioritizeFaq) { + if (settings.showFAQ && !prioritizeFaq) { links.push({ href: faqUrl, className: "faq-link", label: "faq" }); } - const { site } = this; - if (!site.mobileView && !this.capabilities.touch) { + if (!site.mobileView && !capabilities.touch) { links.push({ href: "", action: "showKeyboard", @@ -258,29 +267,28 @@ export default createWidget("hamburger-menu", { }); } - if ( - this.site.mobileView || - (this.siteSettings.enable_mobile_theme && this.capabilities.touch) - ) { + const mobileTouch = siteSettings.enable_mobile_theme && capabilities.touch; + if (site.mobileView || mobileTouch) { links.push({ action: "toggleMobileView", className: "mobile-toggle-link", - label: this.site.mobileView ? "desktop_view" : "mobile_view" + label: site.mobileView ? "desktop_view" : "mobile_view" }); } const extraLinks = flatten( - applyDecorators(this, "footerLinks", this.attrs, this.state) + applyDecorators(this, "footerLinks", attrs, state) ); + return links.concat(extraLinks).map(l => this.attach("link", l)); }, panelContents() { - const { currentUser } = this; + const { attrs, currentUser, settings, siteSettings, state } = this; const results = []; - const faqUrl = this.siteSettings.faq_url || getURL("/faq"); + const faqUrl = siteSettings.faq_url || getURL("/faq"); const prioritizeFaq = - this.settings.showFAQ && this.currentUser && !this.currentUser.read_faq; + settings.showFAQ && currentUser && !currentUser.read_faq; if (prioritizeFaq) { results.push( @@ -300,7 +308,7 @@ export default createWidget("hamburger-menu", { name: "admin-links", contents: () => { const extraLinks = flatten( - applyDecorators(this, "admin-links", this.attrs, this.state) + applyDecorators(this, "admin-links", attrs, state) ); return this.adminLinks().concat(extraLinks); } @@ -315,7 +323,7 @@ export default createWidget("hamburger-menu", { }) ); - if (this.settings.showCategories) { + if (settings.showCategories) { results.push(this.listCategories()); results.push(h("hr.categories-separator")); } @@ -331,7 +339,27 @@ export default createWidget("hamburger-menu", { return results; }, - html() { + refreshReviewableCount(state) { + const { currentUser } = this; + + if (state.loading || !currentUser) return; + + state.loading = true; + + return ajax("/review/count.json") + .then(({ count }) => currentUser.set("reviewable_count", count)) + .finally(() => { + state.loaded = true; + state.loading = false; + this.scheduleRerender(); + }); + }, + + html(attrs, state) { + if (!state.loaded) { + this.refreshReviewableCount(state); + } + return this.attach("menu-panel", { contents: () => this.panelContents(), maxWidth: this.settings.maxWidth diff --git a/app/controllers/reviewables_controller.rb b/app/controllers/reviewables_controller.rb index cdfe645ba4..65de25b429 100644 --- a/app/controllers/reviewables_controller.rb +++ b/app/controllers/reviewables_controller.rb @@ -68,6 +68,10 @@ class ReviewablesController < ApplicationController render_json_dump(json, rest_serializer: true) end + def count + render_json_dump(count: Reviewable.pending_count(current_user)) + end + def topics topic_ids = Set.new diff --git a/app/jobs/regular/notify_reviewable.rb b/app/jobs/regular/notify_reviewable.rb index c6b02c114e..88632a66ae 100644 --- a/app/jobs/regular/notify_reviewable.rb +++ b/app/jobs/regular/notify_reviewable.rb @@ -3,53 +3,39 @@ class Jobs::NotifyReviewable < ::Jobs::Base def execute(args) - reviewable = Reviewable.find_by(id: args[:reviewable_id]) - return unless reviewable.present? + return unless reviewable = Reviewable.find_by(id: args[:reviewable_id]) @contacted = Set.new - notify_admins - notify_moderators if reviewable.reviewable_by_moderator? - if SiteSetting.enable_category_group_moderation? && reviewable.reviewable_by_group.present? - notify_group(reviewable.reviewable_by_group) + counts = Hash.new(0) + + Reviewable.default_visible.pending.each do |r| + counts[:admins] += 1 + counts[:moderators] += 1 if r.reviewable_by_moderator? + counts[r.reviewable_by_group_id] += 1 if r.reviewable_by_group_id + end + + # admins + notify(counts[:admins], User.real.admins.pluck(:id)) + + # moderators + if reviewable.reviewable_by_moderator? + notify(counts[:moderators], User.real.moderators.where("id NOT IN (?)", @contacted).pluck(:id)) + end + + # category moderators + if SiteSetting.enable_category_group_moderation? && (group = reviewable.reviewable_by_group) + group.users.includes(:group_users).where("users.id NOT IN (?)", @contacted).find_each do |user| + count = user.group_users.map { |gu| counts[gu.group_id] }.sum + notify(count, [user.id]) + end end end -protected - - def users - return User if @contacted.blank? - User.where("id NOT IN (?)", @contacted) - end - - def pending - Reviewable.default_visible.pending - end - - def notify_admins - notify(pending.count, users.admins.pluck(:id)) - end - - def notify_moderators - user_ids = users.moderators.pluck(:id) - notify(pending.where(reviewable_by_moderator: true).count, user_ids) - end - - def notify_group(group) - @group_counts = {} - group.users.includes(:group_users).where("users.id NOT IN (?)", @contacted).each do |u| - reviewable_count = u.group_users.map { |gu| count_for_group(gu.group_id) }.sum - MessageBus.publish("/reviewable_counts", { reviewable_count: reviewable_count }, user_ids: [u.id]) - end - end - - def count_for_group(group_id) - @group_counts[group_id] ||= pending.where(reviewable_by_group_id: group_id).count - end + protected def notify(count, user_ids) return if user_ids.blank? - data = { reviewable_count: count } MessageBus.publish("/reviewable_counts", data, user_ids: user_ids) @contacted += user_ids diff --git a/config/routes.rb b/config/routes.rb index 906cb2b206..c124e0b7b3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -338,6 +338,7 @@ Discourse::Application.routes.draw do get "review" => "reviewables#index" # For ember app get "review/:reviewable_id" => "reviewables#show", constraints: { reviewable_id: /\d+/ } get "review/:reviewable_id/explain" => "reviewables#explain", constraints: { reviewable_id: /\d+/ } + get "review/count" => "reviewables#count" get "review/topics" => "reviewables#topics" get "review/settings" => "reviewables#settings" put "review/settings" => "reviewables#settings" diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 537a53fccc..3752ac3092 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -451,38 +451,40 @@ class TopicView end def reviewable_counts - if @reviewable_counts.nil? - - post_ids = @posts.map(&:id) - + @reviewable_counts ||= begin sql = <<~SQL - SELECT target_id, + SELECT + target_id, MAX(r.id) reviewable_id, COUNT(*) total, SUM(CASE WHEN s.status = :pending THEN 1 ELSE 0 END) pending - FROM reviewables r - JOIN reviewable_scores s ON reviewable_id = r.id - WHERE r.target_id IN (:post_ids) AND + FROM + reviewables r + JOIN + reviewable_scores s ON reviewable_id = r.id + WHERE + r.target_id IN (:post_ids) AND r.target_type = 'Post' - GROUP BY target_id + GROUP BY + target_id SQL - @reviewable_counts = {} + counts = {} DB.query( sql, pending: ReviewableScore.statuses[:pending], - post_ids: post_ids + post_ids: @posts.map(&:id) ).each do |row| - @reviewable_counts[row.target_id] = { + counts[row.target_id] = { total: row.total, pending: row.pending, reviewable_id: row.reviewable_id } end - end - @reviewable_counts + counts + end end def pending_posts diff --git a/spec/jobs/notify_reviewable_spec.rb b/spec/jobs/notify_reviewable_spec.rb index c321cc5478..783b78e2c9 100644 --- a/spec/jobs/notify_reviewable_spec.rb +++ b/spec/jobs/notify_reviewable_spec.rb @@ -111,6 +111,6 @@ describe Jobs::NotifyReviewable do described_class.new.execute(reviewable_id: reviewable.id) end - expect(messages.size).to eq(1) + expect(messages.size).to eq(0) end end diff --git a/spec/requests/reviewables_controller_spec.rb b/spec/requests/reviewables_controller_spec.rb index 28a919b309..521a5cdefc 100644 --- a/spec/requests/reviewables_controller_spec.rb +++ b/spec/requests/reviewables_controller_spec.rb @@ -24,6 +24,11 @@ describe ReviewablesController do delete "/review/123" expect(response.code).to eq("403") end + + it "denies count" do + get "/review/count.json" + expect(response.code).to eq("403") + end end context "regular user" do @@ -615,6 +620,28 @@ describe ReviewablesController do end end + context "#count" do + fab!(:admin) { Fabricate(:admin) } + + before do + sign_in(admin) + end + + it "returns the number of reviewables" do + get "/review/count.json" + expect(response.code).to eq("200") + json = response.parsed_body + expect(json["count"]).to eq(0) + + Fabricate(:reviewable_queued_post) + + get "/review/count.json" + expect(response.code).to eq("200") + json = response.parsed_body + expect(json["count"]).to eq(1) + end + end + end end diff --git a/test/javascripts/acceptance/hamburger-menu-test.js b/test/javascripts/acceptance/hamburger-menu-test.js new file mode 100644 index 0000000000..629a903990 --- /dev/null +++ b/test/javascripts/acceptance/hamburger-menu-test.js @@ -0,0 +1,17 @@ +import { acceptance, updateCurrentUser } from "helpers/qunit-helpers"; + +acceptance("Opening the hamburger menu with some reviewables", { + loggedIn: true, + pretend: (server, helper) => { + server.get("/review/count.json", () => helper.response({ count: 3 })); + } +}); + +QUnit.test("As a staff member", async assert => { + updateCurrentUser({ moderator: true, admin: false }); + + await visit("/"); + await click(".hamburger-dropdown"); + + assert.equal(find(".review .badge-notification.reviewables").text(), "3"); +}); diff --git a/test/javascripts/widgets/hamburger-menu-test.js b/test/javascripts/widgets/hamburger-menu-test.js index 675e85352e..68b0075d11 100644 --- a/test/javascripts/widgets/hamburger-menu-test.js +++ b/test/javascripts/widgets/hamburger-menu-test.js @@ -48,20 +48,16 @@ widgetTest("staff menu - not staff", { } }); -widgetTest("staff menu", { +widgetTest("staff menu - moderator", { template: '{{mount-widget widget="hamburger-menu"}}', beforeEach() { - this.currentUser.setProperties({ - moderator: true, - reviewable_count: 3 - }); + this.currentUser.set("moderator", true); }, test(assert) { assert.ok(find(".admin-link").length); assert.ok(find(".review").length); - assert.equal(find(".reviewables").text(), "3"); assert.ok(!find(".settings-link").length); } }); @@ -78,21 +74,6 @@ widgetTest("staff menu - admin", { } }); -widgetTest("reviewable content", { - template: '{{mount-widget widget="hamburger-menu"}}', - - beforeEach() { - this.currentUser.setProperties({ - moderator: true, - reviewable_count: 5 - }); - }, - - test(assert) { - assert.equal(find(".reviewables").text(), "5"); - } -}); - widgetTest("logged in links", { template: '{{mount-widget widget="hamburger-menu"}}',