From c78c5dd40786899d3aee6a51696fed9e58a3a46c Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 16 Nov 2022 09:30:20 +0000 Subject: [PATCH 1/4] DEV: Improve discourse-common/deprecate implementation (take 2) (#19032) - Count deprecations and print them to the console following QUnit runs - In GitHub actions, write the same information as a job summary - Add documentation to `discourse-common/lib/deprecated` - Introduce `id` and `url` options to `deprecated` - Introduce `withSilencedDeprecations` helper to allow testing deprecated code paths without making noise in the logs This was previously reverted in 47035693b770a44b680d18b473035eee4778992b. --- .../discourse-common/addon/lib/deprecated.js | 76 ++++++-- .../discourse/app/components/user-selector.js | 4 +- app/assets/javascripts/discourse/testem.js | 45 ++++- .../tests/helpers/deprecation-counter.js | 96 ++++++++++ .../components/user-selector-test.js | 19 +- .../discourse/tests/setup-tests.js | 3 + .../tests/unit/lib/deprecated-test.js | 178 ++++++++++++++++++ 7 files changed, 403 insertions(+), 18 deletions(-) create mode 100644 app/assets/javascripts/discourse/tests/helpers/deprecation-counter.js create mode 100644 app/assets/javascripts/discourse/tests/unit/lib/deprecated-test.js diff --git a/app/assets/javascripts/discourse-common/addon/lib/deprecated.js b/app/assets/javascripts/discourse-common/addon/lib/deprecated.js index 1c36871d6a..1e6bdd1baa 100644 --- a/app/assets/javascripts/discourse-common/addon/lib/deprecated.js +++ b/app/assets/javascripts/discourse-common/addon/lib/deprecated.js @@ -1,17 +1,39 @@ -export default function deprecated(msg, opts = {}) { - msg = ["Deprecation notice:", msg]; - if (opts.since) { - msg.push(`(deprecated since Discourse ${opts.since})`); +const handlers = []; +const disabledDeprecations = new Set(); + +/** + * Display a deprecation warning with the provided message. The warning will be prefixed with the theme/plugin name + * if it can be automatically determined based on the current stack. + * @param {String} msg The deprecation message + * @param {Object} [options] Deprecation options + * @param {String} [options.id] A unique identifier for this deprecation. This should be namespaced by dots (e.g. discourse.my_deprecation) + * @param {String} [options.since] The Discourse version this deprecation was introduced in + * @param {String} [options.dropFrom] The Discourse version this deprecation will be dropped in. Typically one major version after `since` + * @param {String} [options.url] A URL which provides more detail about the deprecation + * @param {boolean} [options.raiseError] Raise an error when this deprecation is triggered. Defaults to `false` + */ +export default function deprecated(msg, options = {}) { + const { id, since, dropFrom, url, raiseError } = options; + + if (id && disabledDeprecations.has(id)) { + return; } - if (opts.dropFrom) { - msg.push(`(removal in Discourse ${opts.dropFrom})`); + + msg = ["Deprecation notice:", msg]; + if (since) { + msg.push(`[deprecated since Discourse ${since}]`); + } + if (dropFrom) { + msg.push(`[removal in Discourse ${dropFrom}]`); + } + if (id) { + msg.push(`[deprecation id: ${id}]`); + } + if (url) { + msg.push(`[info: ${url}]`); } msg = msg.join(" "); - if (opts.raiseError) { - throw msg; - } - let consolePrefix = ""; if (window.Discourse) { // This module doesn't exist in pretty-text/wizard/etc. @@ -19,5 +41,37 @@ export default function deprecated(msg, opts = {}) { require("discourse/lib/source-identifier").consolePrefix() || ""; } - console.warn(consolePrefix, msg); //eslint-disable-line no-console + handlers.forEach((h) => h(msg, options)); + + if (raiseError) { + throw msg; + } + + console.warn(...[consolePrefix, msg].filter(Boolean)); //eslint-disable-line no-console +} + +/** + * Register a function which will be called whenever a deprecation is triggered + * @param {function} callback The callback function. Arguments will match those of `deprecated()`. + */ +export function registerDeprecationHandler(callback) { + handlers.push(callback); +} + +/** + * Silence one or more deprecations while running `callback` + * @async + * @param {(string|string[])} deprecationIds A single id, or an array of ids, of deprecations to silence + * @param {function} callback The function to call while deprecations are silenced. Can be asynchronous. + */ +export async function withSilencedDeprecations(deprecationIds, callback) { + const idArray = [].concat(deprecationIds); + let result; + try { + idArray.forEach((id) => disabledDeprecations.add(id)); + result = callback(); + } finally { + idArray.forEach((id) => disabledDeprecations.delete(id)); + return result; + } } diff --git a/app/assets/javascripts/discourse/app/components/user-selector.js b/app/assets/javascripts/discourse/app/components/user-selector.js index 4f84623fe5..17c85727db 100644 --- a/app/assets/javascripts/discourse/app/components/user-selector.js +++ b/app/assets/javascripts/discourse/app/components/user-selector.js @@ -16,8 +16,8 @@ export default TextField.extend({ @on("init") deprecateComponent() { deprecated( - "`{{user-selector}}` is deprecated. Please use `{{email-group-user-chooser}}` instead.", - { since: "2.7", dropFrom: "2.8" } + "The `` component is deprecated. Please use `` instead.", + { since: "2.7", dropFrom: "2.8", id: "discourse.user-selector-component" } ); }, diff --git a/app/assets/javascripts/discourse/testem.js b/app/assets/javascripts/discourse/testem.js index 1bcc31d2cd..922a6da905 100644 --- a/app/assets/javascripts/discourse/testem.js +++ b/app/assets/javascripts/discourse/testem.js @@ -1,15 +1,21 @@ const TapReporter = require("testem/lib/reporters/tap_reporter"); const { shouldLoadPluginTestJs } = require("discourse/lib/plugin-js"); +const fs = require("fs"); class Reporter { failReports = []; + deprecationCounts = new Map(); constructor() { this._tapReporter = new TapReporter(...arguments); } reportMetadata(tag, metadata) { - if (tag === "summary-line") { + if (tag === "increment-deprecation") { + const id = metadata.id; + const currentCount = this.deprecationCounts.get(id) || 0; + this.deprecationCounts.set(id, currentCount + 1); + } else if (tag === "summary-line") { process.stdout.write(`\n${metadata.message}\n`); } else { this._tapReporter.reportMetadata(...arguments); @@ -23,9 +29,46 @@ class Reporter { this._tapReporter.report(prefix, data); } + generateDeprecationTable() { + const maxIdLength = Math.max( + ...Array.from(this.deprecationCounts.keys()).map((k) => k.length) + ); + + let msg = `| ${"id".padEnd(maxIdLength)} | count |\n`; + msg += `| ${"".padEnd(maxIdLength, "-")} | ----- |\n`; + + for (const [id, count] of this.deprecationCounts.entries()) { + const countString = count.toString(); + msg += `| ${id.padEnd(maxIdLength)} | ${countString.padStart(5)} |\n`; + } + + return msg; + } + + reportDeprecations() { + let deprecationMessage = "[Deprecation Counter] "; + if (this.deprecationCounts.size > 0) { + const table = this.generateDeprecationTable(); + deprecationMessage += `Test run completed with deprecations:\n\n${table}`; + + if (process.env.GITHUB_ACTIONS && process.env.GITHUB_STEP_SUMMARY) { + let jobSummary = `### ⚠️ JS Deprecations\n\nTest run completed with deprecations:\n\n`; + jobSummary += table; + jobSummary += `\n\n`; + + fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, jobSummary); + } + } else { + deprecationMessage += "No deprecations logged"; + } + process.stdout.write(`\n${deprecationMessage}\n\n`); + } + finish() { this._tapReporter.finish(); + this.reportDeprecations(); + if (this.failReports.length > 0) { process.stdout.write("\nFailures:\n\n"); diff --git a/app/assets/javascripts/discourse/tests/helpers/deprecation-counter.js b/app/assets/javascripts/discourse/tests/helpers/deprecation-counter.js new file mode 100644 index 0000000000..166c7e475a --- /dev/null +++ b/app/assets/javascripts/discourse/tests/helpers/deprecation-counter.js @@ -0,0 +1,96 @@ +import { registerDeprecationHandler } from "@ember/debug"; +import { bind } from "discourse-common/utils/decorators"; +import { registerDeprecationHandler as registerDiscourseDeprecationHandler } from "discourse-common/lib/deprecated"; + +export default class DeprecationCounter { + counts = new Map(); + #configById = new Map(); + + constructor(config) { + for (const c of config) { + this.#configById.set(c.matchId, c.handler); + } + } + + start() { + registerDeprecationHandler(this.handleEmberDeprecation); + registerDiscourseDeprecationHandler(this.handleDiscourseDeprecation); + } + + @bind + handleEmberDeprecation(message, options, next) { + const { id } = options; + const matchingConfig = this.#configById.get(id); + + if (matchingConfig !== "silence") { + this.incrementDeprecation(id); + } + + next(message, options); + } + + @bind + handleDiscourseDeprecation(message, options) { + let { id } = options; + id ||= "discourse.(unknown)"; + + this.incrementDeprecation(id); + } + + incrementDeprecation(id) { + const existingCount = this.counts.get(id) || 0; + this.counts.set(id, existingCount + 1); + if (window.Testem) { + reportToTestem(id); + } + } + + get hasDeprecations() { + return this.counts.size > 0; + } + + generateTable() { + const maxIdLength = Math.max( + ...Array.from(this.counts.keys()).map((k) => k.length) + ); + + let msg = `| ${"id".padEnd(maxIdLength)} | count |\n`; + msg += `| ${"".padEnd(maxIdLength, "-")} | ----- |\n`; + + for (const [id, count] of this.counts.entries()) { + const countString = count.toString(); + msg += `| ${id.padEnd(maxIdLength)} | ${countString.padStart(5)} |\n`; + } + + return msg; + } +} + +function reportToTestem(id) { + window.Testem.useCustomAdapter(function (socket) { + socket.emit("test-metadata", "increment-deprecation", { + id, + }); + }); +} + +export function setupDeprecationCounter(qunit) { + const config = window.deprecationWorkflow?.config?.workflow || {}; + const deprecationCounter = new DeprecationCounter(config); + + qunit.begin(() => deprecationCounter.start()); + + qunit.done(() => { + if (window.Testem) { + return; + } else if (deprecationCounter.hasDeprecations) { + // eslint-disable-next-line no-console + console.warn( + `[Discourse Deprecation Counter] Test run completed with deprecations:\n\n${deprecationCounter.generateTable()}` + ); + } else { + // eslint-disable-next-line no-console + console.log("[Discourse Deprecation Counter] No deprecations found"); + } + }); +} diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-selector-test.js b/app/assets/javascripts/discourse/tests/integration/components/user-selector-test.js index 196eb88ead..7a474c3686 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/user-selector-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/user-selector-test.js @@ -3,6 +3,7 @@ import { setupRenderingTest } from "discourse/tests/helpers/component-test"; import { render } from "@ember/test-helpers"; import { query } from "discourse/tests/helpers/qunit-helpers"; import { hbs } from "ember-cli-htmlbars"; +import { withSilencedDeprecations } from "discourse-common/lib/deprecated"; function paste(element, text) { let e = new Event("paste"); @@ -16,8 +17,13 @@ module("Integration | Component | user-selector", function (hooks) { test("pasting a list of usernames", async function (assert) { this.set("usernames", "evil,trout"); - await render( - hbs`` + await withSilencedDeprecations( + "discourse.user-selector-component", + async () => { + await render( + hbs`` + ); + } ); let element = query(".test-selector"); @@ -45,8 +51,13 @@ module("Integration | Component | user-selector", function (hooks) { this.set("usernames", "mark"); this.set("excludedUsernames", ["jeff", "sam", "robin"]); - await render( - hbs`` + await withSilencedDeprecations( + "discourse.user-selector-component", + async () => { + await render( + hbs`` + ); + } ); let element = query(".test-selector"); diff --git a/app/assets/javascripts/discourse/tests/setup-tests.js b/app/assets/javascripts/discourse/tests/setup-tests.js index d95039c060..6aba31ae17 100644 --- a/app/assets/javascripts/discourse/tests/setup-tests.js +++ b/app/assets/javascripts/discourse/tests/setup-tests.js @@ -40,6 +40,7 @@ import { clearState as clearPresenceState } from "discourse/tests/helpers/presen import { addModuleExcludeMatcher } from "ember-cli-test-loader/test-support/index"; import SiteSettingService from "discourse/services/site-settings"; import jQuery from "jquery"; +import { setupDeprecationCounter } from "discourse/tests/helpers/deprecation-counter"; const Plugin = $.fn.modal; const Modal = Plugin.Constructor; @@ -199,6 +200,8 @@ function writeSummaryLine(message) { export default function setupTests(config) { disableCloaking(); + setupDeprecationCounter(QUnit); + QUnit.config.hidepassed = true; sinon.config = { diff --git a/app/assets/javascripts/discourse/tests/unit/lib/deprecated-test.js b/app/assets/javascripts/discourse/tests/unit/lib/deprecated-test.js new file mode 100644 index 0000000000..c64a8d1ba6 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/unit/lib/deprecated-test.js @@ -0,0 +1,178 @@ +import { + default as deprecated, + withSilencedDeprecations, +} from "discourse-common/lib/deprecated"; +import DeprecationCounter from "discourse/tests/helpers/deprecation-counter"; +import { module, test } from "qunit"; +import Sinon from "sinon"; + +module("Unit | Utility | deprecated", function (hooks) { + hooks.beforeEach(function () { + this.warnStub = Sinon.stub(console, "warn"); + this.counterStub = Sinon.stub( + DeprecationCounter.prototype, + "incrementDeprecation" + ); + }); + + test("works with just a message", function (assert) { + deprecated("My message"); + assert.strictEqual( + this.warnStub.callCount, + 1, + "console warn was called once" + ); + assert.deepEqual( + this.warnStub.args[0], + ["Deprecation notice: My message"], + "console.warn is called with the correct arguments" + ); + assert.strictEqual( + this.counterStub.callCount, + 1, + "incrementDeprecation was called once" + ); + assert.deepEqual( + this.counterStub.args[0], + ["discourse.(unknown)"], + "incrementDeprecation is called with the correct arguments" + ); + }); + + test("works with a message and id", function (assert) { + deprecated("My message", { id: "discourse.my_deprecation_id" }); + assert.strictEqual( + this.warnStub.callCount, + 1, + "console warn was called once" + ); + assert.deepEqual( + this.warnStub.args[0], + [ + "Deprecation notice: My message [deprecation id: discourse.my_deprecation_id]", + ], + "console.warn is called with the correct arguments" + ); + assert.strictEqual( + this.counterStub.callCount, + 1, + "incrementDeprecation was called once" + ); + assert.deepEqual( + this.counterStub.args[0], + ["discourse.my_deprecation_id"], + "incrementDeprecation is called with the correct arguments" + ); + }); + + test("works with all other metadata", async function (assert) { + deprecated("My message", { + id: "discourse.my_deprecation_id", + dropFrom: "v100", + since: "v1", + url: "https://example.com", + }); + assert.strictEqual( + this.warnStub.callCount, + 1, + "console warn was called once" + ); + assert.deepEqual( + this.warnStub.args[0], + [ + "Deprecation notice: My message [deprecated since Discourse v1] [removal in Discourse v100] [deprecation id: discourse.my_deprecation_id] [info: https://example.com]", + ], + "console.warn is called with the correct arguments" + ); + assert.strictEqual( + this.counterStub.callCount, + 1, + "incrementDeprecation was called once" + ); + assert.deepEqual( + this.counterStub.args[0], + ["discourse.my_deprecation_id"], + "incrementDeprecation is called with the correct arguments" + ); + }); + + test("works with raiseError", function (assert) { + assert.throws( + () => + deprecated("My message", { + id: "discourse.my_deprecation_id", + raiseError: true, + }), + "Deprecation notice: My message [deprecation id: discourse.my_deprecation_id]" + ); + assert.strictEqual( + this.counterStub.callCount, + 1, + "incrementDeprecation was called once" + ); + assert.deepEqual( + this.counterStub.args[0], + ["discourse.my_deprecation_id"], + "incrementDeprecation is called with the correct arguments" + ); + }); + + test("can silence individual deprecations in tests", function (assert) { + withSilencedDeprecations("discourse.one", () => + deprecated("message", { id: "discourse.one" }) + ); + assert.strictEqual( + this.warnStub.callCount, + 0, + "console.warn is not called" + ); + assert.strictEqual( + this.counterStub.callCount, + 0, + "counter is not incremented" + ); + + deprecated("message", { id: "discourse.one" }); + assert.strictEqual( + this.warnStub.callCount, + 1, + "console.warn is called outside the silenced function" + ); + assert.strictEqual( + this.counterStub.callCount, + 1, + "counter is incremented outside the silenced function" + ); + + withSilencedDeprecations("discourse.one", () => + deprecated("message", { id: "discourse.two" }) + ); + assert.strictEqual( + this.warnStub.callCount, + 2, + "console.warn is called for a different deprecation" + ); + assert.strictEqual( + this.counterStub.callCount, + 2, + "counter is incremented for a different deprecation" + ); + }); + + test("can silence multiple deprecations in tests", function (assert) { + withSilencedDeprecations(["discourse.one", "discourse.two"], () => { + deprecated("message", { id: "discourse.one" }); + deprecated("message", { id: "discourse.two" }); + }); + assert.strictEqual( + this.warnStub.callCount, + 0, + "console.warn is not called" + ); + assert.strictEqual( + this.counterStub.callCount, + 0, + "counter is not incremented" + ); + }); +}); From 5b16546358bbf3f849de8a6f20567df28172ea77 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Wed, 16 Nov 2022 10:33:47 +0100 Subject: [PATCH 2/4] DEV: Refactor `_warnCannotSeeMention()` (#19042) --- .../app/components/composer-editor.js | 49 +++++++++---------- .../components/composer-editor-test.js | 44 +++++++++++++++++ 2 files changed, 66 insertions(+), 27 deletions(-) create mode 100644 app/assets/javascripts/discourse/tests/integration/components/composer-editor-test.js diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index 545039b79d..1d3ccc6452 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -10,6 +10,7 @@ import { } from "discourse/lib/utilities"; import discourseComputed, { bind, + debounce, observes, on, } from "discourse-common/utils/decorators"; @@ -118,6 +119,11 @@ export default Component.extend(ComposerUploadUppy, { uploadPreProcessors, uploadHandlers, + init() { + this._super(...arguments); + this.warnedCannotSeeMentions = []; + }, + @discourseComputed("composer.requiredCategoryMissing") replyPlaceholder(requiredCategoryMissing) { if (requiredCategoryMissing) { @@ -504,41 +510,30 @@ export default Component.extend(ComposerUploadUppy, { }); }, + // add a delay to allow for typing, so you don't open the warning right away + // previously we would warn after @bob even if you were about to mention @bob2 + @debounce(2000) _warnCannotSeeMention(preview) { - const composerDraftKey = this.get("composer.draftKey"); - - if (composerDraftKey === Composer.NEW_PRIVATE_MESSAGE_KEY) { + if (this.composer.draftKey === Composer.NEW_PRIVATE_MESSAGE_KEY) { return; } - schedule("afterRender", () => { - let found = this.warnedCannotSeeMentions || []; + const warnings = []; - preview?.querySelectorAll(".mention.cannot-see")?.forEach((mention) => { - let name = mention.dataset.name; + preview.querySelectorAll(".mention.cannot-see").forEach((mention) => { + const { name } = mention.dataset; - if (!found.includes(name)) { - // add a delay to allow for typing, so you don't open the warning right away - // previously we would warn after @bob even if you were about to mention @bob2 - discourseLater( - this, - () => { - if ( - preview?.querySelectorAll( - `.mention.cannot-see[data-name="${name}"]` - )?.length > 0 - ) { - this.cannotSeeMention([{ name, reason: cannotSee[name] }]); - found.push(name); - } - }, - 2000 - ); - } - }); + if (this.warnedCannotSeeMentions.includes(name)) { + return; + } - this.set("warnedCannotSeeMentions", found); + this.warnedCannotSeeMentions.push(name); + warnings.push({ name, reason: cannotSee[name] }); }); + + if (warnings.length > 0) { + this.cannotSeeMention(warnings); + } }, _warnHereMention(hereCount) { diff --git a/app/assets/javascripts/discourse/tests/integration/components/composer-editor-test.js b/app/assets/javascripts/discourse/tests/integration/components/composer-editor-test.js new file mode 100644 index 0000000000..6bd6016b72 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/integration/components/composer-editor-test.js @@ -0,0 +1,44 @@ +import { module, test } from "qunit"; +import { setupRenderingTest } from "discourse/tests/helpers/component-test"; +import { fillIn, render } from "@ember/test-helpers"; +import { hbs } from "ember-cli-htmlbars"; +import pretender, { response } from "discourse/tests/helpers/create-pretender"; + +module("Integration | Component | ComposerEditor", function (hooks) { + setupRenderingTest(hooks); + + test("warns about users that will not see a mention", async function (assert) { + assert.expect(1); + + this.set("model", {}); + this.set("noop", () => {}); + this.set("expectation", (warnings) => { + assert.deepEqual(warnings, [ + { name: "user-no", reason: "a reason" }, + { name: "user-nope", reason: "a reason" }, + ]); + }); + + pretender.get("/u/is_local_username", () => { + return response({ + cannot_see: { + "user-no": "a reason", + "user-nope": "a reason", + }, + mentionable_groups: [], + valid: ["user-ok", "user-no", "user-nope"], + valid_groups: [], + }); + }); + + await render(hbs` + + `); + + await fillIn("textarea", "@user-no @user-ok @user-nope"); + }); +}); From 41e6b516e559c9c3794f2d30251046bb14904fc2 Mon Sep 17 00:00:00 2001 From: Du Jiajun Date: Wed, 16 Nov 2022 17:42:37 +0800 Subject: [PATCH 3/4] FIX: Support unicode in search filter @username (#18804) --- lib/search.rb | 6 +++--- spec/lib/search_spec.rb | 6 ++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/search.rb b/lib/search.rb index 4e4df3d96d..1eeaa06202 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -646,10 +646,10 @@ class Search end end - advanced_filter(/^\@([a-zA-Z0-9_\-.]+)$/i) do |posts, match| - username = match.downcase + advanced_filter(/^\@(\S+)$/i) do |posts, match| + username = User.normalize_username(match) - user_id = User.where(staged: false).where(username_lower: username).pluck_first(:id) + user_id = User.not_staged.where(username_lower: username).pluck_first(:id) if !user_id && username == "me" user_id = @guardian.user&.id diff --git a/spec/lib/search_spec.rb b/spec/lib/search_spec.rb index b5d6087ae8..5087760fb4 100644 --- a/spec/lib/search_spec.rb +++ b/spec/lib/search_spec.rb @@ -1597,6 +1597,12 @@ RSpec.describe Search do expect(Search.execute("user:#{post_1.user_id}").posts).to contain_exactly(post_1) expect(Search.execute("@#{post_1.user.username}").posts).to contain_exactly(post_1) + + SiteSetting.unicode_usernames = true + unicode_user = Fabricate(:unicode_user) + post_3 = Fabricate(:post, user: unicode_user, raw: 'post by a unicode user', topic: topic) + + expect(Search.execute("@#{post_3.user.username}").posts).to contain_exactly(post_3) end context "when searching for posts made by users of a group" do From 8e60c50f60e3e434e8eb0457e82f5c64cca5e9c0 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Wed, 16 Nov 2022 10:46:30 +0100 Subject: [PATCH 4/4] DEV: Fix a flaky spec (#18995) topic.posts is not ordered by any column by default --- spec/lib/topic_view_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/lib/topic_view_spec.rb b/spec/lib/topic_view_spec.rb index 469c19a6b4..4e75ee2d36 100644 --- a/spec/lib/topic_view_spec.rb +++ b/spec/lib/topic_view_spec.rb @@ -167,7 +167,6 @@ RSpec.describe TopicView do fab!(:p3) { Fabricate(:post, topic: topic, user: first_poster, percent_rank: 0) } it "it can find the best responses" do - best2 = TopicView.new(topic.id, evil_trout, best: 2) expect(best2.posts.count).to eq(2) expect(best2.posts[0].id).to eq(p2.id) @@ -415,7 +414,7 @@ RSpec.describe TopicView do describe "#bookmarks" do let!(:user) { Fabricate(:user) } let!(:bookmark1) { Fabricate(:bookmark_next_business_day_reminder, bookmarkable: topic.first_post, user: user) } - let!(:bookmark2) { Fabricate(:bookmark_next_business_day_reminder, bookmarkable: topic.posts[1], user: user) } + let!(:bookmark2) { Fabricate(:bookmark_next_business_day_reminder, bookmarkable: topic.posts.order(:post_number)[1], user: user) } it "gets the first post bookmark reminder at for the user" do topic_view = TopicView.new(topic.id, user)