FEATURE: Cook drafts excerpt in user activity (#14315)

The previous excerpt was a simple truncated raw message. Starting with
this commit, the raw content of the draft is cooked and an excerpt is
extracted from it. The logic for extracting the excerpt mimics the the
`ExcerptParser` class, but does not implement all functionality, being
a much simpler implementation.

The two draft controllers have been merged into one and the /draft.json
route has been changed to /drafts.json to be consistent with the other
route names.

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("<", "&lt;").replaceAll(">", "&gt;");
+}
+
+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 += "&hellip;";
+        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;
+}

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", {
+    return ajax(`/drafts/${key}.json`);
-      data: { draft_key: key },
-      dataType: "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,

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 { Promise } from "rsvp";
-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";
 
 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(
+  @discourseComputed("content.length", "loading")
-    "itemsLoaded",
+  noContent(contentLength, loading) {
-    "user.username_lower",
+    return contentLength === 0 && !loading;
-    "/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;
   },
 
   remove(draft) {
-    let content = this.content.filter(
+    this.set(
-      (item) => item.draft_key !== draft.draft_key
+      "content",
+      this.content.filter((item) => item.draft_key !== draft.draft_key)
     );
-    this.setProperties({ content, itemsLoaded: content.length });
   },
 
-  findItems() {
+  findItems(site) {
-    let findUrl = this.baseUrl;
+    if (site) {
-
+      this.set("site", site);
-    const lastLoadedUrl = this.lastLoadedUrl;
-    if (lastLoadedUrl === findUrl) {
-      return Promise.resolve();
     }
 
-    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();
+        if (!result.drafts) {
-          result.drafts.forEach((draft) => {
+          return;
-            let draftData = JSON.parse(draft.data);
+        }
-            draft.post_number = draftData.postId || null;
+
+        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));
+        return Promise.all(promises);
-          });
-          this.content.pushObjects(copy);
-          this.setProperties({
-            loaded: true,
-            itemsLoaded: this.itemsLoaded + result.drafts.length,
-          });
-        }
       })
       .finally(() => {
         this.set("loading", false);
-        this.set("lastLoadedUrl", findUrl);
       });
   },
 });

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");
+    const model = this.modelFor("user").get("userDraftsStream");
-    return userDraftsStream.load(this.site).then(() => userDraftsStream);
+    model.reset();
+    return model.findItems(this.site).then(() => model);
   },
 
   renderTemplate() {

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 })

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 });
     });

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 <img src="/images/emoji/google_classic/slight_smile.png?v=10" title=":slight_smile:" class="emoji" alt=":slight_smile:">'
+    );
+  });
 });

app/assets/javascripts/discourse/tests/fixtures/draft.js

@@ -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
-  }
-};

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
   }
 };

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.delete("/drafts/:draft_key.json", success);
-  pretender.post("/draft.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) => {
+  pretender.get("/drafts.json", () => response(fixturesByUrl["/drafts.json"]));
-    if (request.queryParams.draft_key === "new_topic") {
+
-      return response(fixturesByUrl["/draft.json"]);
+  pretender.get("/drafts/:draft_key.json", (request) =>
-    } else if (request.queryParams.draft_key.startsWith("topic_")) {
+    response(fixturesByUrl[request.url] || { draft: null, draft_sequence: 0 })
-      return response(
-        fixturesByUrl[request.url] || {
-          draft: null,
-          draft_sequence: 0,
-        }
   );
-    }
-    return response({});
-  });
 
   pretender.get("/drafts.json", () => response(fixturesByUrl["/drafts.json"]));
 

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(
+    assert.equal(stream.content.length, 0, "no items are loaded by default");
-      stream.get("itemsLoaded"),
+    assert.blank(stream.content, "no content by default");
-      0,
-      "no items are loaded by default"
-    );
-    assert.blank(stream.get("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"
     );

app/controllers/draft_controller.rb

@@ -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

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
+    stream = Draft.stream(
-
+      user: current_user,
-    unless user == current_user
-      raise Discourse::InvalidAccess
-    end
-
-    opts = {
-      user: 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

app/models/draft.rb

@@ -161,7 +161,11 @@ class Draft < ActiveRecord::Base
   end
 
   def parsed_data
+    begin
       JSON.parse(data)
+    rescue JSON::ParserError
+      {}
+    end
   end
 
   def topic_id

config/routes.rb

@@ -887,10 +887,7 @@ Discourse::Application.routes.draw do
 
     get "message-bus/poll" => "message_bus#poll"
 
-    resources :drafts, only: [:index]
+    resources :drafts, only: [:index, :create, :show, :destroy]
-    get "draft" => "draft#show"
-    post "draft" => "draft#update"
-    delete "draft" => "draft#destroy"
 
     if service_worker_asset = Rails.application.assets_manifest.assets['service-worker.js']
       # https://developers.google.com/web/fundamentals/codelabs/debugging-service-workers/

spec/requests/draft_controller_spec.rb

@@ -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

spec/requests/drafts_controller_spec.rb

@@ -3,6 +3,7 @@
 require 'rails_helper'
 
 describe DraftsController do
+  describe "#index" do
     it 'requires you to be logged in' do
       get "/drafts.json"
       expect(response.status).to eq(403)
@@ -11,28 +12,19 @@ describe DraftsController do
     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 }
+      get "/drafts.json"
       expect(response.status).to eq(200)
       parsed = response.parsed_body
-    expect(parsed["drafts"].length).to eq(1)
+      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", params: { username: user.username }
+      get "/drafts.json"
       expect(response.status).to eq(200)
-    parsed = response.parsed_body
+      expect(response.parsed_body["drafts"].length).to eq(0)
-    expect(parsed["drafts"].length).to eq(0)
-  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
@@ -43,15 +35,178 @@ describe DraftsController do
       Draft.set(other_user, "topic_#{topic.id}", 0, '{}')
 
       sign_in(topic_user)
-    get "/drafts.json", params: { username: topic_user.username }
+      get "/drafts.json"
       expect(response.status).to eq(200)
-    parsed = response.parsed_body
+      expect(response.parsed_body["drafts"].first["title"]).to eq(topic.title)
-    expect(parsed["drafts"].first["title"]).to eq(topic.title)
 
       sign_in(other_user)
-    get "/drafts.json", params: { username: other_user.username }
+      get "/drafts.json"
       expect(response.status).to eq(200)
-    parsed = response.parsed_body
+      expect(response.parsed_body["drafts"].first["title"]).to eq(nil)
-    expect(parsed["drafts"].first["title"]).to eq(nil)
+    end
+  end
+
+  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
+
+  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
+
+  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

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