Skip to content

Commit

Permalink
Fix tests, add applications to eager loading/cache for statuses, fix
Browse files Browse the repository at this point in the history
application website validation, don't link to app website if website isn't set,
also comment out animated boost icon from mastodon#464 until it's consistent with non-animated version
  • Loading branch information
Gargron committed Jan 15, 2017
1 parent ab16554 commit e9737c2
Show file tree
Hide file tree
Showing 14 changed files with 78 additions and 37 deletions.
1 change: 1 addition & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,4 @@ AllCops:
- 'bin/*'
- 'Rakefile'
- 'node_modules/**/*'
- 'Vagrantfile'
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand All @@ -42,6 +44,10 @@ const DetailedStatus = React.createClass({
}
}

if (status.get('application')) {
applicationLink = <span> · <a className='detailed-status__application' style={{ color: 'inherit' }} href={status.getIn(['application', 'website'])} target='_blank' rel='nooopener'>{status.getIn(['application', 'name'])}</a></span>;
}

return (
<div style={{ background: '#2f3441', padding: '14px 10px' }} className='detailed-status'>
<a href={status.getIn(['account', 'url'])} onClick={this.handleAccountClick} className='detailed-status__display-name' style={{ display: 'block', overflow: 'hidden', marginBottom: '15px' }}>
Expand All @@ -54,7 +60,7 @@ const DetailedStatus = React.createClass({
{media}

<div style={{ marginTop: '15px', color: '#616b86', fontSize: '14px', lineHeight: '18px' }}>
<a className='detailed-status__datetime' style={{ color: 'inherit' }} href={status.get('url')} target='_blank' rel='noopener'><FormattedDate value={new Date(status.get('created_at'))} hour12={false} year='numeric' month='short' day='2-digit' hour='2-digit' minute='2-digit' /></a> · <a className='detailed-status__application' style={{ color: 'inherit' }} href={status.getIn(['application', 'website'])} target='_blank' rel='nooopener'>{status.getIn(['application', 'name'])}</a> · <Link to={`/statuses/${status.get('id')}/reblogs`} style={{ color: 'inherit', textDecoration: 'none' }}><i className='fa fa-retweet' /><span style={{ fontWeight: '500', fontSize: '12px', marginLeft: '6px', display: 'inline-block' }}><FormattedNumber value={status.get('reblogs_count')} /></span></Link> · <Link to={`/statuses/${status.get('id')}/favourites`} style={{ color: 'inherit', textDecoration: 'none' }}><i className='fa fa-star' /><span style={{ fontWeight: '500', fontSize: '12px', marginLeft: '6px', display: 'inline-block' }}><FormattedNumber value={status.get('favourites_count')} /></span></Link>
<a className='detailed-status__datetime' style={{ color: 'inherit' }} href={status.get('url')} target='_blank' rel='noopener'><FormattedDate value={new Date(status.get('created_at'))} hour12={false} year='numeric' month='short' day='2-digit' hour='2-digit' minute='2-digit' /></a>{applicationLink} · <Link to={`/statuses/${status.get('id')}/reblogs`} style={{ color: 'inherit', textDecoration: 'none' }}><i className='fa fa-retweet' /><span style={{ fontWeight: '500', fontSize: '12px', marginLeft: '6px', display: 'inline-block' }}><FormattedNumber value={status.get('reblogs_count')} /></span></Link> · <Link to={`/statuses/${status.get('id')}/favourites`} style={{ color: 'inherit', textDecoration: 'none' }}><i className='fa fa-star' /><span style={{ fontWeight: '500', fontSize: '12px', marginLeft: '6px', display: 'inline-block' }}><FormattedNumber value={status.get('favourites_count')} /></span></Link>
</div>
</div>
);
Expand Down
35 changes: 18 additions & 17 deletions app/assets/stylesheets/components.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;

&::before {
display: none !important;
}
}

button.active i.fa-retweet {
transition-duration: 0.9s;
background-position: 0 -209px;
}
// 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;
// }
// }

// button.active i.fa-retweet {
// transition-duration: 0.9s;
// background-position: 0 -209px;
// }
9 changes: 9 additions & 0 deletions app/lib/application_extension.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# frozen_string_literal: true

module ApplicationExtension
extend ActiveSupport::Concern

included do
validates :website, url: true, unless: 'website.blank?'
end
end
14 changes: 14 additions & 0 deletions app/lib/url_validator.rb
Original file line number Diff line number Diff line change
@@ -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
8 changes: 0 additions & 8 deletions app/models/concerns/application.rb

This file was deleted.

2 changes: 1 addition & 1 deletion app/models/status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
9 changes: 8 additions & 1 deletion app/services/post_status_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion app/views/api/v1/apps/show.rabl
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
object @application

attributes :id, :name, :website
attributes :name, :website
10 changes: 6 additions & 4 deletions app/views/stream_entries/_detailed_status.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class Application < Rails::Application

config.to_prepare do
Doorkeeper::AuthorizationsController.layout 'public'
Doorkeeper::Application.send :include, ApplicationExtension
end

config.action_dispatch.default_headers = {
Expand Down
6 changes: 4 additions & 2 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ 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
status_count_before: Who authored
terms: Terms
user_count_after: users
user_count_before: Home to
learn_more: Learn more
accounts:
follow: Follow
followers: Followers
Expand All @@ -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?
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion spec/controllers/api/v1/statuses_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
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 }
Expand Down
5 changes: 5 additions & 0 deletions spec/fabricators/application_fabricator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fabricator(:application, from: Doorkeeper::Application) do
name 'Example'
website 'http://example.com'
redirect_uri 'http://example.com/callback'
end

0 comments on commit e9737c2

Please sign in to comment.