From e445a8af64908b2bdb721bec74c113e8258a129b Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 6 Sep 2019 13:55:51 +0200 Subject: [PATCH] Add timeline read markers API (#11762) Fix #4093 --- app/controllers/api/v1/markers_controller.rb | 44 +++++++++++++ app/javascript/mastodon/actions/markers.js | 30 +++++++++ app/javascript/mastodon/features/ui/index.js | 5 +- app/models/marker.rb | 23 +++++++ app/models/user.rb | 1 + app/serializers/rest/marker_serializer.rb | 13 ++++ config/routes.rb | 1 + db/migrate/20190904222339_create_markers.rb | 14 ++++ db/schema.rb | 14 +++- .../api/v1/markers_controller_spec.rb | 65 +++++++++++++++++++ spec/fabricators/marker_fabricator.rb | 6 ++ spec/models/marker_spec.rb | 5 ++ 12 files changed, 219 insertions(+), 2 deletions(-) create mode 100644 app/controllers/api/v1/markers_controller.rb create mode 100644 app/javascript/mastodon/actions/markers.js create mode 100644 app/models/marker.rb create mode 100644 app/serializers/rest/marker_serializer.rb create mode 100644 db/migrate/20190904222339_create_markers.rb create mode 100644 spec/controllers/api/v1/markers_controller_spec.rb create mode 100644 spec/fabricators/marker_fabricator.rb create mode 100644 spec/models/marker_spec.rb diff --git a/app/controllers/api/v1/markers_controller.rb b/app/controllers/api/v1/markers_controller.rb new file mode 100644 index 0000000000..28c2ec7916 --- /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 index 0000000000..c3a5fe86f1 --- /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 49c5c8d0ec..63c5622b6b 100644 --- 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 index 0000000000..a5bd2176a8 --- /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 a4a20d9751..95f1d8fc5f 100644 --- 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 index 0000000000..2eaf3d5076 --- /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 c5326052e9..74a162f327 100644 --- 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 index 0000000000..71ca70ac3d --- /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 5576f70bf4..834dddd7bd 100644 --- 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 index 0000000000..556a75b9b1 --- /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 index 0000000000..0c94150e0d --- /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 index 0000000000..d716aa75c2 --- /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