From becbe01f68259e96e6614ec3a4d909f1d8e41df4 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 20 Feb 2024 16:16:23 +1100 Subject: [PATCH] FIX: unable to share conversations with persona user (#479) Persona users are still bots, but we were not properly accounting for it and share icon was not showing up. This depends on a core change that adds .topic to transformed posts --- .../discourse/lib/copy-conversation.js | 4 -- .../initializers/ai-bot-replies.js | 13 +++- lib/ai_bot/personas/discourse_helper.rb | 2 +- spec/system/ai_bot/share_spec.rb | 65 +++++++++++++++---- 4 files changed, 63 insertions(+), 21 deletions(-) diff --git a/assets/javascripts/discourse/lib/copy-conversation.js b/assets/javascripts/discourse/lib/copy-conversation.js index 83fc9676..1bd7fb4b 100644 --- a/assets/javascripts/discourse/lib/copy-conversation.js +++ b/assets/javascripts/discourse/lib/copy-conversation.js @@ -56,9 +56,5 @@ async function generateClipboard(topic, fromPostNumber, toPostNumber) { const text = buffer.join("\n"); - if (window.discourseAiTestClipboard) { - window.discourseAiClipboard = text; - } - return text; } diff --git a/assets/javascripts/initializers/ai-bot-replies.js b/assets/javascripts/initializers/ai-bot-replies.js index 8e1f4312..e1827733 100644 --- a/assets/javascripts/initializers/ai-bot-replies.js +++ b/assets/javascripts/initializers/ai-bot-replies.js @@ -143,6 +143,8 @@ function initializePersonaDecorator(api) { ); } +const MAX_PERSONA_USER_ID = -1200; + function initializeShareButton(api) { const currentUser = api.getCurrentUser(); if (!currentUser || !currentUser.ai_enabled_chat_bots) { @@ -159,13 +161,20 @@ function initializeShareButton(api) { }; api.addPostMenuButton("share", (post) => { - // very hacky and ugly, but there is no `.topic` in attrs + // for backwards compat so we don't break if topic is undefined + if (post.topic?.archetype !== "private_message") { + return; + } + if ( !currentUser.ai_enabled_chat_bots.any( (bot) => post.username === bot.username ) ) { - return; + // special handling for personas (persona bot users start at ID -1200 and go down) + if (post.user_id > MAX_PERSONA_USER_ID) { + return; + } } return { diff --git a/lib/ai_bot/personas/discourse_helper.rb b/lib/ai_bot/personas/discourse_helper.rb index 1e40d309..eadfd519 100644 --- a/lib/ai_bot/personas/discourse_helper.rb +++ b/lib/ai_bot/personas/discourse_helper.rb @@ -15,7 +15,7 @@ module DiscourseAi - Discourse Helper Bot understand *markdown* and responds in Discourse **markdown**. - Discourse Helper Bot has access to the search function on meta.discourse.org and can help you find answers to your questions. - Discourse Helper Bot ALWAYS backs up answers with actual search results from meta.discourse.org, even if the information is in your training set - - Discourse Helper Bot does not use the word siscourse in searches, search function is restricted to Discourse Meta and Discourse specific discussions + - Discourse Helper Bot does not use the word Discourse in searches, search function is restricted to Discourse Meta and Discourse specific discussions - Discourse Helper Bot understands that search is keyword based (terms are joined using AND) and that it is important to simplify search terms to find things. - Discourse Helper Bot understands that users often badly phrase and misspell words, it will compensate for that by guessing what user means. diff --git a/spec/system/ai_bot/share_spec.rb b/spec/system/ai_bot/share_spec.rb index 708e96bb..e3c49d36 100644 --- a/spec/system/ai_bot/share_spec.rb +++ b/spec/system/ai_bot/share_spec.rb @@ -27,6 +27,8 @@ RSpec.describe "Share conversation", type: :system do posts end + let(:cdp) { PageObjects::CDP.new } + before do SiteSetting.ai_bot_enabled = true SiteSetting.ai_bot_enabled_chat_bots = "gpt-4" @@ -35,26 +37,62 @@ RSpec.describe "Share conversation", type: :system do bot_user.update!(username: "gpt-4") Group.refresh_automatic_groups! - pm - pm_posts + + cdp.allow_clipboard + page.execute_script("window.navigator.clipboard.writeText('')") + end + + it "can share a conversation with a persona user" do + clip_text = nil + + persona = Fabricate(:ai_persona, name: "Tester") + persona.create_user! + + Fabricate(:post, topic: pm, user: admin, raw: "How do I do stuff?") + Fabricate(:post, topic: pm, user: persona.user, raw: "No idea") + + visit(pm.url) + + find("#post_2 .post-action-menu__share").click + + try_until_success do + clip_text = cdp.read_clipboard + expect(clip_text).not_to eq("") + end + + conversation = (<<~TEXT).strip +
+ + This is my special PM + AI + + + **ai_sharer:** + + How do I do stuff? + + **Tester_bot:** + + No idea +
+ TEXT + + expect(conversation).to eq(clip_text) end it "can share a conversation" do clip_text = nil - visit(pm.url) + pm + pm_posts - # clipboard functionality is extremely hard to test - # we would need special permissions in chrome driver to enable full access - # instead we use a secret variable to signal that we want to store clipboard - # data in window.discourseAiClipboard - page.execute_script("window.discourseAiTestClipboard = true") + visit(pm.url) find("#post_2 .post-action-menu__share").click try_until_success do - clip_text = page.evaluate_script("window.discourseAiClipboard") - expect(clip_text).to be_present + clip_text = cdp.read_clipboard + expect(clip_text).not_to eq("") end conversation = (<<~TEXT).strip @@ -76,16 +114,15 @@ RSpec.describe "Share conversation", type: :system do expect(conversation).to eq(clip_text) - # Test modal functionality as well - page.evaluate_script("window.discourseAiClipboard = null") + page.execute_script("window.navigator.clipboard.writeText('')") find("#post_6 .post-action-menu__share").click find(".ai-share-modal__slider input").set("2") find(".ai-share-modal button.btn-primary").click try_until_success do - clip_text = page.evaluate_script("window.discourseAiClipboard") - expect(clip_text).to be_present + clip_text = cdp.read_clipboard + expect(clip_text).not_to eq("") end conversation = (<<~TEXT).strip