diff --git a/app/assets/javascripts/discourse/app/components/user-menu/default-reviewable-item.hbs b/app/assets/javascripts/discourse/app/components/user-menu/default-reviewable-item.hbs new file mode 100644 index 00000000000..10db4fc1943 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/user-menu/default-reviewable-item.hbs @@ -0,0 +1,9 @@ +
  • + + {{d-icon this.icon}} +
    + {{this.actor}} + {{this.description}} +
    +
    +
  • diff --git a/app/assets/javascripts/discourse/app/components/user-menu/default-reviewable-item.js b/app/assets/javascripts/discourse/app/components/user-menu/default-reviewable-item.js new file mode 100644 index 00000000000..7cf8154dc5c --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/user-menu/default-reviewable-item.js @@ -0,0 +1,28 @@ +import GlimmerComponent from "discourse/components/glimmer"; +import I18n from "I18n"; + +export default class UserMenuReviewableItem extends GlimmerComponent { + constructor() { + super(...arguments); + this.reviewable = this.args.item; + } + + get actor() { + const flagger = this.reviewable.flagger_username; + if (flagger) { + return flagger; + } else { + return I18n.t("user_menu.reviewable.deleted_user"); + } + } + + get description() { + return I18n.t("user_menu.reviewable.default_item", { + reviewable_id: this.reviewable.id, + }); + } + + get icon() { + return "flag"; + } +} diff --git a/app/assets/javascripts/discourse/app/components/user-menu/items-list.hbs b/app/assets/javascripts/discourse/app/components/user-menu/items-list.hbs index 9341686d481..4961f29bf5b 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/items-list.hbs +++ b/app/assets/javascripts/discourse/app/components/user-menu/items-list.hbs @@ -9,7 +9,7 @@ {{/each}}
    - {{#if this.showAll}} + {{#if this.showAllHref}} {{d-icon "chevron-down" aria-label=this.showAllTitle}} diff --git a/app/assets/javascripts/discourse/app/components/user-menu/items-list.js b/app/assets/javascripts/discourse/app/components/user-menu/items-list.js index 77cf43a1cb3..31585618afb 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/items-list.js +++ b/app/assets/javascripts/discourse/app/components/user-menu/items-list.js @@ -14,15 +14,7 @@ export default class UserMenuItemsList extends GlimmerComponent { get itemsCacheKey() {} - get showAll() { - return false; - } - - get showAllHref() { - throw new Error( - `the showAllHref getter must be implemented in ${this.constructor.name}` - ); - } + get showAllHref() {} get showAllTitle() {} diff --git a/app/assets/javascripts/discourse/app/components/user-menu/menu.js b/app/assets/javascripts/discourse/app/components/user-menu/menu.js index a28bf8ba8af..6fec39d2c62 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/menu.js +++ b/app/assets/javascripts/discourse/app/components/user-menu/menu.js @@ -6,6 +6,8 @@ import UserMenuTab from "discourse/lib/user-menu/tab"; const DEFAULT_TAB_ID = "all-notifications"; const DEFAULT_PANEL_COMPONENT = "user-menu/notifications-list"; +const REVIEW_QUEUE_TAB_ID = "review-queue"; + const CORE_TOP_TABS = [ class extends UserMenuTab { get id() { @@ -66,6 +68,28 @@ const CORE_TOP_TABS = [ return !this.currentUser.likes_notifications_disabled; } }, + + class extends UserMenuTab { + get id() { + return REVIEW_QUEUE_TAB_ID; + } + + get icon() { + return "flag"; + } + + get panelComponent() { + return "user-menu/reviewables-list"; + } + + get shouldDisplay() { + return this.currentUser.can_review; + } + + get count() { + return this.currentUser.get("reviewable_count"); + } + }, ]; export default class UserMenu extends GlimmerComponent { diff --git a/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js b/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js index a60cc0ebf73..bb698a5b05f 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js +++ b/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js @@ -7,10 +7,6 @@ export default class UserMenuNotificationsList extends UserMenuItemsList { return null; } - get showAll() { - return true; - } - get showAllHref() { return `${this.currentUser.path}/notifications`; } diff --git a/app/assets/javascripts/discourse/app/components/user-menu/reviewable-flagged-post-item.js b/app/assets/javascripts/discourse/app/components/user-menu/reviewable-flagged-post-item.js new file mode 100644 index 00000000000..215972e6ea7 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/user-menu/reviewable-flagged-post-item.js @@ -0,0 +1,20 @@ +import UserMenuDefaultReviewableItem from "discourse/components/user-menu/default-reviewable-item"; +import I18n from "I18n"; +import { htmlSafe } from "@ember/template"; + +export default class UserMenuReviewableFlaggedPostItem extends UserMenuDefaultReviewableItem { + get description() { + const title = this.reviewable.topic_fancy_title; + const postNumber = this.reviewable.post_number; + if (title && postNumber) { + return htmlSafe( + I18n.t("user_menu.reviewable.post_number_with_topic_title", { + post_number: postNumber, + title, + }) + ); + } else { + return I18n.t("user_menu.reviewable.delete_post"); + } + } +} diff --git a/app/assets/javascripts/discourse/app/components/user-menu/reviewable-queued-post-item.js b/app/assets/javascripts/discourse/app/components/user-menu/reviewable-queued-post-item.js new file mode 100644 index 00000000000..814fe109a2a --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/user-menu/reviewable-queued-post-item.js @@ -0,0 +1,32 @@ +import UserMenuDefaultReviewableItem from "discourse/components/user-menu/default-reviewable-item"; +import I18n from "I18n"; +import { htmlSafe } from "@ember/template"; +import { escapeExpression } from "discourse/lib/utilities"; +import { emojiUnescape } from "discourse/lib/text"; + +export default class UserMenuReviewableQueuedPostItem extends UserMenuDefaultReviewableItem { + get actor() { + return I18n.t("user_menu.reviewable.queue"); + } + + get description() { + let title = this.reviewable.topic_fancy_title; + if (!title) { + title = escapeExpression(this.reviewable.payload_title); + } + title = emojiUnescape(title); + if (this.reviewable.is_new_topic) { + return htmlSafe(title); + } else { + return htmlSafe( + I18n.t("user_menu.reviewable.new_post_in_topic", { + title, + }) + ); + } + } + + get icon() { + return "layer-group"; + } +} diff --git a/app/assets/javascripts/discourse/app/components/user-menu/reviewable-user-item.js b/app/assets/javascripts/discourse/app/components/user-menu/reviewable-user-item.js new file mode 100644 index 00000000000..3eb90dcdcee --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/user-menu/reviewable-user-item.js @@ -0,0 +1,14 @@ +import UserMenuDefaultReviewableItem from "discourse/components/user-menu/default-reviewable-item"; +import I18n from "I18n"; + +export default class UserMenuReviewableUserItem extends UserMenuDefaultReviewableItem { + get description() { + return I18n.t("user_menu.reviewable.suspicious_user", { + username: this.reviewable.username, + }); + } + + get icon() { + return "user"; + } +} diff --git a/app/assets/javascripts/discourse/app/components/user-menu/reviewables-list.js b/app/assets/javascripts/discourse/app/components/user-menu/reviewables-list.js new file mode 100644 index 00000000000..61b5c87e7d2 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/user-menu/reviewables-list.js @@ -0,0 +1,27 @@ +import UserMenuItemsList from "discourse/components/user-menu/items-list"; +import { ajax } from "discourse/lib/ajax"; +import UserMenuReviewable from "discourse/models/user-menu-reviewable"; +import I18n from "I18n"; +import getUrl from "discourse-common/lib/get-url"; + +export default class UserMenuReviewablesList extends UserMenuItemsList { + get showAllHref() { + return getUrl("/review"); + } + + get showAllTitle() { + return I18n.t("user_menu.reviewable.view_all"); + } + + get itemsCacheKey() { + return "pending-reviewables"; + } + + fetchItems() { + return ajax("/review/user-menu-list").then((data) => { + return data.reviewables.map((item) => { + return UserMenuReviewable.create(item); + }); + }); + } +} diff --git a/app/assets/javascripts/discourse/app/models/user-menu-reviewable.js b/app/assets/javascripts/discourse/app/models/user-menu-reviewable.js new file mode 100644 index 00000000000..954c98772c6 --- /dev/null +++ b/app/assets/javascripts/discourse/app/models/user-menu-reviewable.js @@ -0,0 +1,18 @@ +import RestModel from "discourse/models/rest"; +import { tracked } from "@glimmer/tracking"; + +const DEFAULT_COMPONENT = "user-menu/default-reviewable-item"; + +const DEFAULT_ITEM_COMPONENTS = { + ReviewableFlaggedPost: "user-menu/reviewable-flagged-post-item", + ReviewableQueuedPost: "user-menu/reviewable-queued-post-item", + ReviewableUser: "user-menu/reviewable-user-item", +}; + +export default class UserMenuReviewable extends RestModel { + @tracked pending; + + get userMenuComponent() { + return DEFAULT_ITEM_COMPONENTS[this.type] || DEFAULT_COMPONENT; + } +} diff --git a/app/assets/javascripts/discourse/tests/helpers/review-pretender.js b/app/assets/javascripts/discourse/tests/helpers/review-pretender.js index 0af2e904a45..b864494a75c 100644 --- a/app/assets/javascripts/discourse/tests/helpers/review-pretender.js +++ b/app/assets/javascripts/discourse/tests/helpers/review-pretender.js @@ -142,6 +142,82 @@ export default function (helpers) { this.put("/review/settings", () => response(200, {})); + this.get("/review/user-menu-list", () => { + return response({ + reviewables: [ + { + flagger_username: "osama", + id: 17, + type: "ReviewableFlaggedPost", + pending: true, + post_number: 3, + topic_fancy_title: + "Emotion clustering crisis struggling sallyport eagled ask", + }, + { + flagger_username: "osama", + id: 15, + type: "ReviewableFlaggedPost", + pending: true, + post_number: 5, + topic_fancy_title: + "Emotion clustering crisis struggling sallyport eagled ask", + }, + { + flagger_username: "system", + id: 4, + type: "ReviewableUser", + pending: false, + username: "trustlevel003", + }, + { + flagger_username: "osama", + id: 18, + type: "ReviewableFlaggedPost", + pending: false, + post_number: 2, + topic_fancy_title: + "Emotion clustering crisis struggling sallyport eagled ask", + }, + { + flagger_username: "osama", + id: 16, + type: "ReviewableFlaggedPost", + pending: false, + post_number: 4, + topic_fancy_title: + "Emotion clustering crisis struggling sallyport eagled ask", + }, + { + flagger_username: "osama", + id: 12, + type: "ReviewableFlaggedPost", + pending: false, + post_number: 9, + topic_fancy_title: + "Emotion clustering crisis struggling sallyport eagled ask", + }, + { + flagger_username: "tony", + id: 1, + type: "ReviewableQueuedPost", + pending: false, + payload_title: "Hello this is a test topic", + is_new_topic: true, + }, + { + flagger_username: "tony", + id: 100, + type: "ReviewableQueuedPost", + pending: false, + topic_fancy_title: "Hello this is a test topic", + is_new_topic: false, + }, + ], + __rest_serializer: "1", + }); + }); + this.get("/review/:id", () => { return response(200, { reviewable: flag, diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-menu/default-reviewable-item-test.js b/app/assets/javascripts/discourse/tests/integration/components/user-menu/default-reviewable-item-test.js new file mode 100644 index 00000000000..6373cc914ca --- /dev/null +++ b/app/assets/javascripts/discourse/tests/integration/components/user-menu/default-reviewable-item-test.js @@ -0,0 +1,75 @@ +import { module, test } from "qunit"; +import { setupRenderingTest } from "discourse/tests/helpers/component-test"; +import { exists, query } from "discourse/tests/helpers/qunit-helpers"; +import UserMenuReviewable from "discourse/models/user-menu-reviewable"; +import { render } from "@ember/test-helpers"; +import { hbs } from "ember-cli-htmlbars"; +import I18n from "I18n"; + +function getReviewable(overrides = {}) { + return UserMenuReviewable.create( + Object.assign( + { + flagger_username: "sayo2", + id: 17, + pending: false, + post_number: 3, + topic_fancy_title: "anything hello world", + type: "ReviewableFlaggedPost", + }, + overrides + ) + ); +} + +module( + "Integration | Component | user-menu | default-reviewable-item", + function (hooks) { + setupRenderingTest(hooks); + + const template = hbs``; + + test("doesn't push `reviewed` to the classList if the reviewable is pending", async function (assert) { + this.set("item", getReviewable({ pending: true })); + await render(template); + assert.ok(!exists("li.reviewed")); + assert.ok(exists("li")); + }); + + test("pushes `reviewed` to the classList if the reviewable isn't pending", async function (assert) { + this.set("item", getReviewable({ pending: false })); + await render(template); + assert.ok(exists("li.reviewed")); + }); + + test("has elements for label and description", async function (assert) { + this.set("item", getReviewable()); + await render(template); + + const label = query("li .reviewable-label"); + const description = query("li .reviewable-description"); + assert.strictEqual( + label.textContent.trim(), + "sayo2", + "the label is the flagger_username" + ); + assert.strictEqual( + description.textContent.trim(), + I18n.t("user_menu.reviewable.default_item", { + reviewable_id: this.item.id, + }), + "displays the description for the reviewable" + ); + }); + + test("the item's label is a placeholder that indicates deleted user if flagger_username is absent", async function (assert) { + this.set("item", getReviewable({ flagger_username: null })); + await render(template); + const label = query("li .reviewable-label"); + assert.strictEqual( + label.textContent.trim(), + I18n.t("user_menu.reviewable.deleted_user") + ); + }); + } +); diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js b/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js index 54b0cc7c23b..0a668286ef0 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js @@ -1,7 +1,7 @@ import { module, test } from "qunit"; import { setupRenderingTest } from "discourse/tests/helpers/component-test"; import { exists, query, queryAll } from "discourse/tests/helpers/qunit-helpers"; -import { click, render } from "@ember/test-helpers"; +import { click, render, settled } from "@ember/test-helpers"; import { NOTIFICATION_TYPES } from "discourse/tests/fixtures/concerns/notification-types"; import { hbs } from "ember-cli-htmlbars"; import pretender from "discourse/tests/helpers/create-pretender"; @@ -81,12 +81,44 @@ module("Integration | Component | user-menu", function (hooks) { assert.deepEqual( tabs.map((t) => t.dataset.tabNumber), - [...Array(4).keys()].map((n) => n.toString()), + ["0", "1", "2", "3"], "data-tab-number of the tabs has no gaps when the likes tab is hidden" ); }); + test("reviewables tab is shown if current user can review", async function (assert) { + this.currentUser.set("can_review", true); + await render(template); + const tab = query("#user-menu-button-review-queue"); + assert.strictEqual(tab.dataset.tabNumber, "4"); + + const tabs = Array.from(queryAll(".tabs-list .btn")); // top and bottom tabs + assert.strictEqual(tabs.length, 6); + + assert.deepEqual( + tabs.map((t) => t.dataset.tabNumber), + ["0", "1", "2", "3", "4", "5"], + "data-tab-number of the tabs has no gaps when the reviewables tab is show" + ); + }); + + test("reviewables count is shown on the reviewables tab", async function (assert) { + this.currentUser.set("can_review", true); + this.currentUser.set("reviewable_count", 4); + await render(template); + const countBadge = query( + "#user-menu-button-review-queue .badge-notification" + ); + assert.strictEqual(countBadge.textContent, "4"); + + this.currentUser.set("reviewable_count", 0); + await settled(); + + assert.ok(!exists("#user-menu-button-review-queue .badge-notification")); + }); + test("changing tabs", async function (assert) { + this.currentUser.set("can_review", true); await render(template); let queryParams; pretender.get("/notifications", (request) => { @@ -224,5 +256,16 @@ module("Integration | Component | user-menu", function (hooks) { "active tab is now the likes tab" ); assert.strictEqual(queryAll("#quick-access-likes ul li").length, 3); + + await click("#user-menu-button-review-queue"); + assert.ok(exists("#quick-access-review-queue.quick-access-panel")); + activeTabs = queryAll(".top-tabs .btn.active"); + assert.strictEqual(activeTabs.length, 1); + assert.strictEqual( + activeTabs[0].id, + "user-menu-button-review-queue", + "active tab is now the reviewables tab" + ); + assert.strictEqual(queryAll("#quick-access-review-queue ul li").length, 8); }); }); diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-menu/reviewable-queued-post-item-test.js b/app/assets/javascripts/discourse/tests/integration/components/user-menu/reviewable-queued-post-item-test.js new file mode 100644 index 00000000000..23793eb41d0 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/integration/components/user-menu/reviewable-queued-post-item-test.js @@ -0,0 +1,77 @@ +import { module, test } from "qunit"; +import { setupRenderingTest } from "discourse/tests/helpers/component-test"; +import { exists, query } from "discourse/tests/helpers/qunit-helpers"; +import UserMenuReviewable from "discourse/models/user-menu-reviewable"; +import { render } from "@ember/test-helpers"; +import { hbs } from "ember-cli-htmlbars"; +import I18n from "I18n"; + +function getReviewable(overrides = {}) { + return UserMenuReviewable.create( + Object.assign( + { + flagger_username: "sayo2", + id: 17, + pending: false, + topic_fancy_title: "anything hello world", + type: "ReviewableQueuedPost", + }, + overrides + ) + ); +} + +module( + "Integration | Component | user-menu | reviewable-queued-post-item", + function (hooks) { + setupRenderingTest(hooks); + + const template = hbs``; + + test("doesn't escape topic_fancy_title because it's safe", async function (assert) { + this.set( + "item", + getReviewable({ + topic_fancy_title: "This is safe title <a> :heart:", + }) + ); + await render(template); + const description = query(".reviewable-description"); + assert.strictEqual( + description.textContent.trim(), + I18n.t("user_menu.reviewable.new_post_in_topic", { + title: "This is safe title ", + }) + ); + assert.strictEqual( + description.querySelectorAll("img.emoji").length, + 1, + "emojis are rendered" + ); + }); + + test("escapes payload_title because it's not safe", async function (assert) { + this.set( + "item", + getReviewable({ + topic_fancy_title: null, + payload_title: "This is unsafe title :heart:", + }) + ); + await render(template); + const description = query(".reviewable-description"); + assert.strictEqual( + description.textContent.trim(), + I18n.t("user_menu.reviewable.new_post_in_topic", { + title: "This is unsafe title ", + }) + ); + assert.strictEqual( + description.querySelectorAll("img.emoji").length, + 1, + "emojis are rendered" + ); + assert.ok(!exists(".reviewable-description a")); + }); + } +); diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-menu/reviewables-list-test.js b/app/assets/javascripts/discourse/tests/integration/components/user-menu/reviewables-list-test.js new file mode 100644 index 00000000000..c5740ed3ddd --- /dev/null +++ b/app/assets/javascripts/discourse/tests/integration/components/user-menu/reviewables-list-test.js @@ -0,0 +1,32 @@ +import { module, test } from "qunit"; +import { setupRenderingTest } from "discourse/tests/helpers/component-test"; +import { query, queryAll } from "discourse/tests/helpers/qunit-helpers"; +import { render } from "@ember/test-helpers"; +import { hbs } from "ember-cli-htmlbars"; +import I18n from "I18n"; + +module( + "Integration | Component | user-menu | reviewables-list", + function (hooks) { + setupRenderingTest(hooks); + + const template = hbs``; + + test("has a 'show all' link", async function (assert) { + await render(template); + const showAll = query(".panel-body-bottom a.show-all"); + assert.ok(showAll.href.endsWith("/review"), "links to the /review page"); + assert.strictEqual( + showAll.title, + I18n.t("user_menu.reviewable.view_all"), + "the 'show all' link has a title" + ); + }); + + test("renders a list of reviewables", async function (assert) { + await render(template); + const reviewables = queryAll("ul li"); + assert.strictEqual(reviewables.length, 8); + }); + } +); diff --git a/app/controllers/reviewables_controller.rb b/app/controllers/reviewables_controller.rb index 5880bb7babf..0f42edd2486 100644 --- a/app/controllers/reviewables_controller.rb +++ b/app/controllers/reviewables_controller.rb @@ -58,7 +58,7 @@ class ReviewablesController < ApplicationController end, meta: filters.merge( total_rows_reviewables: total_rows, types: meta_types, reviewable_types: Reviewable.types, - reviewable_count: Reviewable.list_for(current_user).count + reviewable_count: current_user.reviewable_count ) } if (offset + PER_PAGE) < total_rows @@ -69,6 +69,14 @@ class ReviewablesController < ApplicationController render_json_dump(json, rest_serializer: true) end + def user_menu_list + reviewables = Reviewable.recent_list_with_pending_first(current_user).to_a + json = { + reviewables: reviewables.map! { |r| r.basic_serializer.new(r, scope: guardian, root: nil).as_json } + } + render_json_dump(json, rest_serializer: true) + end + def count render_json_dump(count: Reviewable.pending_count(current_user)) end diff --git a/app/models/reviewable.rb b/app/models/reviewable.rb index 27baea21cf3..d27924fc590 100644 --- a/app/models/reviewable.rb +++ b/app/models/reviewable.rb @@ -1,6 +1,12 @@ # frozen_string_literal: true class Reviewable < ActiveRecord::Base + TYPE_TO_BASIC_SERIALIZER = { + ReviewableFlaggedPost: BasicReviewableFlaggedPostSerializer, + ReviewableQueuedPost: BasicReviewableQueuedPostSerializer, + ReviewableUser: BasicReviewableUserSerializer + } + class UpdateConflict < StandardError; end class InvalidAction < StandardError @@ -458,7 +464,8 @@ class Reviewable < ActiveRecord::Base sort_order: nil, from_date: nil, to_date: nil, - additional_filters: {} + additional_filters: {}, + preload: true ) order = case sort_order when 'score_asc' @@ -473,11 +480,11 @@ class Reviewable < ActiveRecord::Base if username.present? user_id = User.find_by_username(username)&.id - return [] if user_id.blank? + return none if user_id.blank? end - return [] if user.blank? - result = viewable_by(user, order: order) + return none if user.blank? + result = viewable_by(user, order: order, preload: preload) result = by_status(result, status) result = result.where(id: ids) if ids @@ -489,7 +496,7 @@ class Reviewable < ActiveRecord::Base if reviewed_by reviewed_by_id = User.find_by_username(reviewed_by)&.id - return [] if reviewed_by_id.nil? + return none if reviewed_by_id.nil? result = result.joins(<<~SQL INNER JOIN( @@ -534,10 +541,36 @@ class Reviewable < ActiveRecord::Base result end + def self.recent_list_with_pending_first(user, limit: 30) + min_score = Reviewable.min_score_for_priority + + query = Reviewable + .includes(:created_by, :topic, :target) + .viewable_by(user, preload: false) + .except(:order) + .order(score: :desc, created_at: :desc) + .limit(limit) + + if min_score > 0 + query = query.where(<<~SQL, min_score: min_score) + reviewables.score >= :min_score OR reviewables.force_review + SQL + end + records = query.where(status: Reviewable.statuses[:pending]).to_a + if records.size < limit + records += query.where.not(status: Reviewable.statuses[:pending]).to_a + end + records + end + def serializer self.class.serializer_for(self) end + def basic_serializer + TYPE_TO_BASIC_SERIALIZER[self.type.to_sym] || BasicReviewableSerializer + end + def self.lookup_serializer_for(type) "#{type}Serializer".constantize rescue NameError @@ -753,6 +786,7 @@ end # # Indexes # +# idx_reviewables_score_desc_created_at_desc (score,created_at) # index_reviewables_on_reviewable_by_group_id (reviewable_by_group_id) # index_reviewables_on_status_and_created_at (status,created_at) # index_reviewables_on_status_and_score (status,score) diff --git a/app/models/reviewable_flagged_post.rb b/app/models/reviewable_flagged_post.rb index 68e04c19c85..debfa19c340 100644 --- a/app/models/reviewable_flagged_post.rb +++ b/app/models/reviewable_flagged_post.rb @@ -357,6 +357,7 @@ end # # Indexes # +# idx_reviewables_score_desc_created_at_desc (score,created_at) # index_reviewables_on_reviewable_by_group_id (reviewable_by_group_id) # index_reviewables_on_status_and_created_at (status,created_at) # index_reviewables_on_status_and_score (status,score) diff --git a/app/models/reviewable_post.rb b/app/models/reviewable_post.rb index 1ea91fefd94..a8c24b1878a 100644 --- a/app/models/reviewable_post.rb +++ b/app/models/reviewable_post.rb @@ -137,6 +137,7 @@ end # # Indexes # +# idx_reviewables_score_desc_created_at_desc (score,created_at) # index_reviewables_on_reviewable_by_group_id (reviewable_by_group_id) # index_reviewables_on_status_and_created_at (status,created_at) # index_reviewables_on_status_and_score (status,score) diff --git a/app/models/reviewable_queued_post.rb b/app/models/reviewable_queued_post.rb index 5929242a76f..ccfcea66712 100644 --- a/app/models/reviewable_queued_post.rb +++ b/app/models/reviewable_queued_post.rb @@ -205,6 +205,7 @@ end # # Indexes # +# idx_reviewables_score_desc_created_at_desc (score,created_at) # index_reviewables_on_reviewable_by_group_id (reviewable_by_group_id) # index_reviewables_on_status_and_created_at (status,created_at) # index_reviewables_on_status_and_score (status,score) diff --git a/app/models/reviewable_user.rb b/app/models/reviewable_user.rb index 5e8c188d5af..2fbe915c802 100644 --- a/app/models/reviewable_user.rb +++ b/app/models/reviewable_user.rb @@ -126,6 +126,7 @@ end # # Indexes # +# idx_reviewables_score_desc_created_at_desc (score,created_at) # index_reviewables_on_reviewable_by_group_id (reviewable_by_group_id) # index_reviewables_on_status_and_created_at (status,created_at) # index_reviewables_on_status_and_score (status,score) diff --git a/app/models/user.rb b/app/models/user.rb index 98d979fbff1..e8b4adc193a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -591,6 +591,10 @@ class User < ActiveRecord::Base @unread_total_notifications ||= notifications.where("read = false").count end + def reviewable_count + Reviewable.list_for(self).count + end + def saw_notification_id(notification_id) if seen_notification_id.to_i < notification_id.to_i update_columns(seen_notification_id: notification_id.to_i) diff --git a/app/serializers/basic_reviewable_flagged_post_serializer.rb b/app/serializers/basic_reviewable_flagged_post_serializer.rb new file mode 100644 index 00000000000..e0d71d3e2de --- /dev/null +++ b/app/serializers/basic_reviewable_flagged_post_serializer.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class BasicReviewableFlaggedPostSerializer < BasicReviewableSerializer + attributes :post_number, :topic_fancy_title + + def post_number + object.post.post_number + end + + def topic_fancy_title + object.topic.fancy_title + end + + def include_post_number? + object.post.present? + end + + def include_topic_fancy_title? + object.topic.present? + end +end diff --git a/app/serializers/basic_reviewable_queued_post_serializer.rb b/app/serializers/basic_reviewable_queued_post_serializer.rb new file mode 100644 index 00000000000..1d8e884f036 --- /dev/null +++ b/app/serializers/basic_reviewable_queued_post_serializer.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class BasicReviewableQueuedPostSerializer < BasicReviewableSerializer + attributes :topic_fancy_title, :payload_title, :is_new_topic + + def topic_fancy_title + object.topic.fancy_title + end + + def payload_title + object.payload["title"] + end + + def is_new_topic + object.payload["title"].present? + end + + def include_topic_fancy_title? + object.topic.present? + end + + def include_payload_title? + is_new_topic + end +end diff --git a/app/serializers/basic_reviewable_serializer.rb b/app/serializers/basic_reviewable_serializer.rb new file mode 100644 index 00000000000..35d678fd43f --- /dev/null +++ b/app/serializers/basic_reviewable_serializer.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class BasicReviewableSerializer < ApplicationSerializer + attributes :flagger_username, :id, :type, :pending + + def flagger_username + object.created_by&.username + end + + def pending + object.pending? + end +end diff --git a/app/serializers/basic_reviewable_user_serializer.rb b/app/serializers/basic_reviewable_user_serializer.rb new file mode 100644 index 00000000000..6d344da8fa2 --- /dev/null +++ b/app/serializers/basic_reviewable_user_serializer.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class BasicReviewableUserSerializer < BasicReviewableSerializer + attributes :username + + def username + object.payload["username"] + end +end diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index e220db9564a..d341d014386 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -260,10 +260,6 @@ class CurrentUserSerializer < BasicUserSerializer object.anonymous? end - def reviewable_count - Reviewable.list_for(object).count - end - def can_review scope.can_see_review_queue? end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 5de8dd2d22e..e299b4bcf4a 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2555,6 +2555,14 @@ en: generic_no_items: "There are no items in this list." sr_menu_tabs: "Menu tabs" view_all_notifications: "view all notifications" + reviewable: + view_all: "view all review items" + queue: "Queue" + deleted_user: "(deleted user)" + post_number_with_topic_title: "post #%{post_number} - %{title}" + new_post_in_topic: "new post in %{title}" + suspicious_user: "suspicious user %{username}" + default_item: "reviewable item #%{reviewable_id}" topics: new_messages_marker: "last visit" diff --git a/config/routes.rb b/config/routes.rb index 2772b16e868..79e1e9685ac 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -358,6 +358,7 @@ Discourse::Application.routes.draw do get "review/count" => "reviewables#count" get "review/topics" => "reviewables#topics" get "review/settings" => "reviewables#settings" + get "review/user-menu-list" => "reviewables#user_menu_list", format: :json put "review/settings" => "reviewables#settings" put "review/:reviewable_id/perform/:action_id" => "reviewables#perform", constraints: { reviewable_id: /\d+/, diff --git a/db/migrate/20220727085001_create_index_on_reviewables_score_desc_created_at_desc.rb b/db/migrate/20220727085001_create_index_on_reviewables_score_desc_created_at_desc.rb new file mode 100644 index 00000000000..b2cc0ec199f --- /dev/null +++ b/db/migrate/20220727085001_create_index_on_reviewables_score_desc_created_at_desc.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class CreateIndexOnReviewablesScoreDescCreatedAtDesc < ActiveRecord::Migration[7.0] + def change + add_index( + :reviewables, + [:score, :created_at], + order: { score: :desc, created_at: :desc }, + name: 'idx_reviewables_score_desc_created_at_desc' + ) + end +end diff --git a/spec/models/reviewable_spec.rb b/spec/models/reviewable_spec.rb index 92fdbbdf9c3..72acf5e78fa 100644 --- a/spec/models/reviewable_spec.rb +++ b/spec/models/reviewable_spec.rb @@ -244,6 +244,87 @@ RSpec.describe Reviewable, type: :model do end end + describe ".recent_list_with_pending_first" do + fab!(:pending_reviewable1) do + Fabricate( + :reviewable, + score: 150, + created_at: 7.minutes.ago, + status: Reviewable.statuses[:pending] + ) + end + fab!(:pending_reviewable2) do + Fabricate( + :reviewable, + score: 100, + status: Reviewable.statuses[:pending] + ) + end + fab!(:approved_reviewable1) do + Fabricate( + :reviewable, + created_at: 1.minutes.ago, + score: 300, + status: Reviewable.statuses[:approved] + ) + end + fab!(:approved_reviewable2) do + Fabricate( + :reviewable, + created_at: 5.minutes.ago, + score: 200, + status: Reviewable.statuses[:approved] + ) + end + + fab!(:admin) { Fabricate(:admin) } + + it "returns a list of reviewables with pending items first" do + list = Reviewable.recent_list_with_pending_first(admin) + expect(list.map(&:id)).to eq([ + pending_reviewable1.id, + pending_reviewable2.id, + approved_reviewable1.id, + approved_reviewable2.id + ]) + + pending_reviewable1.update!(status: Reviewable.statuses[:rejected]) + rejected_reviewable = pending_reviewable1 + + list = Reviewable.recent_list_with_pending_first(admin) + expect(list.map(&:id)).to eq([ + pending_reviewable2.id, + approved_reviewable1.id, + approved_reviewable2.id, + rejected_reviewable.id, + ]) + end + + it "only includes reviewables whose score is above the minimum or are forced for review" do + SiteSetting.reviewable_default_visibility = 'high' + Reviewable.set_priorities({ high: 200 }) + + list = Reviewable.recent_list_with_pending_first(admin) + expect(list.map(&:id)).to eq([ + approved_reviewable1.id, + approved_reviewable2.id, + ]) + + pending_reviewable1.update!(force_review: true) + + list = Reviewable.recent_list_with_pending_first(admin) + expect(list.map(&:id)).to eq([ + pending_reviewable1.id, + approved_reviewable1.id, + approved_reviewable2.id, + ]) + end + + it "accepts a limit argument to limit the number of returned records" do + expect(Reviewable.recent_list_with_pending_first(admin, limit: 2).size).to eq(2) + end + end + it "valid_types returns the appropriate types" do expect(Reviewable.valid_type?('ReviewableUser')).to eq(true) expect(Reviewable.valid_type?('ReviewableQueuedPost')).to eq(true) diff --git a/spec/requests/reviewables_controller_spec.rb b/spec/requests/reviewables_controller_spec.rb index 4f8161962ea..f959ce6fa71 100644 --- a/spec/requests/reviewables_controller_spec.rb +++ b/spec/requests/reviewables_controller_spec.rb @@ -254,6 +254,73 @@ RSpec.describe ReviewablesController do end end + describe "#user_menu_list" do + it "renders each reviewable with its basic serializers" do + reviewable_user = Fabricate(:reviewable_user, payload: { username: "someb0dy" }) + reviewable_flagged_post = Fabricate(:reviewable_flagged_post) + reviewable_queued_post = Fabricate(:reviewable_queued_post) + + get "/review/user-menu-list.json" + expect(response.status).to eq(200) + + reviewables = response.parsed_body["reviewables"] + + reviewable_queued_post_json = reviewables.find { |r| r["id"] == reviewable_queued_post.id } + expect(reviewable_queued_post_json["is_new_topic"]).to eq(false) + expect(reviewable_queued_post_json["topic_fancy_title"]).to eq( + reviewable_queued_post.topic.fancy_title + ) + + reviewable_flagged_post_json = reviewables.find { |r| r["id"] == reviewable_flagged_post.id } + expect(reviewable_flagged_post_json["post_number"]).to eq( + reviewable_flagged_post.post.post_number + ) + expect(reviewable_flagged_post_json["topic_fancy_title"]).to eq( + reviewable_flagged_post.topic.fancy_title + ) + + reviewable_user_json = reviewables.find { |r| r["id"] == reviewable_user.id } + expect(reviewable_user_json["username"]).to eq("someb0dy") + end + + it "returns JSON containing basic information of reviewables" do + reviewable1 = Fabricate(:reviewable) + reviewable2 = Fabricate(:reviewable, status: Reviewable.statuses[:approved]) + get "/review/user-menu-list.json" + expect(response.status).to eq(200) + reviewables = response.parsed_body["reviewables"] + expect(reviewables.size).to eq(2) + expect(reviewables[0]["flagger_username"]).to eq(reviewable1.created_by.username) + expect(reviewables[0]["id"]).to eq(reviewable1.id) + expect(reviewables[0]["type"]).to eq(reviewable1.type) + expect(reviewables[0]["pending"]).to eq(true) + + expect(reviewables[1]["flagger_username"]).to eq(reviewable2.created_by.username) + expect(reviewables[1]["id"]).to eq(reviewable2.id) + expect(reviewables[1]["type"]).to eq(reviewable2.type) + expect(reviewables[1]["pending"]).to eq(false) + end + + it "puts pending reviewables on top" do + approved1 = Fabricate( + :reviewable, + status: Reviewable.statuses[:approved] + ) + pending = Fabricate( + :reviewable, + status: Reviewable.statuses[:pending] + ) + approved2 = Fabricate( + :reviewable, + status: Reviewable.statuses[:approved] + ) + get "/review/user-menu-list.json" + expect(response.status).to eq(200) + reviewables = response.parsed_body["reviewables"] + expect(reviewables.map { |r| r["id"] }).to eq([pending.id, approved2.id, approved1.id]) + end + end + describe "#show" do context "basics" do fab!(:reviewable) { Fabricate(:reviewable) } diff --git a/spec/serializers/basic_reviewable_flagged_post_serializer_spec.rb b/spec/serializers/basic_reviewable_flagged_post_serializer_spec.rb new file mode 100644 index 00000000000..ce3bffc3706 --- /dev/null +++ b/spec/serializers/basic_reviewable_flagged_post_serializer_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +describe BasicReviewableFlaggedPostSerializer do + fab!(:topic) { Fabricate(:topic, title: "safe title hello world") } + fab!(:post) { Fabricate(:post, topic: topic) } + + fab!(:reviewable) do + ReviewableFlaggedPost.needs_review!(target: post, topic: topic, created_by: Discourse.system_user) + end + + subject { described_class.new(reviewable, root: false).as_json } + + include_examples "basic reviewable attributes" + + describe "#post_number" do + it "equals the post_number of the post" do + expect(subject[:post_number]).to eq(post.post_number) + end + + it "is not included if the reviewable is associated with no post" do + reviewable.update!(target: nil) + expect(subject.key?(:post_number)).to eq(false) + end + end + + describe "#topic_fancy_title" do + it "equals the fancy_title of the topic" do + expect(subject[:topic_fancy_title]).to eq("Safe title <a> hello world") + end + + it "is not included if the reviewable is associated with no topic" do + reviewable.update!(topic: nil) + expect(subject.key?(:topic_fancy_title)).to eq(false) + end + end +end diff --git a/spec/serializers/basic_reviewable_queued_post_serializer_spec.rb b/spec/serializers/basic_reviewable_queued_post_serializer_spec.rb new file mode 100644 index 00000000000..fab15431171 --- /dev/null +++ b/spec/serializers/basic_reviewable_queued_post_serializer_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +describe BasicReviewableQueuedPostSerializer do + fab!(:topic) { Fabricate(:topic, title: "safe title existing topic") } + fab!(:reviewable) do + ReviewableQueuedPost.create!( + created_by: Discourse.system_user, + topic_id: topic.id, + payload: { raw: "new post 123", title: "unsafe title " } + ) + end + + subject { described_class.new(reviewable, root: false).as_json } + + include_examples "basic reviewable attributes" + + describe "#topic_fancy_title" do + it "equals the topic's fancy_title" do + expect(subject[:topic_fancy_title]).to eq("Safe title <a> existing topic") + end + + it "is not included if the reviewable is associated with no topic" do + reviewable.update!(topic: nil) + expect(subject.key?(:topic_fancy_title)).to eq(false) + end + end + + describe "#is_new_topic" do + it "is true if the reviewable's payload has a title attribute" do + expect(subject[:is_new_topic]).to eq(true) + end + + it "is false if the reviewable's payload doesn't have a title attribute" do + reviewable.update!(payload: { raw: "new post 123" }) + expect(subject[:is_new_topic]).to eq(false) + end + end + + describe "#payload_title" do + it "equals the title in the reviewable's payload" do + expect(subject[:payload_title]).to eq("unsafe title ") + end + + it "is not included if the reviewable's payload doesn't have a title attribute" do + reviewable.update!(payload: { raw: "new post 123" }) + expect(subject.key?(:payload_title)).to eq(false) + end + end +end diff --git a/spec/serializers/basic_reviewable_serializer_spec.rb b/spec/serializers/basic_reviewable_serializer_spec.rb new file mode 100644 index 00000000000..82e85b71608 --- /dev/null +++ b/spec/serializers/basic_reviewable_serializer_spec.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +describe BasicReviewableSerializer do + fab!(:reviewable) { Fabricate(:reviewable) } + subject { described_class.new(reviewable, root: false).as_json } + + include_examples "basic reviewable attributes" +end diff --git a/spec/serializers/basic_reviewable_user_serializer_spec.rb b/spec/serializers/basic_reviewable_user_serializer_spec.rb new file mode 100644 index 00000000000..dcd5456fbe9 --- /dev/null +++ b/spec/serializers/basic_reviewable_user_serializer_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +describe BasicReviewableUserSerializer do + fab!(:user) { Fabricate(:user) } + + fab!(:reviewable) do + ReviewableUser.needs_review!( + target: user, + created_by: Discourse.system_user, + payload: { + username: user.username, + name: user.name, + email: user.email, + bio: "blah whatever", + website: "ff.website.com" + } + ) + end + + subject { described_class.new(reviewable, root: false).as_json } + + include_examples "basic reviewable attributes" + + describe "#username" do + it "equals the username in the reviewable's payload" do + expect(subject[:username]).to eq(user.username) + end + end +end diff --git a/spec/support/common_basic_reviewable_serializer.rb b/spec/support/common_basic_reviewable_serializer.rb new file mode 100644 index 00000000000..c439ddb15bb --- /dev/null +++ b/spec/support/common_basic_reviewable_serializer.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +shared_examples "basic reviewable attributes" do + describe "#id" do + it "equals the reviewable's id" do + expect(subject[:id]).to eq(reviewable.id) + end + end + + describe "#type" do + it "is the reviewable's type" do + expect(subject[:type]).to eq(reviewable.type) + end + end + + describe "#pending" do + it "is false if the reviewable is approved" do + reviewable.update!(status: Reviewable.statuses[:approved]) + expect(subject[:pending]).to eq(false) + end + + it "is false if the reviewable is rejected" do + reviewable.update!(status: Reviewable.statuses[:rejected]) + expect(subject[:pending]).to eq(false) + end + + it "is true if the reviewable is pending" do + reviewable.update!(status: Reviewable.statuses[:pending]) + expect(subject[:pending]).to eq(true) + end + end + + describe "#flagger_username" do + it "equals to the username of the user who created the reviewable" do + reviewable.update!( + created_by: Fabricate(:user, username: "gg.osama") + ) + expect(subject[:flagger_username]).to eq("gg.osama") + end + end +end