diff --git a/app/assets/javascripts/discourse/app/lib/text.js b/app/assets/javascripts/discourse/app/lib/text.js index f83d5069131..ea40982c6c3 100644 --- a/app/assets/javascripts/discourse/app/lib/text.js +++ b/app/assets/javascripts/discourse/app/lib/text.js @@ -110,3 +110,53 @@ export function emojiUrlFor(code) { return buildEmojiUrl(code, opts); } } + +function encode(str) { + return str.replaceAll("<", "<").replaceAll(">", ">"); +} + +function traverse(element, callback) { + if (callback(element)) { + element.childNodes.forEach((child) => traverse(child, callback)); + } +} + +export function excerpt(cooked, length) { + let result = ""; + let resultLength = 0; + + const div = document.createElement("div"); + div.innerHTML = cooked; + traverse(div, (element) => { + if (resultLength >= length) { + return; + } + + if (element.nodeType === Node.TEXT_NODE) { + if (resultLength + element.textContent.length > length) { + const text = element.textContent.substr(0, length - resultLength); + result += encode(text); + result += "…"; + resultLength += text.length; + } else { + result += encode(element.textContent); + resultLength += element.textContent.length; + } + } else if (element.tagName === "A") { + element.innerHTML = element.innerText; + result += element.outerHTML; + resultLength += element.innerText.length; + } else if (element.tagName === "IMG") { + if (element.classList.contains("emoji")) { + result += element.outerHTML; + } else { + result += "[image]"; + resultLength += "[image]".length; + } + } else { + return true; + } + }); + + return result; +} diff --git a/app/assets/javascripts/discourse/app/models/draft.js b/app/assets/javascripts/discourse/app/models/draft.js index 37dda0ba765..4275cab1630 100644 --- a/app/assets/javascripts/discourse/app/models/draft.js +++ b/app/assets/javascripts/discourse/app/models/draft.js @@ -5,17 +5,14 @@ const Draft = EmberObject.extend(); Draft.reopenClass({ clear(key, sequence) { - return ajax("/draft.json", { + return ajax(`/drafts/${key}.json`, { type: "DELETE", data: { draft_key: key, sequence }, }); }, get(key) { - return ajax("/draft.json", { - data: { draft_key: key }, - dataType: "json", - }); + return ajax(`/drafts/${key}.json`); }, getLocal(key, current) { @@ -25,7 +22,7 @@ Draft.reopenClass({ save(key, sequence, data, clientId, { forceSave = false } = {}) { data = typeof data === "string" ? data : JSON.stringify(data); - return ajax("/draft.json", { + return ajax("/drafts.json", { type: "POST", data: { draft_key: key, diff --git a/app/assets/javascripts/discourse/app/models/user-drafts-stream.js b/app/assets/javascripts/discourse/app/models/user-drafts-stream.js index e703e5acb8a..00d6f04c7c6 100644 --- a/app/assets/javascripts/discourse/app/models/user-drafts-stream.js +++ b/app/assets/javascripts/discourse/app/models/user-drafts-stream.js @@ -1,105 +1,100 @@ +import discourseComputed from "discourse-common/utils/decorators"; +import { ajax } from "discourse/lib/ajax"; +import { cookAsync, emojiUnescape, excerpt } from "discourse/lib/text"; +import { escapeExpression } from "discourse/lib/utilities"; import { NEW_PRIVATE_MESSAGE_KEY, NEW_TOPIC_KEY, } from "discourse/models/composer"; -import { A } from "@ember/array"; -import { Promise } from "rsvp"; import RestModel from "discourse/models/rest"; import UserDraft from "discourse/models/user-draft"; -import { ajax } from "discourse/lib/ajax"; -import discourseComputed from "discourse-common/utils/decorators"; -import { emojiUnescape } from "discourse/lib/text"; -import { escapeExpression } from "discourse/lib/utilities"; -import { url } from "discourse/lib/computed"; +import { Promise } from "rsvp"; export default RestModel.extend({ - loaded: false, + limit: 30, + + loading: false, + hasMore: false, + content: null, init() { this._super(...arguments); + this.reset(); + }, + + reset() { this.setProperties({ - itemsLoaded: 0, + loading: false, + hasMore: true, content: [], - lastLoadedUrl: null, }); }, - baseUrl: url( - "itemsLoaded", - "user.username_lower", - "/drafts.json?offset=%@&username=%@" - ), - - load(site) { - this.setProperties({ - itemsLoaded: 0, - content: [], - lastLoadedUrl: null, - site: site, - }); - return this.findItems(); - }, - - @discourseComputed("content.length", "loaded") - noContent(contentLength, loaded) { - return loaded && contentLength === 0; + @discourseComputed("content.length", "loading") + noContent(contentLength, loading) { + return contentLength === 0 && !loading; }, remove(draft) { - let content = this.content.filter( - (item) => item.draft_key !== draft.draft_key + this.set( + "content", + this.content.filter((item) => item.draft_key !== draft.draft_key) ); - this.setProperties({ content, itemsLoaded: content.length }); }, - findItems() { - let findUrl = this.baseUrl; - - const lastLoadedUrl = this.lastLoadedUrl; - if (lastLoadedUrl === findUrl) { - return Promise.resolve(); + findItems(site) { + if (site) { + this.set("site", site); } - if (this.loading) { + if (this.loading || !this.hasMore) { return Promise.resolve(); } this.set("loading", true); - return ajax(findUrl) + const url = `/drafts.json?offset=${this.content.length}&limit=${this.limit}`; + return ajax(url) .then((result) => { - if (result && result.no_results_help) { + if (!result) { + return; + } + + if (result.no_results_help) { this.set("noContentHelp", result.no_results_help); } - if (result && result.drafts) { - const copy = A(); - result.drafts.forEach((draft) => { - let draftData = JSON.parse(draft.data); - draft.post_number = draftData.postId || null; + + if (!result.drafts) { + return; + } + + this.set("hasMore", result.drafts.size >= this.limit); + + const promises = result.drafts.map((draft) => { + draft.data = JSON.parse(draft.data); + return cookAsync(draft.data.reply).then((cooked) => { + draft.excerpt = excerpt(cooked.string, 300); + draft.post_number = draft.data.postId || null; if ( draft.draft_key === NEW_PRIVATE_MESSAGE_KEY || draft.draft_key === NEW_TOPIC_KEY ) { - draft.title = draftData.title; + draft.title = draft.data.title; } draft.title = emojiUnescape(escapeExpression(draft.title)); - if (draft.category_id) { + if (draft.data.categoryId) { draft.category = - this.site.categories.findBy("id", draft.category_id) || null; + this.site.categories.findBy("id", draft.data.categoryId) || + null; } + this.content.push(UserDraft.create(draft)); + }); + }); - copy.pushObject(UserDraft.create(draft)); - }); - this.content.pushObjects(copy); - this.setProperties({ - loaded: true, - itemsLoaded: this.itemsLoaded + result.drafts.length, - }); - } + return Promise.all(promises); }) .finally(() => { this.set("loading", false); - this.set("lastLoadedUrl", findUrl); }); }, }); diff --git a/app/assets/javascripts/discourse/app/routes/user-activity-drafts.js b/app/assets/javascripts/discourse/app/routes/user-activity-drafts.js index 840015b7b92..1116dee350d 100644 --- a/app/assets/javascripts/discourse/app/routes/user-activity-drafts.js +++ b/app/assets/javascripts/discourse/app/routes/user-activity-drafts.js @@ -2,8 +2,9 @@ import DiscourseRoute from "discourse/routes/discourse"; export default DiscourseRoute.extend({ model() { - let userDraftsStream = this.modelFor("user").get("userDraftsStream"); - return userDraftsStream.load(this.site).then(() => userDraftsStream); + const model = this.modelFor("user").get("userDraftsStream"); + model.reset(); + return model.findItems(this.site).then(() => model); }, renderTemplate() { diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-draft-saving-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-draft-saving-test.js index 44f6b4d87f9..22676e94145 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/composer-draft-saving-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-draft-saving-test.js @@ -13,7 +13,7 @@ acceptance("Composer - Draft saving", function (needs) { const draftThatWillBeSaved = "This_will_be_saved_successfully"; needs.pretender((server, helper) => { - server.post("/draft.json", (request) => { + server.post("/drafts.json", (request) => { const success = request.requestBody.includes(draftThatWillBeSaved); return success ? helper.response({ success: true }) diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-edit-conflict-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-edit-conflict-test.js index 96f970237a0..841545e9e4e 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/composer-edit-conflict-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-edit-conflict-test.js @@ -8,7 +8,7 @@ acceptance("Composer - Edit conflict", function (needs) { let lastBody; needs.pretender((server, helper) => { - server.post("/draft.json", (request) => { + server.post("/drafts.json", (request) => { lastBody = request.requestBody; return helper.response({ success: true }); }); diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-drafts-stream-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-drafts-stream-test.js index 1b2aa090f5b..c762e7ddce5 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-drafts-stream-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-drafts-stream-test.js @@ -2,6 +2,7 @@ import { acceptance, count, exists, + query, queryAll, visible, } from "discourse/tests/helpers/qunit-helpers"; @@ -36,4 +37,17 @@ acceptance("User Drafts", function (needs) { "A fun new topic for testing drafts." ); }); + + test("Stream - has excerpt", async function (assert) { + await visit("/u/eviltrout/activity/drafts"); + assert.ok(exists(".user-stream-item"), "has drafts"); + assert.equal( + query(".user-stream-item:nth-child(3) .category").textContent, + "meta" + ); + assert.equal( + query(".user-stream-item:nth-child(3) .excerpt").innerHTML.trim(), + 'here goes a reply to a PM :slight_smile:' + ); + }); }); diff --git a/app/assets/javascripts/discourse/tests/fixtures/draft.js b/app/assets/javascripts/discourse/tests/fixtures/draft.js deleted file mode 100644 index a1d5ce6b9cc..00000000000 --- a/app/assets/javascripts/discourse/tests/fixtures/draft.js +++ /dev/null @@ -1,21 +0,0 @@ -export default { - "/draft.json": { - draft: null, - draft_sequence: 0 - }, - "/draft.json?draft_key=topic_280": { - draft: - '{"reply":"This is a draft of the first post","action":"reply","categoryId":1,"archetypeId":"regular","metaData":null,"composerTime":2863,"typingTime":200}', - draft_sequence: 42 - }, - "/draft.json?draft_key=topic_281": { - draft: - '{"reply":"dum de dum da ba.","action":"createTopic","title":"dum da ba dum dum","categoryId":null,"archetypeId":"regular","metaData":null,"composerTime":540879,"typingTime":3400}', - draft_sequence: 0 - }, - "/draft.json?draft_key=topic_9": { - draft: - '{"reply":"This is a draft of the first post","action":"reply","categoryId":1,"archetypeId":"regular","metaData":null,"composerTime":2863,"typingTime":200}', - draft_sequence: 42 - } -}; diff --git a/app/assets/javascripts/discourse/tests/fixtures/drafts.js b/app/assets/javascripts/discourse/tests/fixtures/drafts.js index f5f31958f6e..31f33ace696 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/drafts.js +++ b/app/assets/javascripts/discourse/tests/fixtures/drafts.js @@ -47,7 +47,7 @@ export default { draft_username: "eviltrout", avatar_template: "/user_avatar/localhost/eviltrout/{size}/2_1.png", data: - '{"reply":"here goes a reply to a PM.","action":"reply","categoryId":null,"postId":212,"archetypeId":"regular","whisper":false,"metaData":null,"composerTime":455711,"typingTime":5400}', + '{"reply":"here goes a reply to a PM :slight_smile:","action":"reply","categoryId":3,"postId":212,"archetypeId":"regular","whisper":false,"metaData":null,"composerTime":455711,"typingTime":5400}', topic_id: 93, username: "eviltrout", name: null, @@ -57,5 +57,20 @@ export default { archetype: "private_message" } ] + }, + "/drafts/topic_280.json": { + draft: + '{"reply":"This is a draft of the first post","action":"reply","categoryId":1,"archetypeId":"regular","metaData":null,"composerTime":2863,"typingTime":200}', + draft_sequence: 42 + }, + "/drafts/topic_281.json": { + draft: + '{"reply":"dum de dum da ba.","action":"createTopic","title":"dum da ba dum dum","categoryId":null,"archetypeId":"regular","metaData":null,"composerTime":540879,"typingTime":3400}', + draft_sequence: 0 + }, + "/drafts/topic_9.json": { + draft: + '{"reply":"This is a draft of the first post","action":"reply","categoryId":1,"archetypeId":"regular","metaData":null,"composerTime":2863,"typingTime":200}', + draft_sequence: 42 } }; diff --git a/app/assets/javascripts/discourse/tests/helpers/create-pretender.js b/app/assets/javascripts/discourse/tests/helpers/create-pretender.js index 96ae6e91b5c..fd1df4131b8 100644 --- a/app/assets/javascripts/discourse/tests/helpers/create-pretender.js +++ b/app/assets/javascripts/discourse/tests/helpers/create-pretender.js @@ -291,8 +291,8 @@ export function applyDefaultHandlers(pretender) { pretender.get("/permalink-check.json", () => response({ found: false })); - pretender.delete("/draft.json", success); - pretender.post("/draft.json", success); + pretender.delete("/drafts/:draft_key.json", success); + pretender.post("/drafts.json", success); pretender.get("/u/:username/staff-info.json", () => response({})); @@ -358,19 +358,11 @@ export function applyDefaultHandlers(pretender) { response(fixturesByUrl["/c/11/show.json"]) ); - pretender.get("/draft.json", (request) => { - if (request.queryParams.draft_key === "new_topic") { - return response(fixturesByUrl["/draft.json"]); - } else if (request.queryParams.draft_key.startsWith("topic_")) { - return response( - fixturesByUrl[request.url] || { - draft: null, - draft_sequence: 0, - } - ); - } - return response({}); - }); + pretender.get("/drafts.json", () => response(fixturesByUrl["/drafts.json"])); + + pretender.get("/drafts/:draft_key.json", (request) => + response(fixturesByUrl[request.url] || { draft: null, draft_sequence: 0 }) + ); pretender.get("/drafts.json", () => response(fixturesByUrl["/drafts.json"])); diff --git a/app/assets/javascripts/discourse/tests/unit/models/user-drafts-test.js b/app/assets/javascripts/discourse/tests/unit/models/user-drafts-test.js index 118089d6446..55d7e781eba 100644 --- a/app/assets/javascripts/discourse/tests/unit/models/user-drafts-test.js +++ b/app/assets/javascripts/discourse/tests/unit/models/user-drafts-test.js @@ -7,14 +7,10 @@ import UserDraft from "discourse/models/user-draft"; module("Unit | Model | user-draft", function () { test("stream", function (assert) { const user = User.create({ id: 1, username: "eviltrout" }); - const stream = user.get("userDraftsStream"); + const stream = user.userDraftsStream; assert.present(stream, "a user has a drafts stream by default"); - assert.equal( - stream.get("itemsLoaded"), - 0, - "no items are loaded by default" - ); - assert.blank(stream.get("content"), "no content by default"); + assert.equal(stream.content.length, 0, "no items are loaded by default"); + assert.blank(stream.content, "no content by default"); }); test("draft", function (assert) { @@ -30,7 +26,7 @@ module("Unit | Model | user-draft", function () { assert.equal(drafts.length, 2, "drafts count is right"); assert.equal( - drafts[1].get("draftType"), + drafts[1].draftType, I18n.t("drafts.new_topic"), "loads correct draftType label" ); diff --git a/app/controllers/draft_controller.rb b/app/controllers/draft_controller.rb deleted file mode 100644 index 3cc77a32914..00000000000 --- a/app/controllers/draft_controller.rb +++ /dev/null @@ -1,86 +0,0 @@ -# frozen_string_literal: true - -class DraftController < ApplicationController - requires_login - - skip_before_action :check_xhr, :preload_json - - def show - raise Discourse::NotFound.new if params[:draft_key].blank? - - seq = params[:sequence] || DraftSequence.current(current_user, params[:draft_key]) - render json: { draft: Draft.get(current_user, params[:draft_key], seq), draft_sequence: seq } - end - - def update - raise Discourse::NotFound.new if params[:draft_key].blank? - - sequence = - begin - Draft.set( - current_user, - params[:draft_key], - params[:sequence].to_i, - params[:data], - params[:owner], - force_save: params[:force_save] - ) - rescue Draft::OutOfSequence - - begin - if !Draft.exists?(user_id: current_user.id, draft_key: params[:draft_key]) - Draft.set( - current_user, - params[:draft_key], - DraftSequence.current(current_user, params[:draft_key]), - params[:data], - params[:owner] - ) - else - raise Draft::OutOfSequence - end - - rescue Draft::OutOfSequence - render_json_error I18n.t('draft.sequence_conflict_error.title'), - status: 409, - extras: { - description: I18n.t('draft.sequence_conflict_error.description') - } - return - end - end - - json = success_json.merge(draft_sequence: sequence) - - begin - data = JSON::parse(params[:data]) - rescue JSON::ParserError - raise Discourse::InvalidParameters.new(:data) - end - - if data.present? - # this is a bit of a kludge we need to remove (all the parsing) too many special cases here - # we need to catch action edit and action editSharedDraft - if data["postId"].present? && data["originalText"].present? && data["action"].to_s.start_with?("edit") - post = Post.find_by(id: data["postId"]) - if post && post.raw != data["originalText"] - conflict_user = BasicUserSerializer.new(post.last_editor, root: false) - render json: json.merge(conflict_user: conflict_user) - return - end - end - end - - render json: json - end - - def destroy - begin - Draft.clear(current_user, params[:draft_key], params[:sequence].to_i) - rescue Draft::OutOfSequence - # nothing really we can do here, if try clearing a draft that is not ours, just skip it. - end - render json: success_json - end - -end diff --git a/app/controllers/drafts_controller.rb b/app/controllers/drafts_controller.rb index db6f87782ab..8ed7e539b3d 100644 --- a/app/controllers/drafts_controller.rb +++ b/app/controllers/drafts_controller.rb @@ -6,29 +6,96 @@ class DraftsController < ApplicationController skip_before_action :check_xhr, :preload_json def index - params.require(:username) params.permit(:offset) params.permit(:limit) - user = fetch_user_from_params - - unless user == current_user - raise Discourse::InvalidAccess - end - - opts = { - user: user, + stream = Draft.stream( + user: current_user, offset: params[:offset], limit: params[:limit] - } - - stream = Draft.stream(opts) + ) render json: { drafts: stream ? serialize_data(stream, DraftSerializer) : [], no_results_help: I18n.t("user_activity.no_drafts.self") } - end + def show + raise Discourse::NotFound.new if params[:id].blank? + + seq = params[:sequence] || DraftSequence.current(current_user, params[:id]) + render json: { draft: Draft.get(current_user, params[:id], seq), draft_sequence: seq } + end + + def create + raise Discourse::NotFound.new if params[:draft_key].blank? + + sequence = + begin + Draft.set( + current_user, + params[:draft_key], + params[:sequence].to_i, + params[:data], + params[:owner], + force_save: params[:force_save] + ) + rescue Draft::OutOfSequence + + begin + if !Draft.exists?(user_id: current_user.id, draft_key: params[:draft_key]) + Draft.set( + current_user, + params[:draft_key], + DraftSequence.current(current_user, params[:draft_key]), + params[:data], + params[:owner] + ) + else + raise Draft::OutOfSequence + end + + rescue Draft::OutOfSequence + render_json_error I18n.t('draft.sequence_conflict_error.title'), + status: 409, + extras: { + description: I18n.t('draft.sequence_conflict_error.description') + } + return + end + end + + json = success_json.merge(draft_sequence: sequence) + + begin + data = JSON::parse(params[:data]) + rescue JSON::ParserError + raise Discourse::InvalidParameters.new(:data) + end + + if data.present? + # this is a bit of a kludge we need to remove (all the parsing) too many special cases here + # we need to catch action edit and action editSharedDraft + if data["postId"].present? && data["originalText"].present? && data["action"].to_s.start_with?("edit") + post = Post.find_by(id: data["postId"]) + if post && post.raw != data["originalText"] + conflict_user = BasicUserSerializer.new(post.last_editor, root: false) + render json: json.merge(conflict_user: conflict_user) + return + end + end + end + + render json: json + end + + def destroy + begin + Draft.clear(current_user, params[:id], params[:sequence].to_i) + rescue Draft::OutOfSequence + # nothing really we can do here, if try clearing a draft that is not ours, just skip it. + end + render json: success_json + end end diff --git a/app/models/draft.rb b/app/models/draft.rb index 924c987edcf..459ddea41c7 100644 --- a/app/models/draft.rb +++ b/app/models/draft.rb @@ -161,7 +161,11 @@ class Draft < ActiveRecord::Base end def parsed_data - JSON.parse(data) + begin + JSON.parse(data) + rescue JSON::ParserError + {} + end end def topic_id diff --git a/config/routes.rb b/config/routes.rb index 63b1498aed4..5a4b246d1a4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -887,10 +887,7 @@ Discourse::Application.routes.draw do get "message-bus/poll" => "message_bus#poll" - resources :drafts, only: [:index] - get "draft" => "draft#show" - post "draft" => "draft#update" - delete "draft" => "draft#destroy" + resources :drafts, only: [:index, :create, :show, :destroy] if service_worker_asset = Rails.application.assets_manifest.assets['service-worker.js'] # https://developers.google.com/web/fundamentals/codelabs/debugging-service-workers/ diff --git a/spec/requests/draft_controller_spec.rb b/spec/requests/draft_controller_spec.rb deleted file mode 100644 index f828e6d89de..00000000000 --- a/spec/requests/draft_controller_spec.rb +++ /dev/null @@ -1,175 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe DraftController do - it 'requires you to be logged in' do - post "/draft" - expect(response.status).to eq(403) - end - - it 'saves a draft on update' do - user = sign_in(Fabricate(:user)) - - post "/draft.json", params: { - draft_key: 'xyz', - data: { my: "data" }.to_json, - sequence: 0 - } - - expect(response.status).to eq(200) - expect(Draft.get(user, 'xyz', 0)).to eq(%q({"my":"data"})) - end - - it "returns 404 when the key is missing" do - _user = sign_in(Fabricate(:user)) - post "/draft.json", params: { data: { my: "data" }.to_json, sequence: 0 } - expect(response.status).to eq(404) - end - - it "returns a draft if requested" do - user = sign_in(Fabricate(:user)) - Draft.set(user, 'hello', 0, 'test') - - get "/draft.json", params: { draft_key: 'hello' } - expect(response.status).to eq(200) - json = response.parsed_body - expect(json['draft']).to eq('test') - - get "/draft.json" - expect(response.status).to eq(404) - end - - it 'checks for an conflict on update' do - user = sign_in(Fabricate(:user)) - post = Fabricate(:post, user: user) - - post "/draft.json", params: { - draft_key: "topic", - sequence: 0, - data: { - postId: post.id, - originalText: post.raw, - action: "edit" - }.to_json - } - - expect(response.parsed_body['conflict_user']).to eq(nil) - - post "/draft.json", params: { - draft_key: "topic", - sequence: 0, - data: { - postId: post.id, - originalText: "something else", - action: "edit" - }.to_json - } - - json = response.parsed_body - - expect(json['conflict_user']['id']).to eq(post.last_editor.id) - expect(json['conflict_user']).to include('avatar_template') - end - - it 'destroys drafts when required' do - user = sign_in(Fabricate(:user)) - Draft.set(user, 'xxx', 0, 'hi') - delete "/draft.json", params: { draft_key: 'xxx', sequence: 0 } - expect(response.status).to eq(200) - expect(Draft.get(user, 'xxx', 0)).to eq(nil) - end - - it 'cant trivially resolve conflicts without interaction' do - - user = sign_in(Fabricate(:user)) - - DraftSequence.next!(user, "abc") - - post "/draft.json", params: { - draft_key: "abc", - sequence: 0, - data: { a: "test" }.to_json, - owner: "abcdefg" - } - - expect(response.status).to eq(200) - json = response.parsed_body - expect(json["draft_sequence"]).to eq(1) - end - - it 'has a clean protocol for ownership handover' do - user = sign_in(Fabricate(:user)) - - post "/draft.json", params: { - draft_key: "abc", - sequence: 0, - data: { a: "test" }.to_json, - owner: "abcdefg" - } - - expect(response.status).to eq(200) - - json = response.parsed_body - expect(json["draft_sequence"]).to eq(0) - - post "/draft.json", params: { - draft_key: "abc", - sequence: 0, - data: { b: "test" }.to_json, - owner: "hijklmnop" - } - - expect(response.status).to eq(200) - json = response.parsed_body - expect(json["draft_sequence"]).to eq(1) - - expect(DraftSequence.current(user, "abc")).to eq(1) - - post "/draft.json", params: { - draft_key: "abc", - sequence: 1, - data: { c: "test" }.to_json, - owner: "hijklmnop" - } - - expect(response.status).to eq(200) - json = response.parsed_body - expect(json["draft_sequence"]).to eq(2) - - post "/draft.json", params: { - draft_key: "abc", - sequence: 2, - data: { c: "test" }.to_json, - owner: "abc" - } - - expect(response.status).to eq(200) - json = response.parsed_body - expect(json["draft_sequence"]).to eq(3) - end - - it 'raises an error for out-of-sequence draft setting' do - - user = sign_in(Fabricate(:user)) - seq = DraftSequence.next!(user, "abc") - Draft.set(user, "abc", seq, { b: "test" }.to_json) - - post "/draft.json", params: { - draft_key: "abc", - sequence: seq - 1, - data: { a: "test" }.to_json - } - - expect(response.status).to eq(409) - - post "/draft.json", params: { - draft_key: "abc", - sequence: seq + 1, - data: { a: "test" }.to_json - } - - expect(response.status).to eq(409) - - end -end diff --git a/spec/requests/drafts_controller_spec.rb b/spec/requests/drafts_controller_spec.rb index 0ce0be86638..3afa76127ef 100644 --- a/spec/requests/drafts_controller_spec.rb +++ b/spec/requests/drafts_controller_spec.rb @@ -3,55 +3,210 @@ require 'rails_helper' describe DraftsController do - it 'requires you to be logged in' do - get "/drafts.json" - expect(response.status).to eq(403) + describe "#index" do + it 'requires you to be logged in' do + get "/drafts.json" + expect(response.status).to eq(403) + end + + it 'returns correct stream length after adding a draft' do + user = sign_in(Fabricate(:user)) + Draft.set(user, 'xxx', 0, '{}') + get "/drafts.json" + expect(response.status).to eq(200) + parsed = response.parsed_body + expect(response.parsed_body["drafts"].length).to eq(1) + end + + it 'has empty stream after deleting last draft' do + user = sign_in(Fabricate(:user)) + Draft.set(user, 'xxx', 0, '{}') + Draft.clear(user, 'xxx', 0) + get "/drafts.json" + expect(response.status).to eq(200) + expect(response.parsed_body["drafts"].length).to eq(0) + end + + it 'does not include topic details when user cannot see topic' do + topic = Fabricate(:private_message_topic) + topic_user = topic.user + other_user = Fabricate(:user) + Draft.set(topic_user, "topic_#{topic.id}", 0, '{}') + Draft.set(other_user, "topic_#{topic.id}", 0, '{}') + + sign_in(topic_user) + get "/drafts.json" + expect(response.status).to eq(200) + expect(response.parsed_body["drafts"].first["title"]).to eq(topic.title) + + sign_in(other_user) + get "/drafts.json" + expect(response.status).to eq(200) + expect(response.parsed_body["drafts"].first["title"]).to eq(nil) + end end - it 'returns correct stream length after adding a draft' do - user = sign_in(Fabricate(:user)) - Draft.set(user, 'xxx', 0, '{}') - get "/drafts.json", params: { username: user.username } - expect(response.status).to eq(200) - parsed = response.parsed_body - expect(parsed["drafts"].length).to eq(1) + describe "#show" do + it "returns a draft if requested" do + user = sign_in(Fabricate(:user)) + Draft.set(user, 'hello', 0, 'test') + + get "/drafts/hello.json" + expect(response.status).to eq(200) + expect(response.parsed_body['draft']).to eq('test') + end end - it 'has empty stream after deleting last draft' do - user = sign_in(Fabricate(:user)) - Draft.set(user, 'xxx', 0, '{}') - Draft.clear(user, 'xxx', 0) - get "/drafts.json", params: { username: user.username } - expect(response.status).to eq(200) - parsed = response.parsed_body - expect(parsed["drafts"].length).to eq(0) + describe "#create" do + it 'requires you to be logged in' do + post "/drafts.json" + expect(response.status).to eq(403) + end + + it 'saves a draft' do + user = sign_in(Fabricate(:user)) + + post "/drafts.json", params: { + draft_key: 'xyz', + data: { my: "data" }.to_json, + sequence: 0 + } + + expect(response.status).to eq(200) + expect(Draft.get(user, 'xyz', 0)).to eq(%q({"my":"data"})) + end + + it "returns 404 when the key is missing" do + sign_in(Fabricate(:user)) + post "/drafts.json", params: { data: { my: "data" }.to_json, sequence: 0 } + expect(response.status).to eq(404) + end + + it 'checks for an conflict on update' do + user = sign_in(Fabricate(:user)) + post = Fabricate(:post, user: user) + + post "/drafts.json", params: { + draft_key: "topic", + sequence: 0, + data: { + postId: post.id, + originalText: post.raw, + action: "edit" + }.to_json + } + + expect(response.status).to eq(200) + expect(response.parsed_body['conflict_user']).to eq(nil) + + post "/drafts.json", params: { + draft_key: "topic", + sequence: 0, + data: { + postId: post.id, + originalText: "something else", + action: "edit" + }.to_json + } + + expect(response.status).to eq(200) + expect(response.parsed_body['conflict_user']['id']).to eq(post.last_editor.id) + expect(response.parsed_body['conflict_user']).to include('avatar_template') + end + + it 'cant trivially resolve conflicts without interaction' do + + user = sign_in(Fabricate(:user)) + + DraftSequence.next!(user, "abc") + + post "/drafts.json", params: { + draft_key: "abc", + sequence: 0, + data: { a: "test" }.to_json, + owner: "abcdefg" + } + + expect(response.status).to eq(200) + expect(response.parsed_body["draft_sequence"]).to eq(1) + end + + it 'has a clean protocol for ownership handover' do + user = sign_in(Fabricate(:user)) + + post "/drafts.json", params: { + draft_key: "abc", + sequence: 0, + data: { a: "test" }.to_json, + owner: "abcdefg" + } + + expect(response.status).to eq(200) + expect(response.parsed_body["draft_sequence"]).to eq(0) + + post "/drafts.json", params: { + draft_key: "abc", + sequence: 0, + data: { b: "test" }.to_json, + owner: "hijklmnop" + } + + expect(response.status).to eq(200) + expect(response.parsed_body["draft_sequence"]).to eq(1) + + expect(DraftSequence.current(user, "abc")).to eq(1) + + post "/drafts.json", params: { + draft_key: "abc", + sequence: 1, + data: { c: "test" }.to_json, + owner: "hijklmnop" + } + + expect(response.status).to eq(200) + expect(response.parsed_body["draft_sequence"]).to eq(2) + + post "/drafts.json", params: { + draft_key: "abc", + sequence: 2, + data: { c: "test" }.to_json, + owner: "abc" + } + + expect(response.status).to eq(200) + expect(response.parsed_body["draft_sequence"]).to eq(3) + end + + it 'raises an error for out-of-sequence draft setting' do + user = sign_in(Fabricate(:user)) + seq = DraftSequence.next!(user, "abc") + Draft.set(user, "abc", seq, { b: "test" }.to_json) + + post "/drafts.json", params: { + draft_key: "abc", + sequence: seq - 1, + data: { a: "test" }.to_json + } + + expect(response.status).to eq(409) + + post "/drafts.json", params: { + draft_key: "abc", + sequence: seq + 1, + data: { a: "test" }.to_json + } + + expect(response.status).to eq(409) + end end - it 'does not let a user see drafts stream of another user' do - user_b = Fabricate(:user) - Draft.set(user_b, 'xxx', 0, '{}') - sign_in(Fabricate(:user)) - get "/drafts.json", params: { username: user_b.username } - expect(response.status).to eq(403) - end - - it 'does not include topic details when user cannot see topic' do - topic = Fabricate(:private_message_topic) - topic_user = topic.user - other_user = Fabricate(:user) - Draft.set(topic_user, "topic_#{topic.id}", 0, '{}') - Draft.set(other_user, "topic_#{topic.id}", 0, '{}') - - sign_in(topic_user) - get "/drafts.json", params: { username: topic_user.username } - expect(response.status).to eq(200) - parsed = response.parsed_body - expect(parsed["drafts"].first["title"]).to eq(topic.title) - - sign_in(other_user) - get "/drafts.json", params: { username: other_user.username } - expect(response.status).to eq(200) - parsed = response.parsed_body - expect(parsed["drafts"].first["title"]).to eq(nil) + describe "#destroy" do + it 'destroys drafts when required' do + user = sign_in(Fabricate(:user)) + Draft.set(user, 'xxx', 0, 'hi') + delete "/drafts/xxx.json", params: { sequence: 0 } + expect(response.status).to eq(200) + expect(Draft.get(user, 'xxx', 0)).to eq(nil) + end end end diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index 74fce7879a6..18042a9efa2 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -478,7 +478,7 @@ RSpec.describe SessionController do UserAuthToken.destroy_all # we need a route that will call current user - post '/draft.json', params: {} + post '/drafts.json', params: {} expect(response.headers['Discourse-Logged-Out']).to eq("1") end end