From a710d3f3774dc8812294f84877dd0fdeb0f8b0f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= <1108771+megothss@users.noreply.github.com> Date: Thu, 28 Nov 2024 17:19:35 -0300 Subject: [PATCH] DEV: Ensure `post.updateFromPost` syncs tracked properties (#29970) This commit ensures that tracked properties added to the post model are correctly synced when using `post.updateFromPost`. It also introduces a plugin API to allow plugins to register new tracked properties in the post model without needing to modify the class. --- .../discourse/app/lib/plugin-api.gjs | 14 ++- .../javascripts/discourse/app/models/post.js | 87 ++++++++++++++++--- .../discourse/tests/helpers/qunit-helpers.js | 2 + .../discourse/tests/unit/models/post-test.js | 21 ++++- docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md | 4 + 5 files changed, 112 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/discourse/app/lib/plugin-api.gjs b/app/assets/javascripts/discourse/app/lib/plugin-api.gjs index c106af60607..bbb30201cd3 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-api.gjs +++ b/app/assets/javascripts/discourse/app/lib/plugin-api.gjs @@ -3,7 +3,7 @@ // docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md whenever you change the version // using the format described at https://keepachangelog.com/en/1.0.0/. -export const PLUGIN_API_VERSION = "1.38.0"; +export const PLUGIN_API_VERSION = "1.39.0"; import $ from "jquery"; import { h } from "virtual-dom"; @@ -119,6 +119,7 @@ import Composer, { registerCustomizationCallback, } from "discourse/models/composer"; import { addNavItem } from "discourse/models/nav-item"; +import { _addTrackedPostProperty } from "discourse/models/post"; import { registerCustomLastUnreadUrlCallback } from "discourse/models/topic"; import { addSaveableUserField, @@ -819,6 +820,17 @@ class PluginApi { includeAttributes(...attributes); } + /** + * Adds a tracked property to the post model. + * + * This method is used to mark a property as tracked for post updates. + * + * @param {string} name - The name of the property to track. + */ + addTrackedPostProperty(name) { + _addTrackedPostProperty(name); + } + /** * Add a new button below a post with your plugin. * diff --git a/app/assets/javascripts/discourse/app/models/post.js b/app/assets/javascripts/discourse/app/models/post.js index e1d83242c75..00d16c92062 100644 --- a/app/assets/javascripts/discourse/app/models/post.js +++ b/app/assets/javascripts/discourse/app/models/post.js @@ -10,6 +10,7 @@ import { popupAjaxError } from "discourse/lib/ajax-error"; import { propertyEqual } from "discourse/lib/computed"; import { cook } from "discourse/lib/text"; import { fancyTitle } from "discourse/lib/topic-fancy-title"; +import { defineTrackedProperty } from "discourse/lib/tracked-tools"; import { userPath } from "discourse/lib/url"; import { postUrl } from "discourse/lib/utilities"; import ActionSummary from "discourse/models/action-summary"; @@ -20,6 +21,46 @@ import User from "discourse/models/user"; import discourseComputed from "discourse-common/utils/decorators"; import { i18n } from "discourse-i18n"; +const pluginTrackedProperties = new Set(); +const trackedPropertiesForPostUpdate = new Set(); + +/** + * @internal + * Adds a tracked property to the post model. + * + * Intended to be used only in the plugin API. + * + * @param {string} propertyKey - The key of the property to track. + */ +export function _addTrackedPostProperty(propertyKey) { + pluginTrackedProperties.add(propertyKey); +} + +/** + * Clears all tracked properties added using the API + * + * USE ONLY FOR TESTING PURPOSES. + */ +export function clearAddedTrackedPostProperties() { + pluginTrackedProperties.clear(); +} + +/** + * Decorator to mark a property as post property as tracked. + * + * It extends the standard Ember @tracked behavior to also keep track of the fields + * that need to be copied when using `post.updateFromPost`. + * + * @param {Object} target - The target object. + * @param {string} propertyKey - The key of the property to track. + * @param {PropertyDescriptor} descriptor - The property descriptor. + * @returns {PropertyDescriptor} The updated property descriptor. + */ +function trackedPostProperty(target, propertyKey, descriptor) { + trackedPropertiesForPostUpdate.add(propertyKey); + return tracked(target, propertyKey, descriptor); +} + export default class Post extends RestModel { static munge(json) { if (json.actions_summary) { @@ -107,17 +148,22 @@ export default class Post extends RestModel { @service currentUser; @service site; - @tracked bookmarked; - @tracked can_delete; - @tracked can_edit; - @tracked can_permanently_delete; - @tracked can_recover; - @tracked deleted_at; - @tracked likeAction; - @tracked post_type; - @tracked user_deleted; - @tracked user_id; - @tracked yours; + // Use @trackedPostProperty here instead of Glimmer's @tracked because we need to know which properties are tracked + // in order to correctly update the post in the updateFromPost method. Currently this is not possible using only + // the standard tracked method because these properties are added to the class prototype and are not enumarated by + // object.keys(). + // See https://github.com/emberjs/ember.js/issues/18220 + @trackedPostProperty bookmarked; + @trackedPostProperty can_delete; + @trackedPostProperty can_edit; + @trackedPostProperty can_permanently_delete; + @trackedPostProperty can_recover; + @trackedPostProperty deleted_at; + @trackedPostProperty likeAction; + @trackedPostProperty post_type; + @trackedPostProperty user_deleted; + @trackedPostProperty user_id; + @trackedPostProperty yours; customShare = null; @@ -132,6 +178,15 @@ export default class Post extends RestModel { // Posts can show up as deleted if the topic is deleted @and("firstPost", "topic.deleted_at") deletedViaTopic; + constructor() { + super(...arguments); + + // adds tracked properties defined by plugin to the instance + pluginTrackedProperties.forEach((propertyKey) => { + defineTrackedProperty(this, propertyKey); + }); + } + @discourseComputed("url", "customShare") shareUrl(url) { return this.customShare || resolveShareUrl(url, this.currentUser); @@ -466,11 +521,15 @@ export default class Post extends RestModel { } /** - Updates a post from another's attributes. This will normally happen when a post is loading but - is already found in an identity map. + * Updates a post from another's attributes. This will normally happen when a post is loading but + * is already found in an identity map. **/ updateFromPost(otherPost) { - Object.keys(otherPost).forEach((key) => { + [ + ...Object.keys(otherPost), + ...trackedPropertiesForPostUpdate, + ...pluginTrackedProperties, + ].forEach((key) => { let value = otherPost[key], oldValue = this[key]; diff --git a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js index 4c3d732d5b1..dbb3660f975 100644 --- a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js +++ b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js @@ -82,6 +82,7 @@ import { resetUserSearchCache } from "discourse/lib/user-search"; import { resetComposerCustomizations } from "discourse/models/composer"; import { clearAuthMethods } from "discourse/models/login-method"; import { clearNavItems } from "discourse/models/nav-item"; +import { clearAddedTrackedPostProperties } from "discourse/models/post"; import { resetLastEditNotificationClick } from "discourse/models/post-stream"; import Site from "discourse/models/site"; import User from "discourse/models/user"; @@ -257,6 +258,7 @@ export function testCleanup(container, app) { clearPluginHeaderActionComponents(); clearRegisteredTabs(); resetNeedsHbrTopicList(); + clearAddedTrackedPostProperties(); } function cleanupCssGeneratorTags() { diff --git a/app/assets/javascripts/discourse/tests/unit/models/post-test.js b/app/assets/javascripts/discourse/tests/unit/models/post-test.js index e1e471446d3..e9dd417967d 100644 --- a/app/assets/javascripts/discourse/tests/unit/models/post-test.js +++ b/app/assets/javascripts/discourse/tests/unit/models/post-test.js @@ -1,6 +1,7 @@ import { getOwner } from "@ember/owner"; import { setupTest } from "ember-qunit"; import { module, test } from "qunit"; +import { withPluginApi } from "discourse/lib/plugin-api"; module("Unit | Model | post", function (hooks) { setupTest(hooks); @@ -32,18 +33,36 @@ module("Unit | Model | post", function (hooks) { }); test("updateFromPost", function (assert) { + withPluginApi("1.39.0", (api) => { + api.addTrackedPostProperty("plugin_property"); + }); + const post = this.store.createRecord("post", { post_number: 1, raw: "hello world", + likeAction: null, // `likeAction` is a tracked property from the model added using `@trackedPostProperty` }); post.updateFromPost( this.store.createRecord("post", { raw: "different raw", + yours: false, + likeAction: { count: 1 }, + plugin_property: "different plugin value", }) ); - assert.strictEqual(post.raw, "different raw", "raw field updated"); + assert.strictEqual(post.raw, "different raw", "`raw` field was updated"); + assert.deepEqual( + post.likeAction, + { count: 1 }, + "`likeAction` field was updated" + ); + assert.strictEqual( + post.plugin_property, + "different plugin value", + "`plugin_property` field was updated" + ); }); test("destroy by staff", async function (assert) { diff --git a/docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md b/docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md index 808a71bab0e..a04b66aeff4 100644 --- a/docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md +++ b/docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md @@ -7,6 +7,10 @@ in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.39.0] - 2024-11-27 + +- Added `addTrackedPostProperty` which allows plugins/TCs to add a new tracked property to the post model. + ## [1.38.0] - 2024-10-30 - Added `registerMoreTopicsTab` and "more-topics-tabs" value transformer that allows to add or remove new tabs to the "more topics" (suggested/related) area.