From 86e2e0188e5f5e2a5b7645f99bf72bf85504099d Mon Sep 17 00:00:00 2001 From: Keegan George Date: Wed, 4 Dec 2024 11:16:37 -0800 Subject: [PATCH 1/4] DEV: Improve explain suggestion footnote replacement --- .../components/ai-post-helper-menu.gjs | 30 +++- .../ai-post-helper-trigger.gjs | 1 + config/locales/client.en.yml | 2 +- spec/system/ai_helper/ai_post_helper_spec.rb | 71 ---------- .../acceptance/post-helper-menu-test.js | 133 ++++++++++++++++++ .../javascripts/fixtures/ai-helper-prompts.js | 66 +++++++++ 6 files changed, 230 insertions(+), 73 deletions(-) create mode 100644 test/javascripts/acceptance/post-helper-menu-test.js create mode 100644 test/javascripts/fixtures/ai-helper-prompts.js diff --git a/assets/javascripts/discourse/components/ai-post-helper-menu.gjs b/assets/javascripts/discourse/components/ai-post-helper-menu.gjs index 9a20037bc..7843e159c 100644 --- a/assets/javascripts/discourse/components/ai-post-helper-menu.gjs +++ b/assets/javascripts/discourse/components/ai-post-helper-menu.gjs @@ -18,6 +18,8 @@ import I18n from "discourse-i18n"; import eq from "truth-helpers/helpers/eq"; import AiHelperLoading from "../components/ai-helper-loading"; import AiHelperOptionsList from "../components/ai-helper-options-list"; +import { modifier } from "ember-modifier"; +import { htmlSafe } from "@ember/template"; export default class AiPostHelperMenu extends Component { @service messageBus; @@ -26,6 +28,7 @@ export default class AiPostHelperMenu extends Component { @service siteSettings; @service currentUser; @service menu; + @service tooltip; @tracked menuState = this.MENU_STATES.options; @tracked loading = false; @@ -38,6 +41,7 @@ export default class AiPostHelperMenu extends Component { @tracked streaming = false; @tracked lastSelectedOption = null; @tracked isSavingFootnote = false; + @tracked supportsAddFootnote = this.args.data.supportsFastEdit; MENU_STATES = { options: "OPTIONS", @@ -47,6 +51,29 @@ export default class AiPostHelperMenu extends Component { @tracked _activeAiRequest = null; + showFootnoteTooltip = modifier((element) => { + if (this.supportsAddFootnote || this.streaming) { + return; + } + + const instance = this.tooltip.register(element, { + identifier: "cannot-add-footnote-tooltip", + content: I18n.t( + "discourse_ai.ai_helper.post_options_menu.footnote_disabled" + ), + placement: "top", + triggers: this.site.mobileView ? ["hold"] : ["hover"], + }); + + return () => { + instance.destroy(); + }; + }); + + get footnoteDisabled() { + return this.streaming || !this.supportsAddFootnote; + } + get helperOptions() { let prompts = this.currentUser?.ai_helper_prompts; @@ -329,8 +356,9 @@ export default class AiPostHelperMenu extends Component { @label="discourse_ai.ai_helper.post_options_menu.insert_footnote" @action={{this.insertFootnote}} @isLoading={{this.isSavingFootnote}} - @disabled={{this.streaming}} + @disabled={{this.footnoteDisabled}} class="btn-flat ai-post-helper__suggestion__insert-footnote" + {{this.showFootnoteTooltip}} /> {{/if}} diff --git a/assets/javascripts/discourse/connectors/post-text-buttons/ai-post-helper-trigger.gjs b/assets/javascripts/discourse/connectors/post-text-buttons/ai-post-helper-trigger.gjs index ef39aac5f..a211ada99 100644 --- a/assets/javascripts/discourse/connectors/post-text-buttons/ai-post-helper-trigger.gjs +++ b/assets/javascripts/discourse/connectors/post-text-buttons/ai-post-helper-trigger.gjs @@ -135,6 +135,7 @@ export default class AiPostHelperTrigger extends Component { identifier: "ai-post-helper-menu", component: AiPostHelperMenu, inline: true, + interactive: true, placement: this.shouldRenderUnder ? "bottom-start" : "top-start", fallbackPlacements: this.shouldRenderUnder ? ["bottom-end", "top-start"] diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6a27f16c1..b0893c917 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -145,7 +145,6 @@ en: response_tokens: "Response tokens" cached_tokens: "Cached tokens" - ai_persona: tool_strategies: all: "Apply to all replies" @@ -395,6 +394,7 @@ en: copied: "Copied!" cancel: "Cancel" insert_footnote: "Add footnote" + footnote_disabled: "Automatic insertion disabled, click copy button and edit it in manually" footnote_credits: "Explanation by AI" fast_edit: suggest_button: "Suggest edit" diff --git a/spec/system/ai_helper/ai_post_helper_spec.rb b/spec/system/ai_helper/ai_post_helper_spec.rb index e3ef46568..93c5244c8 100644 --- a/spec/system/ai_helper/ai_post_helper_spec.rb +++ b/spec/system/ai_helper/ai_post_helper_spec.rb @@ -79,77 +79,6 @@ def select_post_text(selected_post) expect(post.like_count).to eq(1) end - context "when using explain mode" do - let(:mode) { CompletionPrompt::EXPLAIN } - - let(:explain_response) { <<~STRING } - In this context, pie refers to a baked dessert typically consisting of a pastry crust and filling. - The person states they enjoy eating pie, considering it a good dessert. They note that some people wastefully - throw pie at others, but the person themselves chooses to eat the pie rather than throwing it. Overall, pie - is being used to refer the the baked dessert food item. - STRING - - skip "TODO: Streaming causing timing issue in test" do - it "shows an explanation of the selected text" do - select_post_text(post) - post_ai_helper.click_ai_button - - DiscourseAi::Completions::Llm.with_prepared_responses([explain_response]) do - expected_value = explain_response.gsub(/"/, "").strip - - post_ai_helper.select_helper_model(mode) - Jobs.run_immediately! - - wait_for(timeout: 10) do - post_ai_helper.suggestion_value.gsub(/"/, "").strip == expected_value - end - - expect(post_ai_helper.suggestion_value.gsub(/"/, "").strip).to eq(expected_value) - end - end - - it "adds explained text as footnote to post" do - select_post_text(post) - post_ai_helper.click_ai_button - - DiscourseAi::Completions::Llm.with_prepared_responses([explain_response]) do - expected_value = explain_response.gsub(/"/, "").strip - - post_ai_helper.select_helper_model(mode) - Jobs.run_immediately! - - wait_for(timeout: 10) do - post_ai_helper.suggestion_value.gsub(/"/, "").strip == expected_value - end - - post_ai_helper.click_add_footnote - expect(page.has_css?(".expand-footnote")).to eq(true) - end - end - end - end - - context "when using translate mode" do - let(:mode) { CompletionPrompt::TRANSLATE } - - let(:translated_input) { "The rain in Spain, stays mainly in the Plane." } - - skip "TODO: Streaming causing timing issue in test" do - it "shows a translation of the selected text" do - select_post_text(post_2) - post_ai_helper.click_ai_button - - DiscourseAi::Completions::Llm.with_prepared_responses([translated_input]) do - post_ai_helper.select_helper_model(mode) - - wait_for { post_ai_helper.suggestion_value == translated_input } - - expect(post_ai_helper.suggestion_value).to eq(translated_input) - end - end - end - end - context "when using proofread mode" do let(:mode) { CompletionPrompt::PROOFREAD } let(:proofread_response) do diff --git a/test/javascripts/acceptance/post-helper-menu-test.js b/test/javascripts/acceptance/post-helper-menu-test.js new file mode 100644 index 000000000..612eb5e85 --- /dev/null +++ b/test/javascripts/acceptance/post-helper-menu-test.js @@ -0,0 +1,133 @@ +import { click, visit } from "@ember/test-helpers"; +import { test } from "qunit"; +import topicFixtures from "discourse/tests/fixtures/topic"; +import { settled, focus } from "@ember/test-helpers"; +import { + acceptance, + publishToMessageBus, + updateCurrentUser, + selectText, + query, +} from "discourse/tests/helpers/qunit-helpers"; +import { AUTO_GROUPS } from "discourse/lib/constants"; +import { cloneJSON } from "discourse-common/lib/object"; +import aiHelperPrompts from "../fixtures/ai-helper-prompts"; + +acceptance("AI Helper - Post Helper Menu", function (needs) { + needs.settings({ + discourse_ai_enabled: true, + ai_helper_enabled: true, + post_ai_helper_allowed_groups: "1|2", + ai_helper_enabled_features: "suggestions|context_menu", + share_quote_visibility: "anonymous", + enable_markdown_footnotes: true, + display_footnotes_inline: true, + }); + needs.user({ + admin: true, + moderator: true, + groups: [AUTO_GROUPS.admins], + can_use_assistant_in_post: true, + ai_helper_prompts: aiHelperPrompts, + trust_level: 4, + }); + needs.pretender((server, helper) => { + server.get("/t/1.json", () => { + const json = cloneJSON(topicFixtures["/t/28830/1.json"]); + json.post_stream.posts[0].can_edit_post = true; + json.post_stream.posts[0].can_edit = true; + return helper.response(json); + }); + + server.get("/t/2.json", () => { + const json = cloneJSON(topicFixtures["/t/28830/1.json"]); + json.post_stream.posts[0].cooked = + "

La lluvia en España se queda principalmente en el avión.

"; + return helper.response(json); + }); + + server.post(`/discourse-ai/ai-helper/stream_suggestion/`, (request) => { + return helper.response({ + result: "This is a suggestio", + done: false, + }); + }); + + // server.post("/posts/398", (request) => { + // return helper.response({ + // post: { + // id: 398, + // name: "Uwe Keim", + // username: "uwe_keim", + // cooked: + // "

Any THIS IS A FOOTNOTE plans to support localization of UI elements, so that I (for example) could set up a completely German speaking forum?

", + // topic_id: 280, + // post_number: 1, + // raw: "Any THIS IS A FOOTNOTE plans to support localization of UI elements, so that I (for example) could set up a completely German speaking forum?", + // }, + // }); + // }); + }); + + test("displays streamed explanation", async function (assert) { + await visit("/t/-/1"); + const suggestion = "This is a suggestion that is completed"; + const textNode = query("#post_1 .cooked p").childNodes[0]; + await selectText(textNode, 9); + await click(".ai-post-helper__trigger"); + await click(".ai-helper-options__button[data-name='explain']"); + await publishToMessageBus( + `/discourse-ai/ai-helper/stream_suggestion/118591`, + { + done: true, + result: suggestion, + } + ); + assert.dom(".ai-post-helper__suggestion__text").hasText(suggestion); + }); + + async function selectSpecificText(textNode, start, end) { + const range = document.createRange(); + range.setStart(textNode, start); + range.setEnd(textNode, end); + const selection = window.getSelection(); + selection.removeAllRanges(); + selection.addRange(range); + await settled(); + } + + test("adds explained text as footnote to post", async function (assert) { + await visit("/t/-/1"); + const suggestion = "This is a suggestion that is completed"; + + const textNode = query("#post_1 .cooked p").childNodes[0]; + await selectSpecificText(textNode, 72, 77); + await click(".ai-post-helper__trigger"); + await click(".ai-helper-options__button[data-name='explain']"); + await publishToMessageBus( + `/discourse-ai/ai-helper/stream_suggestion/118591`, + { + done: true, + result: suggestion, + } + ); + + assert.dom(".ai-post-helper__suggestion__insert-footnote").isDisabled(); + }); + + test("shows translated post", async function (assert) { + await visit("/t/-/2"); + const translated = "The rain in Spain, stays mainly in the Plane."; + await selectText(query("#post_1 .cooked p")); + await click(".ai-post-helper__trigger"); + await click(".ai-helper-options__button[data-name='translate']"); + await publishToMessageBus( + `/discourse-ai/ai-helper/stream_suggestion/118591`, + { + done: true, + result: translated, + } + ); + assert.dom(".ai-post-helper__suggestion__text").hasText(translated); + }); +}); diff --git a/test/javascripts/fixtures/ai-helper-prompts.js b/test/javascripts/fixtures/ai-helper-prompts.js new file mode 100644 index 000000000..79054ef8a --- /dev/null +++ b/test/javascripts/fixtures/ai-helper-prompts.js @@ -0,0 +1,66 @@ +export default [ + { + id: -301, + name: "translate", + translated_name: "Translate to English (US)", + prompt_type: "text", + icon: "language", + location: ["composer", "post"], + }, + { + id: -303, + name: "proofread", + translated_name: "Proofread text", + prompt_type: "diff", + icon: "spell-check", + location: ["composer", "post"], + }, + { + id: -304, + name: "markdown_table", + translated_name: "Generate Markdown table", + prompt_type: "diff", + icon: "table", + location: ["composer"], + }, + { + id: -305, + name: "custom_prompt", + translated_name: "Custom Prompt", + prompt_type: "diff", + icon: "comment", + location: ["composer", "post"], + }, + { + id: -306, + name: "explain", + translated_name: "Explain", + prompt_type: "text", + icon: "question", + location: ["post"], + }, + { + id: -307, + name: "generate_titles", + translated_name: "Suggest topic titles", + prompt_type: "list", + icon: "heading", + location: ["composer"], + }, + { + id: -308, + name: "illustrate_post", + translated_name: "Illustrate Post", + prompt_type: "list", + icon: "images", + location: ["composer"], + }, + { + id: -309, + name: "detect_text_locale", + translated_name: "detect_text_locale", + prompt_type: "text", + icon: null, + location: [], + }, +]; From 1431672b5cd56d492c93e713298ad3c9812e2c05 Mon Sep 17 00:00:00 2001 From: Keegan George Date: Wed, 4 Dec 2024 11:20:00 -0800 Subject: [PATCH 2/4] DEV: Remove commented out code --- .../acceptance/post-helper-menu-test.js | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/test/javascripts/acceptance/post-helper-menu-test.js b/test/javascripts/acceptance/post-helper-menu-test.js index 612eb5e85..effe8330c 100644 --- a/test/javascripts/acceptance/post-helper-menu-test.js +++ b/test/javascripts/acceptance/post-helper-menu-test.js @@ -52,21 +52,6 @@ acceptance("AI Helper - Post Helper Menu", function (needs) { done: false, }); }); - - // server.post("/posts/398", (request) => { - // return helper.response({ - // post: { - // id: 398, - // name: "Uwe Keim", - // username: "uwe_keim", - // cooked: - // "

Any THIS IS A FOOTNOTE plans to support localization of UI elements, so that I (for example) could set up a completely German speaking forum?

", - // topic_id: 280, - // post_number: 1, - // raw: "Any THIS IS A FOOTNOTE plans to support localization of UI elements, so that I (for example) could set up a completely German speaking forum?", - // }, - // }); - // }); }); test("displays streamed explanation", async function (assert) { From 1c659d41572ff2b6a590b53a96bbb17bfaeab8ef Mon Sep 17 00:00:00 2001 From: Keegan George Date: Wed, 4 Dec 2024 11:25:19 -0800 Subject: [PATCH 3/4] DEV: Hover works better on mobile Hold results in copy/paste menu covering the tooltip, while hover lets the tooltip show on a single tap. --- assets/javascripts/discourse/components/ai-post-helper-menu.gjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/javascripts/discourse/components/ai-post-helper-menu.gjs b/assets/javascripts/discourse/components/ai-post-helper-menu.gjs index 7843e159c..e3efff1fb 100644 --- a/assets/javascripts/discourse/components/ai-post-helper-menu.gjs +++ b/assets/javascripts/discourse/components/ai-post-helper-menu.gjs @@ -62,7 +62,7 @@ export default class AiPostHelperMenu extends Component { "discourse_ai.ai_helper.post_options_menu.footnote_disabled" ), placement: "top", - triggers: this.site.mobileView ? ["hold"] : ["hover"], + triggers: "hover", }); return () => { From 669a6ee7beb44a1bd5d79d4e14437fd3fb28c1dd Mon Sep 17 00:00:00 2001 From: Keegan George Date: Wed, 4 Dec 2024 11:30:56 -0800 Subject: [PATCH 4/4] FIX: Linting --- .../discourse/components/ai-post-helper-menu.gjs | 7 +++---- test/javascripts/acceptance/post-helper-menu-test.js | 10 ++++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/assets/javascripts/discourse/components/ai-post-helper-menu.gjs b/assets/javascripts/discourse/components/ai-post-helper-menu.gjs index e3efff1fb..a51c9efab 100644 --- a/assets/javascripts/discourse/components/ai-post-helper-menu.gjs +++ b/assets/javascripts/discourse/components/ai-post-helper-menu.gjs @@ -4,6 +4,7 @@ import { action } from "@ember/object"; import didInsert from "@ember/render-modifiers/modifiers/did-insert"; import willDestroy from "@ember/render-modifiers/modifiers/will-destroy"; import { service } from "@ember/service"; +import { modifier } from "ember-modifier"; import { and } from "truth-helpers"; import CookText from "discourse/components/cook-text"; import DButton from "discourse/components/d-button"; @@ -18,8 +19,6 @@ import I18n from "discourse-i18n"; import eq from "truth-helpers/helpers/eq"; import AiHelperLoading from "../components/ai-helper-loading"; import AiHelperOptionsList from "../components/ai-helper-options-list"; -import { modifier } from "ember-modifier"; -import { htmlSafe } from "@ember/template"; export default class AiPostHelperMenu extends Component { @service messageBus; @@ -49,8 +48,6 @@ export default class AiPostHelperMenu extends Component { result: "RESULT", }; - @tracked _activeAiRequest = null; - showFootnoteTooltip = modifier((element) => { if (this.supportsAddFootnote || this.streaming) { return; @@ -70,6 +67,8 @@ export default class AiPostHelperMenu extends Component { }; }); + @tracked _activeAiRequest = null; + get footnoteDisabled() { return this.streaming || !this.supportsAddFootnote; } diff --git a/test/javascripts/acceptance/post-helper-menu-test.js b/test/javascripts/acceptance/post-helper-menu-test.js index effe8330c..4b50f03a2 100644 --- a/test/javascripts/acceptance/post-helper-menu-test.js +++ b/test/javascripts/acceptance/post-helper-menu-test.js @@ -1,15 +1,13 @@ -import { click, visit } from "@ember/test-helpers"; +import { click, settled, visit } from "@ember/test-helpers"; import { test } from "qunit"; +import { AUTO_GROUPS } from "discourse/lib/constants"; import topicFixtures from "discourse/tests/fixtures/topic"; -import { settled, focus } from "@ember/test-helpers"; import { acceptance, publishToMessageBus, - updateCurrentUser, - selectText, query, + selectText, } from "discourse/tests/helpers/qunit-helpers"; -import { AUTO_GROUPS } from "discourse/lib/constants"; import { cloneJSON } from "discourse-common/lib/object"; import aiHelperPrompts from "../fixtures/ai-helper-prompts"; @@ -46,7 +44,7 @@ acceptance("AI Helper - Post Helper Menu", function (needs) { return helper.response(json); }); - server.post(`/discourse-ai/ai-helper/stream_suggestion/`, (request) => { + server.post(`/discourse-ai/ai-helper/stream_suggestion/`, () => { return helper.response({ result: "This is a suggestio", done: false,