Add timeline read markers API (#11762)

Fix #4093

diff --git a/app/controllers/api/v1/markers_controller.rb b/app/controllers/api/v1/markers_controller.rb
new file mode 100644 (file)
index 0000000..28c2ec7
--- /dev/null
+++ b/app/controllers/api/v1/markers_controller.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+class Api::V1::MarkersController < Api::BaseController
+  before_action -> { doorkeeper_authorize! :read, :'read:statuses' }, only: [:index]
+  before_action -> { doorkeeper_authorize! :write, :'write:statuses' }, except: [:index]
+
+  before_action :require_user!
+
+  def index
+    @markers = current_user.markers.where(timeline: Array(params[:timeline])).each_with_object({}) { |marker, h| h[marker.timeline] = marker }
+    render json: serialize_map(@markers)
+  end
+
+  def create
+    Marker.transaction do
+      @markers = {}
+
+      resource_params.each_pair do |timeline, timeline_params|
+        @markers[timeline] = current_user.markers.find_or_initialize_by(timeline: timeline)
+        @markers[timeline].update!(timeline_params)
+      end
+    end
+
+    render json: serialize_map(@markers)
+  rescue ActiveRecord::StaleObjectError
+    render json: { error: 'Conflict during update, please try again' }, status: 409
+  end
+
+  private
+
+  def serialize_map(map)
+    serialized = {}
+
+    map.each_pair do |key, value|
+      serialized[key] = ActiveModelSerializers::SerializableResource.new(value, serializer: REST::MarkerSerializer).as_json
+    end
+
+    Oj.dump(serialized)
+  end
+
+  def resource_params
+    params.slice(*Marker::TIMELINES).permit(*Marker::TIMELINES.map { |timeline| { timeline.to_sym => [:last_read_id] } })
+  end
+end

diff --git a/app/javascript/mastodon/actions/markers.js b/app/javascript/mastodon/actions/markers.js
new file mode 100644 (file)
index 0000000..c3a5fe8
--- /dev/null
+++ b/app/javascript/mastodon/actions/markers.js
@@ -0,0 +1,30 @@
+export const submitMarkers = () => (dispatch, getState) => {
+  const accessToken = getState().getIn(['meta', 'access_token'], '');
+  const params      = {};
+
+  const lastHomeId         = getState().getIn(['timelines', 'home', 'items', 0]);
+  const lastNotificationId = getState().getIn(['notifications', 'items', 0, 'id']);
+
+  if (lastHomeId) {
+    params.home = {
+      last_read_id: lastHomeId,
+    };
+  }
+
+  if (lastNotificationId) {
+    params.notifications = {
+      last_read_id: lastNotificationId,
+    };
+  }
+
+  if (Object.keys(params).length === 0) {
+    return;
+  }
+
+  const client = new XMLHttpRequest();
+
+  client.open('POST', '/api/v1/markers', false);
+  client.setRequestHeader('Content-Type', 'application/json');
+  client.setRequestHeader('Authorization', `Bearer ${accessToken}`);
+  client.send(JSON.stringify(params));
+};

diff --git a/app/javascript/mastodon/features/ui/index.js b/app/javascript/mastodon/features/ui/index.js
index 49c5c8d0ec19094bf697e0f44fcbf389151814a2..63c5622b6bfe04fe59709089acc3ecf446b14246 100644 (file)
--- a/app/javascript/mastodon/features/ui/index.js
+++ b/app/javascript/mastodon/features/ui/index.js
@@ -16,6 +16,7 @@ import { expandNotifications } from '../../actions/notifications';
 import { fetchFilters } from '../../actions/filters';
 import { clearHeight } from '../../actions/height_cache';
 import { focusApp, unfocusApp } from 'mastodon/actions/app';
+import { submitMarkers } from 'mastodon/actions/markers';
 import { WrappedSwitch, WrappedRoute } from './util/react_router_helpers';
 import UploadArea from './components/upload_area';
 import ColumnsAreaContainer from './containers/columns_area_container';
@@ -241,7 +242,9 @@ class UI extends React.PureComponent {
   };
 
   handleBeforeUnload = e => {
-    const { intl, isComposing, hasComposingText, hasMediaAttachments } = this.props;
+    const { intl, dispatch, isComposing, hasComposingText, hasMediaAttachments } = this.props;
+
+    dispatch(submitMarkers());
 
     if (isComposing && (hasComposingText || hasMediaAttachments)) {
       // Setting returnValue to any string causes confirmation dialog.

diff --git a/app/models/marker.rb b/app/models/marker.rb
new file mode 100644 (file)
index 0000000..a5bd217
--- /dev/null
+++ b/app/models/marker.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+# == Schema Information
+#
+# Table name: markers
+#
+#  id           :bigint(8)        not null, primary key
+#  user_id      :bigint(8)
+#  timeline     :string           default(""), not null
+#  last_read_id :bigint(8)        default(0), not null
+#  lock_version :integer          default(0), not null
+#  created_at   :datetime         not null
+#  updated_at   :datetime         not null
+#
+
+class Marker < ApplicationRecord
+  TIMELINES = %w(home notifications).freeze
+
+  belongs_to :user
+
+  validates :timeline, :last_read_id, presence: true
+  validates :timeline, inclusion: { in: TIMELINES }
+end

diff --git a/app/models/user.rb b/app/models/user.rb
index a4a20d97512707f070910d48dd718c96eb2fb96f..95f1d8fc5ff33543a74b827ac344cfb4d420a075 100644 (file)
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -74,6 +74,7 @@ class User < ApplicationRecord
   has_many :applications, class_name: 'Doorkeeper::Application', as: :owner
   has_many :backups, inverse_of: :user
   has_many :invites, inverse_of: :user
+  has_many :markers, inverse_of: :user, dependent: :destroy
 
   has_one :invite_request, class_name: 'UserInviteRequest', inverse_of: :user, dependent: :destroy
   accepts_nested_attributes_for :invite_request, reject_if: ->(attributes) { attributes['text'].blank? }

diff --git a/app/serializers/rest/marker_serializer.rb b/app/serializers/rest/marker_serializer.rb
new file mode 100644 (file)
index 0000000..2eaf3d5
--- /dev/null
+++ b/app/serializers/rest/marker_serializer.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class REST::MarkerSerializer < ActiveModel::Serializer
+  attributes :last_read_id, :version, :updated_at
+
+  def last_read_id
+    object.last_read_id.to_s
+  end
+
+  def version
+    object.lock_version
+  end
+end

diff --git a/config/routes.rb b/config/routes.rb
index c5326052e90512595b4a779b02be8186eea59e21..74a162f327b16905000bfb12d0d2e7ac017f618e 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -314,6 +314,7 @@ Rails.application.routes.draw do
       resources :trends,       only: [:index]
       resources :filters,      only: [:index, :create, :show, :update, :destroy]
       resources :endorsements, only: [:index]
+      resources :markers,      only: [:index, :create]
 
       namespace :apps do
         get :verify_credentials, to: 'credentials#show'

diff --git a/db/migrate/20190904222339_create_markers.rb b/db/migrate/20190904222339_create_markers.rb
new file mode 100644 (file)
index 0000000..71ca70a
--- /dev/null
+++ b/db/migrate/20190904222339_create_markers.rb
@@ -0,0 +1,14 @@
+class CreateMarkers < ActiveRecord::Migration[5.2]
+  def change
+    create_table :markers do |t|
+      t.references :user, foreign_key: { on_delete: :cascade, index: false }
+      t.string :timeline, default: '', null: false
+      t.bigint :last_read_id, default: 0, null: false
+      t.integer :lock_version, default: 0, null: false
+
+      t.timestamps
+    end
+
+    add_index :markers, [:user_id, :timeline], unique: true
+  end
+end

diff --git a/db/schema.rb b/db/schema.rb
index 5576f70bf4d1e1f7f5e13201c8e5fbf869b9d9b1..834dddd7bd43ef9cf06906c854959603495ab568 100644 (file)
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_09_01_040524) do
+ActiveRecord::Schema.define(version: 2019_09_04_222339) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -366,6 +366,17 @@ ActiveRecord::Schema.define(version: 2019_09_01_040524) do
     t.index ["account_id"], name: "index_lists_on_account_id"
   end
 
+  create_table "markers", force: :cascade do |t|
+    t.bigint "user_id"
+    t.string "timeline", default: "", null: false
+    t.bigint "last_read_id", default: 0, null: false
+    t.integer "lock_version", default: 0, null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["user_id", "timeline"], name: "index_markers_on_user_id_and_timeline", unique: true
+    t.index ["user_id"], name: "index_markers_on_user_id"
+  end
+
   create_table "media_attachments", force: :cascade do |t|
     t.bigint "status_id"
     t.string "file_file_name"
@@ -791,6 +802,7 @@ ActiveRecord::Schema.define(version: 2019_09_01_040524) do
   add_foreign_key "list_accounts", "follows", on_delete: :cascade
   add_foreign_key "list_accounts", "lists", on_delete: :cascade
   add_foreign_key "lists", "accounts", on_delete: :cascade
+  add_foreign_key "markers", "users", on_delete: :cascade
   add_foreign_key "media_attachments", "accounts", name: "fk_96dd81e81b", on_delete: :nullify
   add_foreign_key "media_attachments", "scheduled_statuses", on_delete: :nullify
   add_foreign_key "media_attachments", "statuses", on_delete: :nullify

diff --git a/spec/controllers/api/v1/markers_controller_spec.rb b/spec/controllers/api/v1/markers_controller_spec.rb
new file mode 100644 (file)
index 0000000..556a75b
--- /dev/null
+++ b/spec/controllers/api/v1/markers_controller_spec.rb
@@ -0,0 +1,65 @@
+require 'rails_helper'
+
+RSpec.describe Api::V1::MarkersController, type: :controller do
+  render_views
+
+  let!(:user)  { Fabricate(:user, account: Fabricate(:account, username: 'alice')) }
+  let!(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:statuses write:statuses') }
+
+  before { allow(controller).to receive(:doorkeeper_token) { token } }
+
+  describe 'GET #index' do
+    before do
+      Fabricate(:marker, timeline: 'home', last_read_id: 123, user: user)
+      Fabricate(:marker, timeline: 'notifications', last_read_id: 456, user: user)
+
+      get :index, params: { timeline: %w(home notifications) }
+    end
+
+    it 'returns http success' do
+      expect(response).to have_http_status(200)
+    end
+
+    it 'returns markers' do
+      json = body_as_json
+
+      expect(json.key?(:home)).to be true
+      expect(json[:home][:last_read_id]).to eq '123'
+      expect(json.key?(:notifications)).to be true
+      expect(json[:notifications][:last_read_id]).to eq '456'
+    end
+  end
+
+  describe 'POST #create' do
+    context 'when no marker exists' do
+      before do
+        post :create, params: { home: { last_read_id: '69420' } }
+      end
+
+      it 'returns http success' do
+        expect(response).to have_http_status(200)
+      end
+
+      it 'creates a marker' do
+        expect(user.markers.first.timeline).to eq 'home'
+        expect(user.markers.first.last_read_id).to eq 69420
+      end
+    end
+
+    context 'when a marker exists' do
+      before do
+        post :create, params: { home: { last_read_id: '69420' } }
+        post :create, params: { home: { last_read_id: '70120' } }
+      end
+
+      it 'returns http success' do
+        expect(response).to have_http_status(200)
+      end
+
+      it 'updates a marker' do
+        expect(user.markers.first.timeline).to eq 'home'
+        expect(user.markers.first.last_read_id).to eq 70120
+      end
+    end
+  end
+end

diff --git a/spec/fabricators/marker_fabricator.rb b/spec/fabricators/marker_fabricator.rb
new file mode 100644 (file)
index 0000000..0c94150
--- /dev/null
+++ b/spec/fabricators/marker_fabricator.rb
@@ -0,0 +1,6 @@
+Fabricator(:marker) do
+  user
+  timeline     'home'
+  last_read_id 0
+  lock_version 0
+end

diff --git a/spec/models/marker_spec.rb b/spec/models/marker_spec.rb
new file mode 100644 (file)
index 0000000..d716aa7
--- /dev/null
+++ b/spec/models/marker_spec.rb
@@ -0,0 +1,5 @@
+require 'rails_helper'
+
+RSpec.describe Marker, type: :model do
+  pending "add some examples to (or delete) #{__FILE__}"
+end