From d6bc0e8db4d25a4533feb56164b0d9cd3ef2af6e Mon Sep 17 00:00:00 2001 From: Effy Elden Date: Sun, 15 Jan 2017 08:58:50 +1100 Subject: [PATCH 1/4] Add tracking of OAuth app that posted a status, extend OAuth apps to have optional website field, add application details to API, show application name and website on detailed status views. Resolves #11 --- .../features/status/components/detailed_status.jsx | 2 +- app/assets/stylesheets/components.scss | 2 +- app/controllers/api/v1/apps_controller.rb | 2 +- app/controllers/api/v1/statuses_controller.rb | 2 +- app/models/concerns/application.rb | 8 ++++++++ app/models/status.rb | 2 ++ app/services/post_status_service.rb | 2 +- app/views/api/v1/apps/show.rabl | 3 +++ app/views/api/v1/statuses/_show.rabl | 4 ++++ app/views/stream_entries/_detailed_status.html.haml | 3 +++ db/migrate/20170114194937_add_application_to_statuses.rb | 5 +++++ .../20170114203041_add_website_to_oauth_application.rb | 5 +++++ db/schema.rb | 4 +++- 13 files changed, 38 insertions(+), 6 deletions(-) create mode 100644 app/models/concerns/application.rb create mode 100644 app/views/api/v1/apps/show.rabl create mode 100644 db/migrate/20170114194937_add_application_to_statuses.rb create mode 100644 db/migrate/20170114203041_add_website_to_oauth_application.rb diff --git a/app/assets/javascripts/components/features/status/components/detailed_status.jsx b/app/assets/javascripts/components/features/status/components/detailed_status.jsx index b967d966f03..7cbca463327 100644 --- a/app/assets/javascripts/components/features/status/components/detailed_status.jsx +++ b/app/assets/javascripts/components/features/status/components/detailed_status.jsx @@ -54,7 +54,7 @@ const DetailedStatus = React.createClass({ {media}
- · · + · {status.getIn(['application', 'name'])} · ·
); diff --git a/app/assets/stylesheets/components.scss b/app/assets/stylesheets/components.scss index f1edfce9dfd..9e9e564f9f0 100644 --- a/app/assets/stylesheets/components.scss +++ b/app/assets/stylesheets/components.scss @@ -183,7 +183,7 @@ } } -.status__display-name, .status__relative-time, .detailed-status__display-name, .detailed-status__datetime, .account__display-name { +.status__display-name, .status__relative-time, .detailed-status__display-name, .detailed-status__datetime, .detailed-status__application, .account__display-name { text-decoration: none; } diff --git a/app/controllers/api/v1/apps_controller.rb b/app/controllers/api/v1/apps_controller.rb index 1b33770f4e3..ca9dd0b7eea 100644 --- a/app/controllers/api/v1/apps_controller.rb +++ b/app/controllers/api/v1/apps_controller.rb @@ -4,6 +4,6 @@ class Api::V1::AppsController < ApiController respond_to :json def create - @app = Doorkeeper::Application.create!(name: params[:client_name], redirect_uri: params[:redirect_uris], scopes: (params[:scopes] || Doorkeeper.configuration.default_scopes)) + @app = Doorkeeper::Application.create!(name: params[:client_name], redirect_uri: params[:redirect_uris], scopes: (params[:scopes] || Doorkeeper.configuration.default_scopes), website: params[:website]) end end diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index f7b4ed6100a..f033ef6c1e6 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -52,7 +52,7 @@ class Api::V1::StatusesController < ApiController end def create - @status = PostStatusService.new.call(current_user.account, params[:status], params[:in_reply_to_id].blank? ? nil : Status.find(params[:in_reply_to_id]), media_ids: params[:media_ids], sensitive: params[:sensitive], visibility: params[:visibility]) + @status = PostStatusService.new.call(current_user.account, params[:status], params[:in_reply_to_id].blank? ? nil : Status.find(params[:in_reply_to_id]), media_ids: params[:media_ids], sensitive: params[:sensitive], visibility: params[:visibility], application: doorkeeper_token.application) render action: :show end diff --git a/app/models/concerns/application.rb b/app/models/concerns/application.rb new file mode 100644 index 00000000000..613be34eeb6 --- /dev/null +++ b/app/models/concerns/application.rb @@ -0,0 +1,8 @@ +module ApplicationExtension + extend ActiveSupport::Concern + included do + validates :website + end +end + +Doorkeeper::Application.send :include, ApplicationExtension \ No newline at end of file diff --git a/app/models/status.rb b/app/models/status.rb index bc595c93b15..8301ae16eea 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -7,6 +7,8 @@ class Status < ApplicationRecord enum visibility: [:public, :unlisted, :private], _suffix: :visibility + belongs_to :application, class_name: 'Doorkeeper::Application' + belongs_to :account, inverse_of: :statuses belongs_to :in_reply_to_account, foreign_key: 'in_reply_to_account_id', class_name: 'Account' diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index 55405c0dbed..86a84f512bf 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -10,7 +10,7 @@ class PostStatusService < BaseService # @option [Enumerable] :media_ids Optional array of media IDs to attach # @return [Status] def call(account, text, in_reply_to = nil, options = {}) - status = account.statuses.create!(text: text, thread: in_reply_to, sensitive: options[:sensitive], visibility: options[:visibility]) + status = account.statuses.create!(text: text, thread: in_reply_to, sensitive: options[:sensitive], visibility: options[:visibility], application: options[:application]) attach_media(status, options[:media_ids]) process_mentions_service.call(status) process_hashtags_service.call(status) diff --git a/app/views/api/v1/apps/show.rabl b/app/views/api/v1/apps/show.rabl new file mode 100644 index 00000000000..30cfd81ab52 --- /dev/null +++ b/app/views/api/v1/apps/show.rabl @@ -0,0 +1,3 @@ +object @application + +attributes :id, :name, :website \ No newline at end of file diff --git a/app/views/api/v1/statuses/_show.rabl b/app/views/api/v1/statuses/_show.rabl index a3391a67e77..a3fc7876303 100644 --- a/app/views/api/v1/statuses/_show.rabl +++ b/app/views/api/v1/statuses/_show.rabl @@ -6,6 +6,10 @@ node(:url) { |status| TagManager.instance.url_for(status) } node(:reblogs_count) { |status| defined?(@reblogs_counts_map) ? (@reblogs_counts_map[status.id] || 0) : status.reblogs.count } node(:favourites_count) { |status| defined?(@favourites_counts_map) ? (@favourites_counts_map[status.id] || 0) : status.favourites.count } +child :application do + extends 'api/v1/apps/show' +end + child :account do extends 'api/v1/accounts/show' end diff --git a/app/views/stream_entries/_detailed_status.html.haml b/app/views/stream_entries/_detailed_status.html.haml index 32f7c2e40b3..bc994091503 100644 --- a/app/views/stream_entries/_detailed_status.html.haml +++ b/app/views/stream_entries/_detailed_status.html.haml @@ -28,6 +28,9 @@ = link_to TagManager.instance.url_for(status), class: 'detailed-status__datetime u-url u-uid', target: @external_links ? '_blank' : nil, rel: 'noopener' do %span= l(status.created_at) · + = link_to status.application.website, class: 'detailed-status__application', target: @external_links ? '_blank' : nil, rel: 'noooper' do + %span= status.application.name + · %span = fa_icon('retweet') %span= status.reblogs.count diff --git a/db/migrate/20170114194937_add_application_to_statuses.rb b/db/migrate/20170114194937_add_application_to_statuses.rb new file mode 100644 index 00000000000..b699db2ac6b --- /dev/null +++ b/db/migrate/20170114194937_add_application_to_statuses.rb @@ -0,0 +1,5 @@ +class AddApplicationToStatuses < ActiveRecord::Migration[5.0] + def change + add_column :statuses, :application_id, :int + end +end diff --git a/db/migrate/20170114203041_add_website_to_oauth_application.rb b/db/migrate/20170114203041_add_website_to_oauth_application.rb new file mode 100644 index 00000000000..ee674be7228 --- /dev/null +++ b/db/migrate/20170114203041_add_website_to_oauth_application.rb @@ -0,0 +1,5 @@ +class AddWebsiteToOauthApplication < ActiveRecord::Migration[5.0] + def change + add_column :oauth_applications, :website, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 1cd1258db8b..37da0c44ebb 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: 20170112154826) do +ActiveRecord::Schema.define(version: 20170114203041) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -153,6 +153,7 @@ ActiveRecord::Schema.define(version: 20170112154826) do t.datetime "created_at" t.datetime "updated_at" t.boolean "superapp", default: false, null: false + t.string "website" t.index ["uid"], name: "index_oauth_applications_on_uid", unique: true, using: :btree end @@ -259,6 +260,7 @@ ActiveRecord::Schema.define(version: 20170112154826) do t.boolean "sensitive", default: false t.integer "visibility", default: 0, null: false t.integer "in_reply_to_account_id" + t.integer "application_id" t.index ["account_id"], name: "index_statuses_on_account_id", using: :btree t.index ["in_reply_to_id"], name: "index_statuses_on_in_reply_to_id", using: :btree t.index ["reblog_of_id"], name: "index_statuses_on_reblog_of_id", using: :btree From 65122798b2fdec5c3655ce9d7495c91de41e7397 Mon Sep 17 00:00:00 2001 From: Effy Elden Date: Sun, 15 Jan 2017 09:04:14 +1100 Subject: [PATCH 2/4] Fix typo in rel --- app/views/stream_entries/_detailed_status.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/stream_entries/_detailed_status.html.haml b/app/views/stream_entries/_detailed_status.html.haml index bc994091503..a751abf3c59 100644 --- a/app/views/stream_entries/_detailed_status.html.haml +++ b/app/views/stream_entries/_detailed_status.html.haml @@ -28,7 +28,7 @@ = link_to TagManager.instance.url_for(status), class: 'detailed-status__datetime u-url u-uid', target: @external_links ? '_blank' : nil, rel: 'noopener' do %span= l(status.created_at) · - = link_to status.application.website, class: 'detailed-status__application', target: @external_links ? '_blank' : nil, rel: 'noooper' do + = link_to status.application.website, class: 'detailed-status__application', target: @external_links ? '_blank' : nil, rel: 'noopener' do %span= status.application.name · %span From 6c002cf615128151e089b197bebdbe813bc64da6 Mon Sep 17 00:00:00 2001 From: Effy Elden Date: Sun, 15 Jan 2017 09:10:44 +1100 Subject: [PATCH 3/4] Test for presence of Application in haml before trying to render details, to fix errors identified by CI --- app/views/stream_entries/_detailed_status.html.haml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/views/stream_entries/_detailed_status.html.haml b/app/views/stream_entries/_detailed_status.html.haml index bc994091503..a89503cff00 100644 --- a/app/views/stream_entries/_detailed_status.html.haml +++ b/app/views/stream_entries/_detailed_status.html.haml @@ -28,9 +28,10 @@ = link_to TagManager.instance.url_for(status), class: 'detailed-status__datetime u-url u-uid', target: @external_links ? '_blank' : nil, rel: 'noopener' do %span= l(status.created_at) · - = link_to status.application.website, class: 'detailed-status__application', target: @external_links ? '_blank' : nil, rel: 'noooper' do - %span= status.application.name - · + - if status.application + = link_to status.application.website, class: 'detailed-status__application', target: @external_links ? '_blank' : nil, rel: 'noooper' do + %span= status.application.name + · %span = fa_icon('retweet') %span= status.reblogs.count From e9737c2235ec56502e650bd1adad3f32bf85f0ef Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 15 Jan 2017 14:01:33 +0100 Subject: [PATCH 4/4] Fix tests, add applications to eager loading/cache for statuses, fix application website validation, don't link to app website if website isn't set, also comment out animated boost icon from #464 until it's consistent with non-animated version --- .rubocop.yml | 1 + .../status/components/detailed_status.jsx | 10 ++++-- app/assets/stylesheets/components.scss | 31 ++++++++++--------- app/lib/application_extension.rb | 9 ++++++ app/lib/url_validator.rb | 14 +++++++++ app/models/concerns/application.rb | 8 ----- app/models/status.rb | 2 +- app/services/post_status_service.rb | 9 +++++- app/views/api/v1/apps/show.rabl | 2 +- .../stream_entries/_detailed_status.html.haml | 10 +++--- config/application.rb | 1 + config/locales/en.yml | 6 ++-- .../api/v1/statuses_controller_spec.rb | 3 +- spec/fabricators/application_fabricator.rb | 5 +++ 14 files changed, 76 insertions(+), 35 deletions(-) create mode 100644 app/lib/application_extension.rb create mode 100644 app/lib/url_validator.rb delete mode 100644 app/models/concerns/application.rb create mode 100644 spec/fabricators/application_fabricator.rb diff --git a/.rubocop.yml b/.rubocop.yml index 28c7359130e..ab28c0fe1b0 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -87,3 +87,4 @@ AllCops: - 'bin/*' - 'Rakefile' - 'node_modules/**/*' + - 'Vagrantfile' diff --git a/app/assets/javascripts/components/features/status/components/detailed_status.jsx b/app/assets/javascripts/components/features/status/components/detailed_status.jsx index 7cbca463327..14a504c7cdb 100644 --- a/app/assets/javascripts/components/features/status/components/detailed_status.jsx +++ b/app/assets/javascripts/components/features/status/components/detailed_status.jsx @@ -32,7 +32,9 @@ const DetailedStatus = React.createClass({ render () { const status = this.props.status.get('reblog') ? this.props.status.get('reblog') : this.props.status; - let media = ''; + + let media = ''; + let applicationLink = ''; if (status.get('media_attachments').size > 0) { if (status.getIn(['media_attachments', 0, 'type']) === 'video') { @@ -42,6 +44,10 @@ const DetailedStatus = React.createClass({ } } + if (status.get('application')) { + applicationLink = · {status.getIn(['application', 'name'])}; + } + return ( ); diff --git a/app/assets/stylesheets/components.scss b/app/assets/stylesheets/components.scss index f4d822dcfde..2d99fcfe837 100644 --- a/app/assets/stylesheets/components.scss +++ b/app/assets/stylesheets/components.scss @@ -663,20 +663,21 @@ } } -button i.fa-retweet { - height: 19px; - width: 24px; - background: image-url('boost_sprite.png') no-repeat; - background-position: 0 0; - transition: background-position 0.9s steps(11); - transition-duration: 0s; +// Commented out until sprite matches non-sprite icon visually +// button i.fa-retweet { +// height: 19px; +// width: 24px; +// background: image-url('boost_sprite.png') no-repeat; +// background-position: 0 0; +// transition: background-position 0.9s steps(11); +// transition-duration: 0s; - &::before { - display: none !important; - } -} +// &::before { +// display: none !important; +// } +// } -button.active i.fa-retweet { - transition-duration: 0.9s; - background-position: 0 -209px; -} +// button.active i.fa-retweet { +// transition-duration: 0.9s; +// background-position: 0 -209px; +// } diff --git a/app/lib/application_extension.rb b/app/lib/application_extension.rb new file mode 100644 index 00000000000..93c0f42f068 --- /dev/null +++ b/app/lib/application_extension.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module ApplicationExtension + extend ActiveSupport::Concern + + included do + validates :website, url: true, unless: 'website.blank?' + end +end diff --git a/app/lib/url_validator.rb b/app/lib/url_validator.rb new file mode 100644 index 00000000000..4a5c4ef3fff --- /dev/null +++ b/app/lib/url_validator.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class UrlValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + record.errors.add(attribute, I18n.t('applications.invalid_url')) unless compliant?(value) + end + + private + + def compliant?(url) + parsed_url = Addressable::URI.parse(url) + !parsed_url.nil? && %w(http https).include?(parsed_url.scheme) && parsed_url.host + end +end diff --git a/app/models/concerns/application.rb b/app/models/concerns/application.rb deleted file mode 100644 index 613be34eeb6..00000000000 --- a/app/models/concerns/application.rb +++ /dev/null @@ -1,8 +0,0 @@ -module ApplicationExtension - extend ActiveSupport::Concern - included do - validates :website - end -end - -Doorkeeper::Application.send :include, ApplicationExtension \ No newline at end of file diff --git a/app/models/status.rb b/app/models/status.rb index 8301ae16eea..5710f9ccacc 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -35,7 +35,7 @@ class Status < ApplicationRecord scope :remote, -> { where.not(uri: nil) } scope :local, -> { where(uri: nil) } - cache_associated :account, :media_attachments, :tags, :stream_entry, mentions: :account, reblog: [:account, :stream_entry, :tags, :media_attachments, mentions: :account], thread: :account + cache_associated :account, :application, :media_attachments, :tags, :stream_entry, mentions: :account, reblog: [:account, :application, :stream_entry, :tags, :media_attachments, mentions: :account], thread: :account def local? uri.nil? diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index 86a84f512bf..af31c923ff3 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -7,10 +7,17 @@ class PostStatusService < BaseService # @param [Status] in_reply_to Optional status to reply to # @param [Hash] options # @option [Boolean] :sensitive + # @option [String] :visibility # @option [Enumerable] :media_ids Optional array of media IDs to attach + # @option [Doorkeeper::Application] :application # @return [Status] def call(account, text, in_reply_to = nil, options = {}) - status = account.statuses.create!(text: text, thread: in_reply_to, sensitive: options[:sensitive], visibility: options[:visibility], application: options[:application]) + status = account.statuses.create!(text: text, + thread: in_reply_to, + sensitive: options[:sensitive], + visibility: options[:visibility], + application: options[:application]) + attach_media(status, options[:media_ids]) process_mentions_service.call(status) process_hashtags_service.call(status) diff --git a/app/views/api/v1/apps/show.rabl b/app/views/api/v1/apps/show.rabl index 30cfd81ab52..6d9e607db99 100644 --- a/app/views/api/v1/apps/show.rabl +++ b/app/views/api/v1/apps/show.rabl @@ -1,3 +1,3 @@ object @application -attributes :id, :name, :website \ No newline at end of file +attributes :name, :website diff --git a/app/views/stream_entries/_detailed_status.html.haml b/app/views/stream_entries/_detailed_status.html.haml index 946adbd8e7a..bc09d359796 100644 --- a/app/views/stream_entries/_detailed_status.html.haml +++ b/app/views/stream_entries/_detailed_status.html.haml @@ -29,13 +29,15 @@ %span= l(status.created_at) · - if status.application - = link_to status.application.website, class: 'detailed-status__application', target: @external_links ? '_blank' : nil, rel: 'noopener' do - %span= status.application.name + - if status.application.website.blank? + %strong.detailed-status__application= status.application.name + - else + = link_to status.application.name, status.application.website, class: 'detailed-status__application', target: '_blank', rel: 'noopener' · - %span + %span< = fa_icon('retweet') %span= status.reblogs.count · - %span + %span< = fa_icon('star') %span= status.favourites.count diff --git a/config/application.rb b/config/application.rb index 79ace8521cb..e561d04738e 100644 --- a/config/application.rb +++ b/config/application.rb @@ -46,6 +46,7 @@ module Mastodon config.to_prepare do Doorkeeper::AuthorizationsController.layout 'public' + Doorkeeper::Application.send :include, ApplicationExtension end config.action_dispatch.default_headers = { diff --git a/config/locales/en.yml b/config/locales/en.yml index 128a4d40ed3..f7d7ed72984 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -8,6 +8,7 @@ en: domain_count_after: other instances domain_count_before: Connected to get_started: Get started + learn_more: Learn more links: Links source_code: Source code status_count_after: statuses @@ -15,7 +16,6 @@ en: terms: Terms user_count_after: users user_count_before: Home to - learn_more: Learn more accounts: follow: Follow followers: Followers @@ -28,6 +28,8 @@ en: unfollow: Unfollow application_mailer: signature: Mastodon notifications from %{instance} + applications: + invalid_url: The provided URL is invalid auth: change_password: Change password didnt_get_confirmation: Didn't receive confirmation instructions? @@ -88,9 +90,9 @@ en: proceed: Proceed to follow prompt: 'You are going to follow:' settings: + back: Back to Mastodon edit_profile: Edit profile preferences: Preferences - back: Back to Mastodon stream_entries: click_to_show: Click to show favourited: favourited a post by diff --git a/spec/controllers/api/v1/statuses_controller_spec.rb b/spec/controllers/api/v1/statuses_controller_spec.rb index d9c73f9529a..669956659a6 100644 --- a/spec/controllers/api/v1/statuses_controller_spec.rb +++ b/spec/controllers/api/v1/statuses_controller_spec.rb @@ -4,7 +4,8 @@ RSpec.describe Api::V1::StatusesController, type: :controller do render_views let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } - let(:token) { double acceptable?: true, resource_owner_id: user.id } + let(:app) { Fabricate(:application, name: 'Test app', website: 'http://testapp.com') } + let(:token) { double acceptable?: true, resource_owner_id: user.id, application: app } before do allow(controller).to receive(:doorkeeper_token) { token } diff --git a/spec/fabricators/application_fabricator.rb b/spec/fabricators/application_fabricator.rb new file mode 100644 index 00000000000..42b7009dc23 --- /dev/null +++ b/spec/fabricators/application_fabricator.rb @@ -0,0 +1,5 @@ +Fabricator(:application, from: Doorkeeper::Application) do + name 'Example' + website 'http://example.com' + redirect_uri 'http://example.com/callback' +end