DEV: Change reply action in the discoveries controller to be a POST and added tests for discobot-discoveries service (#38338)

Updated the action to be a post as it does make side effects, in our
case, it enqueues a job, and updated the client accordingly – I've
checked if there is any other usage of this endpoint in plugins

Also, improved reply action in the client to allow retries when failed,
ensured duplicate requests are not sent, and added their respective
tests.
This commit is contained in:
Gabriel Grubba
2026-03-09 14:50:10 -03:00
committed by GitHub
parent fb19e92f37
commit 55a4534653
4 changed files with 62 additions and 5 deletions
@@ -77,6 +77,10 @@ export default class DiscobotDiscoveries extends Service {
return;
}
if (this.discoveryTimeout) {
cancel(this.discoveryTimeout);
}
this.resetDiscovery();
if (query?.length === 0) {
@@ -95,9 +99,14 @@ export default class DiscobotDiscoveries extends Service {
this.lastQuery = query;
await ajax("/discourse-ai/discoveries/reply", {
type: "POST",
data: { query },
});
} catch {
if (this.lastQuery === query) {
this.lastQuery = "";
}
this.timeoutDiscovery();
}
}
+1 -1
View File
@@ -32,7 +32,7 @@ DiscourseAi::Engine.routes.draw do
end
scope module: :discover, path: "/discoveries", defaults: { format: :json } do
get "reply" => "discoveries#reply"
post "reply" => "discoveries#reply"
post "continue-convo" => "discoveries#continue_convo"
end
@@ -17,7 +17,7 @@ RSpec.describe DiscourseAi::Discover::DiscoveriesController do
context "when the user doesn't have access to the persona" do
it "returns a 403" do
get "/discourse-ai/discoveries/reply", params: { query: "What is Discourse?" }
post "/discourse-ai/discoveries/reply", params: { query: "What is Discourse?" }
expect(response.status).to eq(403)
end
@@ -31,14 +31,14 @@ RSpec.describe DiscourseAi::Discover::DiscoveriesController do
it "returns a 200 and queues a job to reply" do
expect {
get "/discourse-ai/discoveries/reply", params: { query: "What is Discourse?" }
post "/discourse-ai/discoveries/reply", params: { query: "What is Discourse?" }
}.to change(Jobs::StreamDiscoverReply.jobs, :size).by(1)
expect(response.status).to eq(200)
end
it "retues a 400 if the query is missing" do
get "/discourse-ai/discoveries/reply"
it "returns a 400 if the query is missing" do
post "/discourse-ai/discoveries/reply"
expect(response.status).to eq(400)
end
@@ -0,0 +1,48 @@
import { getOwner } from "@ember/owner";
import { cancel } from "@ember/runloop";
import { setupTest } from "ember-qunit";
import { module, test } from "qunit";
import pretender, { response } from "discourse/tests/helpers/create-pretender";
module("Unit | Service | discobot-discoveries", function (hooks) {
setupTest(hooks);
hooks.afterEach(function () {
const service = getOwner(this).lookup("service:discobot-discoveries");
cancel(service.discoveryTimeout);
});
test("does not send duplicate requests for the same successful query", async function (assert) {
let requestsCount = 0;
pretender.post("/discourse-ai/discoveries/reply", () => {
requestsCount += 1;
return response(200, {});
});
const service = getOwner(this).lookup("service:discobot-discoveries");
await service.triggerDiscovery("What is Discourse?");
await service.triggerDiscovery("What is Discourse?");
cancel(service.discoveryTimeout);
assert.strictEqual(requestsCount, 1);
});
test("allows retrying the same query when the request fails", async function (assert) {
let requestsCount = 0;
pretender.post("/discourse-ai/discoveries/reply", () => {
requestsCount += 1;
return response(500, {});
});
const service = getOwner(this).lookup("service:discobot-discoveries");
await service.triggerDiscovery("What is Discourse?");
await service.triggerDiscovery("What is Discourse?");
cancel(service.discoveryTimeout);
assert.strictEqual(requestsCount, 2);
});
});