⚡ Optimize get-profiles-for-file-comments query (#11622)

Rewrite sql:file-comment-users to join comment with
comment_thread and union the requesting profile id, then
join the resulting small id set against profile.

The previous "id IN (subquery) OR id = ?" forced a
sequential scan over the whole profile table with a hashed
subplan filter, taking ~1.9s on large instances. The
semi-join lets the planner use profile_pkey, dropping the
query to sub-millisecond time. UNION (not UNION ALL) keeps
the previous dedup semantics when the requesting profile is
also a commenter.

AI-assisted-by: deepseek-flash

backend/src/app/rpc/commands/comments.clj

@@ -390,18 +390,26 @@
 
 (def ^:private sql:file-comment-users
   "WITH available_profiles AS (
-     SELECT DISTINCT owner_id AS id
-       FROM comment
-      WHERE thread_id IN (SELECT id FROM comment_thread WHERE file_id=?)
+     SELECT DISTINCT c.owner_id AS id
+     FROM comment c
+     JOIN comment_thread ct
+       ON ct.id = c.thread_id
+    WHERE ct.file_id = ?::uuid
+  ),
+  profile_ids AS (
+    SELECT id FROM available_profiles
+    UNION
+    SELECT ?::uuid
   )
   SELECT p.id,
          p.email,
          p.fullname AS name,
-         p.fullname AS fullname,
+         p.fullname,
          p.photo_id,
          p.is_active
-    FROM profile AS p
-   WHERE p.id IN (SELECT id FROM available_profiles) OR p.id=?")
+    FROM profile p
+    JOIN profile_ids AS x
+      ON x.id = p.id;")
 
 (defn get-file-comments-users
   [conn file-id profile-id]

common/src/app/common/types/shape/interactions.cljc

@@ -712,21 +712,14 @@
   (conj (or interactions []) interaction))
 
 (defn remove-interaction
-  "Interactions without the one at `index`; unchanged when `index` addresses none."
   [interactions index]
   (let [interactions (or interactions [])]
-    (if (and (int? index) (< -1 index (count interactions)))
-      (into (subvec interactions 0 index)
-            (subvec interactions (inc index)))
-      interactions)))
+    (into (subvec interactions 0 index)
+          (subvec interactions (inc index)))))
 
 (defn update-interaction
-  "Interactions with `update-fn` applied at `index`; unchanged when `index`
-  addresses none."
   [interactions index update-fn]
-  (if (and (int? index) (< -1 index (count interactions)))
-    (update interactions index update-fn)
-    interactions))
+  (update interactions index update-fn))
 
 (defn remap-interactions
   "Update all interactions whose destination points to a shape in the

common/test/common_tests/types/shape_interactions_test.cljc

@@ -858,20 +858,7 @@
     (t/testing "Update interaction"
       (let [new-interactions (ctsi/update-interaction interactions 1 #(ctsi/set-action-type % :open-url))]
         (t/is (= (count new-interactions) 2))
-        (t/is (= (:action-type (last new-interactions)) :open-url))))
-
-    (t/testing "Remove interaction with an index out of range"
-      (t/is (= interactions (ctsi/remove-interaction interactions 2)))
-      (t/is (= interactions (ctsi/remove-interaction interactions -1)))
-      (t/is (= interactions (ctsi/remove-interaction interactions nil)))
-      (t/is (= [] (ctsi/remove-interaction nil 0))))
-
-    (t/testing "Update interaction with an index out of range"
-      (let [update-fn #(ctsi/set-action-type % :open-url)]
-        (t/is (= interactions (ctsi/update-interaction interactions 2 update-fn)))
-        (t/is (= interactions (ctsi/update-interaction interactions -1 update-fn)))
-        (t/is (= interactions (ctsi/update-interaction interactions nil update-fn)))
-        (t/is (nil? (ctsi/update-interaction nil 0 update-fn)))))))
+        (t/is (= (:action-type (last new-interactions)) :open-url))))))
 
 
 (t/deftest remap-interactions

frontend/src/app/plugins/shape.cljs

@@ -80,102 +80,89 @@
   (obj/type-of? p "InteractionProxy"))
 
 (defn interaction-proxy
-  "Proxy over one interaction of a shape.
+  [plugin-id file-id page-id shape-id index]
+  (obj/reify {:name "InteractionProxy"}
+    :$plugin {:enumerable false :get (fn [] plugin-id)}
+    :$file   {:enumerable false :get (fn [] file-id)}
+    :$page   {:enumerable false :get (fn [] page-id)}
+    :$shape  {:enumerable false :get (fn [] shape-id)}
+    :$index  {:enumerable false :get (fn [] index)}
 
-  Interactions are addressed by position, which shifts as interactions are added
-  or removed, so the position is resolved on each access from `interaction`,
-  kept up to date with the writes made through the proxy."
-  [plugin-id file-id page-id shape-id interaction index]
-  (let [current      (atom interaction)
-        locate-index (fn [] (u/locate-interaction-index file-id page-id shape-id @current index))]
-    (obj/reify {:name "InteractionProxy"}
-      :$plugin {:enumerable false :get (fn [] plugin-id)}
-      :$file   {:enumerable false :get (fn [] file-id)}
-      :$page   {:enumerable false :get (fn [] page-id)}
-      :$shape  {:enumerable false :get (fn [] shape-id)}
-      :$index  {:enumerable false :get locate-index}
+    ;; Not enumerable so we don't have an infinite loop
+    :shape
+    {:enumerable false
+     :get (fn [] (shape-proxy plugin-id file-id page-id shape-id))}
 
-      ;; Not enumerable so we don't have an infinite loop
-      :shape
-      {:enumerable false
-       :get (fn [] (shape-proxy plugin-id file-id page-id shape-id))}
-
-      :trigger
-      {:this true
-       :get #(-> % u/proxy->interaction :event-type format/format-key)
-       :set
-       (fn [_ value]
-         (let [value (parser/parse-keyword value)]
-           (cond
-             (not (contains? ctsi/event-types value))
-             (u/not-valid plugin-id :trigger value)
-
-             (not (r/check-permission plugin-id "content:write"))
-             (u/not-valid plugin-id :trigger "Plugin doesn't have 'content:write' permission")
-
-             :else
-             (do
-               (st/emit! (dwi/update-interaction
-                          (u/locate-shape file-id page-id shape-id)
-                          (locate-index)
-                          #(assoc % :event-type value)
-                          {:page-id page-id}))
-               (swap! current assoc :event-type value)))))}
-
-      :delay
-      {:this true
-       :get #(-> % u/proxy->interaction :delay)
-       :set
-       (fn [_ value]
+    :trigger
+    {:this true
+     :get #(-> % u/proxy->interaction :event-type format/format-key)
+     :set
+     (fn [_ value]
+       (let [value (parser/parse-keyword value)]
          (cond
-           (or (not (sm/valid-safe-int? value)) (neg? value))
-           (u/not-valid plugin-id :delay value)
+           (not (contains? ctsi/event-types value))
+           (u/not-valid plugin-id :trigger value)
 
            (not (r/check-permission plugin-id "content:write"))
-           (u/not-valid plugin-id :delay "Plugin doesn't have 'content:write' permission")
+           (u/not-valid plugin-id :trigger "Plugin doesn't have 'content:write' permission")
 
            :else
-           (do
-             (st/emit! (dwi/update-interaction
-                        (u/locate-shape file-id page-id shape-id)
-                        (locate-index)
-                        #(assoc % :delay value)
-                        {:page-id page-id}))
-             (swap! current assoc :delay value))))}
+           (st/emit! (dwi/update-interaction
+                      (u/locate-shape file-id page-id shape-id)
+                      index
+                      #(assoc % :event-type value)
+                      {:page-id page-id})))))}
 
-      :action
-      {:this true
-       :get #(-> % u/proxy->interaction (format/format-action plugin-id file-id page-id))
-       :set
-       (fn [self value]
-         (let [params (parser/parse-action value)
-               interaction
-               (-> (u/proxy->interaction self)
-                   (d/patch-object params))]
-           (cond
-             (not (sm/validate ctsi/schema:interaction interaction))
-             (u/not-valid plugin-id :action interaction)
+    :delay
+    {:this true
+     :get #(-> % u/proxy->interaction :delay)
+     :set
+     (fn [_ value]
+       (cond
+         (or (not (sm/valid-safe-int? value)) (neg? value))
+         (u/not-valid plugin-id :delay value)
 
-             (not (r/check-permission plugin-id "content:write"))
-             (u/not-valid plugin-id :action "Plugin doesn't have 'content:write' permission")
+         (not (r/check-permission plugin-id "content:write"))
+         (u/not-valid plugin-id :delay "Plugin doesn't have 'content:write' permission")
 
-             :else
-             (do
-               (st/emit! (dwi/update-interaction
-                          (u/locate-shape file-id page-id shape-id)
-                          (locate-index)
-                          #(d/patch-object % params)
-                          {:page-id page-id}))
-               (reset! current interaction)))))}
+         :else
+         (st/emit! (dwi/update-interaction
+                    (u/locate-shape file-id page-id shape-id)
+                    index
+                    #(assoc % :delay value)
+                    {:page-id page-id}))))}
 
-      :remove
-      (fn []
-        (cond
-          (not (r/check-permission plugin-id "content:write"))
-          (u/not-valid plugin-id :remove "Plugin doesn't have 'content:write' permission")
+    :action
+    {:this true
+     :get #(-> % u/proxy->interaction (format/format-action plugin-id file-id page-id))
+     :set
+     (fn [self value]
+       (let [params (parser/parse-action value)
+             interaction
+             (-> (u/proxy->interaction self)
+                 (d/patch-object params))]
+         (cond
+           (not (sm/validate ctsi/schema:interaction interaction))
+           (u/not-valid plugin-id :action interaction)
 
-          :else
-          (st/emit! (dwi/remove-interaction {:id shape-id} (locate-index))))))))
+           (not (r/check-permission plugin-id "content:write"))
+           (u/not-valid plugin-id :action "Plugin doesn't have 'content:write' permission")
+
+           :else
+           (st/emit! (dwi/update-interaction
+                      (u/locate-shape file-id page-id shape-id)
+                      index
+                      #(d/patch-object % params)
+                      {:page-id page-id})))))}
+
+    :remove
+    (fn []
+      (cond
+        (not (r/check-permission plugin-id "content:write"))
+        (u/not-valid plugin-id :remove "Plugin doesn't have 'content:write' permission")
+
+        :else
+        (st/emit! (dwi/remove-interaction {:id shape-id} index))))))
 
 (def lib-typography-proxy? nil)
 (def lib-component-proxy nil)
@@ -993,9 +980,8 @@
             (fn [self]
               (let [interactions (-> self u/proxy->shape :interactions)]
                 (format/format-array
-                 (fn [[index interaction]]
-                   (interaction-proxy plugin-id file-id page-id id interaction index))
-                 (d/enumerate interactions))))}
+                 #(interaction-proxy plugin-id file-id page-id id %)
+                 (range 0 (count interactions)))))}
 
            ;; Methods
            :resize
@@ -1640,7 +1626,7 @@
                    (st/emit!
                     (dwi/add-interaction page-id id interaction)
                     (se/event plugin-id "add-interaction"))
-                   (interaction-proxy plugin-id file-id page-id id interaction index)))))
+                   (interaction-proxy plugin-id file-id page-id id index)))))
 
            :removeInteraction
            (fn [interaction]
@@ -1651,9 +1637,6 @@
                (not (r/check-permission plugin-id "content:write"))
                (u/not-valid plugin-id :removeInteraction "Plugin doesn't have 'content:write' permission")
 
-               (not= id (obj/get interaction "$shape"))
-               (u/not-valid plugin-id :removeInteraction "The interaction doesn't belong to this shape")
-
                :else
                (st/emit!
                 (dwi/remove-interaction {:id id} (obj/get interaction "$index"))

frontend/src/app/plugins/utils.cljs

@@ -206,15 +206,6 @@
   (when-let [shape (locate-shape file-id page-id shape-id)]
     (get-in shape [:interactions index])))
 
-(defn locate-interaction-index
-  "Position of `interaction` within the shape's current interactions, falling
-  back to `index` while it addresses an existing interaction."
-  [file-id page-id shape-id interaction index]
-  (let [interactions (-> (locate-shape file-id page-id shape-id) :interactions)]
-    (or (d/index-of interactions interaction)
-        (when (and (int? index) (< -1 index (count interactions)))
-          index))))
-
 (defn proxy->interaction
   [proxy]
   (let [file-id (obj/get proxy "$file")

frontend/translations/en.po

@@ -1770,7 +1770,7 @@ msgstr "At least 1 uppercase letter"
 
 #: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs
 msgid "errors.weak-password.insufficient-digits"
-msgstr "At least 1 number"
+msgstr "At least 1 digit"
 
 #: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs
 msgid "errors.weak-password.insufficient-special"

frontend/translations/es.po

@@ -1735,7 +1735,7 @@ msgstr "Al menos 1 letra mayúscula"
 
 #: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs
 msgid "errors.weak-password.insufficient-digits"
-msgstr "Al menos 1 número"
+msgstr "Al menos 1 dígito"
 
 #: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs
 msgid "errors.weak-password.insufficient-special"

plugins/CHANGELOG.md

@@ -8,8 +8,6 @@
 
 ### 🩹 Fixes
 
-- **plugins-runtime**: An interaction obtained from `Shape.interactions` now keeps addressing that interaction instead of the position it held when the array was read. Removing every interaction of a shape from a single read removes all of them rather than leaving some behind, and writing through a held interaction after an earlier one is removed no longer lands on a different interaction.
-- **plugins-runtime**: `Shape.removeInteraction()` now rejects an interaction belonging to a different shape with a validation error, instead of removing whichever interaction sat at the same position on the target shape.
 - **plugins-runtime**: `Library.createComponent()` now rejects invalid input (an empty shape list, or a shape inside a component copy) with a validation error instead of returning a component proxy pointing at nothing.
 - **plugins-runtime**: Setting an individual padding/margin side (`leftPadding`, `topMargin`, …) now re-derives the padding/margin type, switching to `multiple` when the four sides stop being symmetric (so the value is actually painted) and back to `simple` once top/bottom and left/right are mirrored again.
 

plugins/apps/plugin-api-test-suite/src/tests/interactions.test.ts

@@ -349,70 +349,6 @@ describe('Interactions', () => {
     expect(r.interactions.length).toBe(before - 1);
   });
 
-  // Removing an interaction shifts the ones after it, so draining a shape from
-  // a single read of the array must reach every interaction it returned. Both
-  // removal entry points are covered.
-  test('every interaction can be removed from one read of the array', async (ctx) => {
-    const r = rect(ctx);
-    r.addInteraction('click', { type: 'open-url', url: 'https://a.example' });
-    await ctx.penpot.waitForLayoutUpdate();
-    r.addInteraction('mouse-enter', {
-      type: 'open-url',
-      url: 'https://b.example',
-    });
-    await ctx.penpot.waitForLayoutUpdate();
-    expect(r.interactions).toHaveLength(2);
-
-    for (const interaction of r.interactions) {
-      interaction.remove();
-      await ctx.penpot.waitForLayoutUpdate();
-    }
-    expect(r.interactions).toHaveLength(0);
-  });
-
-  test('removeInteraction can drain a shape from one read of the array', async (ctx) => {
-    const r = rect(ctx);
-    r.addInteraction('click', { type: 'open-url', url: 'https://a.example' });
-    await ctx.penpot.waitForLayoutUpdate();
-    r.addInteraction('mouse-enter', {
-      type: 'open-url',
-      url: 'https://b.example',
-    });
-    await ctx.penpot.waitForLayoutUpdate();
-    expect(r.interactions).toHaveLength(2);
-
-    for (const interaction of r.interactions) {
-      r.removeInteraction(interaction);
-      await ctx.penpot.waitForLayoutUpdate();
-    }
-    expect(r.interactions).toHaveLength(0);
-  });
-
-  // A held interaction addresses itself rather than a position, so a write
-  // reaches it even once an earlier interaction has shifted it.
-  test('an interaction still writes to itself after an earlier one is removed', async (ctx) => {
-    const r = rect(ctx);
-    for (const trigger of ['click', 'mouse-enter', 'mouse-leave'] as const) {
-      r.addInteraction(trigger, {
-        type: 'open-url',
-        url: `https://${trigger}.example`,
-      });
-      await ctx.penpot.waitForLayoutUpdate();
-    }
-    const [first, , last] = r.interactions;
-
-    first.remove();
-    await ctx.penpot.waitForLayoutUpdate();
-    last.delay = 500;
-    await ctx.penpot.waitForLayoutUpdate();
-
-    expect(r.interactions.map((i) => i.trigger)).toEqual([
-      'mouse-enter',
-      'mouse-leave',
-    ]);
-    expect(r.interactions.map((i) => i.delay)).toEqual([null, 500]);
-  });
-
   test('interaction trigger can be changed', (ctx) => {
     const dest = board(ctx);
     const r = rect(ctx);