From a30ee0d082e5c6b2d40ee0a26053aa26940017cc Mon Sep 17 00:00:00 2001 From: Marcin Kulik Date: Sat, 15 Nov 2014 17:46:46 +0000 Subject: [PATCH 01/15] Allow making asciicasts "private" --- app/models/asciicast.rb | 12 ++++++++++++ app/models/user.rb | 2 +- .../20141115172153_add_secret_token_to_asciicasts.rb | 11 +++++++++++ .../20141115172558_add_private_to_asciicasts.rb | 5 +++++ ...20141115172711_add_index_on_asciicasts_private.rb | 5 +++++ ...43_add_unique_index_to_asciicasts_secret_token.rb | 5 +++++ db/schema.rb | 4 ++++ 7 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20141115172153_add_secret_token_to_asciicasts.rb create mode 100644 db/migrate/20141115172558_add_private_to_asciicasts.rb create mode 100644 db/migrate/20141115172711_add_index_on_asciicasts_private.rb create mode 100644 db/migrate/20141115174443_add_unique_index_to_asciicasts_secret_token.rb diff --git a/app/models/asciicast.rb b/app/models/asciicast.rb index 8164f0a..4ea2c2a 100644 --- a/app/models/asciicast.rb +++ b/app/models/asciicast.rb @@ -29,6 +29,8 @@ class Asciicast < ActiveRecord::Base featured.by_random.limit(n).includes(:user) } + before_create :generate_secret_token + def self.cache_key timestamps = scoped.select(:updated_at).map { |o| o.updated_at.to_i } Digest::MD5.hexdigest timestamps.join('/') @@ -62,6 +64,10 @@ class Asciicast < ActiveRecord::Base value ? super(value.strip[0...255]) : super end + def self.generate_secret_token + SecureRandom.hex.to_i(16).to_s(36) + end + def stdout return @stdout if @stdout @stdout = Stdout::Buffered.new(get_stdout) @@ -103,4 +109,10 @@ class Asciicast < ActiveRecord::Base Digest::SHA1.hexdigest(input) end + def generate_secret_token + begin + self.secret_token = self.class.generate_secret_token + end while self.class.exists?(secret_token: secret_token) + end + end diff --git a/app/models/user.rb b/app/models/user.rb index 2534092..1bedcd6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -131,7 +131,7 @@ class User < ActiveRecord::Base def generate_auth_token begin self[:auth_token] = self.class.generate_auth_token - end while User.exists?(auth_token: self[:auth_token]) + end while self.class.exists?(auth_token: self[:auth_token]) end end diff --git a/db/migrate/20141115172153_add_secret_token_to_asciicasts.rb b/db/migrate/20141115172153_add_secret_token_to_asciicasts.rb new file mode 100644 index 0000000..7984244 --- /dev/null +++ b/db/migrate/20141115172153_add_secret_token_to_asciicasts.rb @@ -0,0 +1,11 @@ +class AddSecretTokenToAsciicasts < ActiveRecord::Migration + def change + add_column :asciicasts, :secret_token, :string + + Asciicast.find_each do |asciicast| + asciicast.update_attribute(:secret_token, SecureRandom.hex.to_i(16).to_s(36)) + end + + change_column :asciicasts, :secret_token, :string, null: false + end +end diff --git a/db/migrate/20141115172558_add_private_to_asciicasts.rb b/db/migrate/20141115172558_add_private_to_asciicasts.rb new file mode 100644 index 0000000..7ec6eea --- /dev/null +++ b/db/migrate/20141115172558_add_private_to_asciicasts.rb @@ -0,0 +1,5 @@ +class AddPrivateToAsciicasts < ActiveRecord::Migration + def change + add_column :asciicasts, :private, :boolean, null: false, default: false + end +end diff --git a/db/migrate/20141115172711_add_index_on_asciicasts_private.rb b/db/migrate/20141115172711_add_index_on_asciicasts_private.rb new file mode 100644 index 0000000..0fa3072 --- /dev/null +++ b/db/migrate/20141115172711_add_index_on_asciicasts_private.rb @@ -0,0 +1,5 @@ +class AddIndexOnAsciicastsPrivate < ActiveRecord::Migration + def change + add_index :asciicasts, :private + end +end diff --git a/db/migrate/20141115174443_add_unique_index_to_asciicasts_secret_token.rb b/db/migrate/20141115174443_add_unique_index_to_asciicasts_secret_token.rb new file mode 100644 index 0000000..df3f92f --- /dev/null +++ b/db/migrate/20141115174443_add_unique_index_to_asciicasts_secret_token.rb @@ -0,0 +1,5 @@ +class AddUniqueIndexToAsciicastsSecretToken < ActiveRecord::Migration + def change + add_index :asciicasts, :secret_token, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index c8575be..3b62c01 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -59,11 +59,15 @@ ActiveRecord::Schema.define(version: 20150401161102) do t.string "image" t.integer "image_width" t.integer "image_height" + t.string "secret_token", null: false + t.boolean "private", default: false, null: false end add_index "asciicasts", ["created_at"], name: "index_asciicasts_on_created_at", using: :btree add_index "asciicasts", ["featured"], name: "index_asciicasts_on_featured", using: :btree add_index "asciicasts", ["likes_count"], name: "index_asciicasts_on_likes_count", using: :btree + add_index "asciicasts", ["private"], name: "index_asciicasts_on_private", using: :btree + add_index "asciicasts", ["secret_token"], name: "index_asciicasts_on_secret_token", unique: true, using: :btree add_index "asciicasts", ["user_id"], name: "index_asciicasts_on_user_id", using: :btree add_index "asciicasts", ["views_count"], name: "index_asciicasts_on_views_count", using: :btree From 5dbe2c84c056c5fa90a432bc41f21f139bea074f Mon Sep 17 00:00:00 2001 From: Marcin Kulik Date: Sat, 15 Nov 2014 18:30:05 +0000 Subject: [PATCH 02/15] UI for toggling asciicasts private/public --- app/models/asciicast.rb | 4 ++ app/policies/asciicast_policy.rb | 19 ++++++++-- app/presenters/asciicast_page_presenter.rb | 8 ++++ app/views/asciicasts/show.html.slim | 10 +++++ spec/policies/asciicast_policy_spec.rb | 44 +++++++++++++++++++++- 5 files changed, 80 insertions(+), 5 deletions(-) diff --git a/app/models/asciicast.rb b/app/models/asciicast.rb index 4ea2c2a..1360d53 100644 --- a/app/models/asciicast.rb +++ b/app/models/asciicast.rb @@ -92,6 +92,10 @@ class Asciicast < ActiveRecord::Base !image.file || (image.file.filename != image_filename) end + def owner?(user) + user && self.user == user + end + private def get_stdout diff --git a/app/policies/asciicast_policy.rb b/app/policies/asciicast_policy.rb index 707c257..b5a985e 100644 --- a/app/policies/asciicast_policy.rb +++ b/app/policies/asciicast_policy.rb @@ -7,9 +7,10 @@ class AsciicastPolicy < ApplicationPolicy end def permitted_attributes - if user.admin? || record.user == user + if user.admin? || record.owner?(user) attrs = [:title, :description, :theme_name, :snapshot_at] attrs << :featured if user.admin? + attrs << :private if record.owner?(user) attrs else @@ -20,13 +21,13 @@ class AsciicastPolicy < ApplicationPolicy def update? return false unless user - user.admin? || record.user == user + user.admin? || record.owner?(user) end def destroy? return false unless user - user.admin? || record.user == user + user.admin? || record.owner?(user) end def feature? @@ -41,4 +42,16 @@ class AsciicastPolicy < ApplicationPolicy user.admin? end + def make_public? + return false unless user + + record.owner?(user) + end + + def make_private? + return false unless user + + record.owner?(user) + end + end diff --git a/app/presenters/asciicast_page_presenter.rb b/app/presenters/asciicast_page_presenter.rb index fa70729..be18b53 100644 --- a/app/presenters/asciicast_page_presenter.rb +++ b/app/presenters/asciicast_page_presenter.rb @@ -94,6 +94,14 @@ class AsciicastPagePresenter asciicast.featured? && policy.unfeature? end + def show_make_private_link? + !asciicast.private? && policy.make_private? + end + + def show_make_public_link? + asciicast.private? && policy.make_public? + end + def show_description? asciicast.description.present? end diff --git a/app/views/asciicasts/show.html.slim b/app/views/asciicasts/show.html.slim index 8825f6f..cfe57f2 100644 --- a/app/views/asciicasts/show.html.slim +++ b/app/views/asciicasts/show.html.slim @@ -55,6 +55,16 @@ = link_to(asciicast_path(page.asciicast, 'asciicast[featured]' => 0), method: :put) do span.glyphicon.glyphicon-eye-close ' Make not featured + - if page.show_make_public_link? + li + = link_to(asciicast_path(page.asciicast, 'asciicast[private]' => 0), method: :put) do + span.glyphicon.glyphicon-eye-open + ' Make public + - if page.show_make_private_link? + li + = link_to(asciicast_path(page.asciicast, 'asciicast[private]' => 1), method: :put) do + span.glyphicon.glyphicon-eye-close + ' Make private - if page.show_delete_link? li = link_to(asciicast_path(page.asciicast), method: :delete, data: { confirm: 'Really delete this asciicast?' }) do diff --git a/spec/policies/asciicast_policy_spec.rb b/spec/policies/asciicast_policy_spec.rb index 443ebac..f333bf7 100644 --- a/spec/policies/asciicast_policy_spec.rb +++ b/spec/policies/asciicast_policy_spec.rb @@ -27,8 +27,8 @@ describe AsciicastPolicy do context "and is creator of the asciicast" do let(:asciicast) { Asciicast.new(user: user) } - it "includes form field, but no featured" do - expect(subject).to eq([:title, :description, :theme_name, :snapshot_at]) + it "doesn't include featured but includes private" do + expect(subject).to eq([:title, :description, :theme_name, :snapshot_at, :private]) end end end @@ -106,4 +106,44 @@ describe AsciicastPolicy do end end + permissions :make_public? do + let(:asciicast) { Asciicast.new } + + it "denies access if user is nil" do + expect(subject).not_to permit(nil, asciicast) + end + + it "grants access if user is owner of the asciicast" do + user = stub_model(User) + asciicast.user = user + expect(subject).to permit(user, asciicast) + end + + it "denies access if user isn't owner of the asciicast" do + user = stub_model(User) + asciicast.user = stub_model(User) + expect(subject).not_to permit(user, asciicast) + end + end + + permissions :make_private? do + let(:asciicast) { Asciicast.new } + + it "denies access if user is nil" do + expect(subject).not_to permit(nil, asciicast) + end + + it "grants access if user is owner of the asciicast" do + user = stub_model(User) + asciicast.user = user + expect(subject).to permit(user, asciicast) + end + + it "denies access if user isn't owner of the asciicast" do + user = stub_model(User) + asciicast.user = stub_model(User) + expect(subject).not_to permit(user, asciicast) + end + end + end From 2c0bfee0cd0769c5b0effb7e5b816d25788b0eb9 Mon Sep 17 00:00:00 2001 From: Marcin Kulik Date: Sat, 15 Nov 2014 19:07:16 +0000 Subject: [PATCH 03/15] Filter out private asciicasts on homepage --- app/models/asciicast.rb | 9 ++++----- app/presenters/home_page_presenter.rb | 4 ++-- spec/presenters/home_page_presenter_spec.rb | 6 +++--- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/app/models/asciicast.rb b/app/models/asciicast.rb index 1360d53..c4469af 100644 --- a/app/models/asciicast.rb +++ b/app/models/asciicast.rb @@ -24,10 +24,9 @@ class Asciicast < ActiveRecord::Base scope :featured, -> { where(featured: true) } scope :by_recency, -> { order("created_at DESC") } scope :by_random, -> { order("RANDOM()") } - scope :latest_limited, -> (n) { by_recency.limit(n).includes(:user) } - scope :random_featured_limited, -> (n) { - featured.by_random.limit(n).includes(:user) - } + scope :non_private, -> { where(private: false) } + scope :homepage_latest, -> { non_private.by_recency.limit(6).includes(:user) } + scope :homepage_featured, -> { non_private.featured.by_random.limit(6).includes(:user) } before_create :generate_secret_token @@ -41,7 +40,7 @@ class Asciicast < ActiveRecord::Base end def self.for_category_ordered(category, order, page = nil, per_page = nil) - collection = all + collection = self.non_private if category == :featured collection = collection.featured diff --git a/app/presenters/home_page_presenter.rb b/app/presenters/home_page_presenter.rb index d37e69d..1e7ac91 100644 --- a/app/presenters/home_page_presenter.rb +++ b/app/presenters/home_page_presenter.rb @@ -11,11 +11,11 @@ class HomePagePresenter end def latest_asciicasts - Asciicast.latest_limited(6).decorate + Asciicast.homepage_latest.decorate end def featured_asciicasts - Asciicast.random_featured_limited(6).decorate + Asciicast.homepage_featured.decorate end def install_script_url diff --git a/spec/presenters/home_page_presenter_spec.rb b/spec/presenters/home_page_presenter_spec.rb index f87bc2b..54b3a5d 100644 --- a/spec/presenters/home_page_presenter_spec.rb +++ b/spec/presenters/home_page_presenter_spec.rb @@ -43,7 +43,7 @@ describe HomePagePresenter do let(:decorated_latest) { double('decorated_latest') } before do - allow(Asciicast).to receive(:latest_limited) { latest } + allow(Asciicast).to receive(:homepage_latest) { latest } end it "returns decorated latest asciicasts" do @@ -58,10 +58,10 @@ describe HomePagePresenter do let(:decorated_featured) { double('decorated_featured') } before do - allow(Asciicast).to receive(:random_featured_limited) { featured } + allow(Asciicast).to receive(:homepage_featured) { featured } end - it "returns decorated random featured asciicasts" do + it "returns decorated featured asciicasts" do expect(subject).to be(decorated_featured) end end From 6bc2fd1048e9299da7f5d5b3a7c407f261666cc2 Mon Sep 17 00:00:00 2001 From: Marcin Kulik Date: Sat, 15 Nov 2014 23:41:14 +0000 Subject: [PATCH 04/15] Pad secret token correctly After converting base 16 (hex) string of length 16 to integer and then to base 36 we can end up with a shorter string due to implicit zeroes on leading positions in the intermediate integer. This ensures "00000000000000000000000000000000", "ffffffffffffffffffffffffffffffff" and everything in between result in a string of length 25 after converting to base 36. --- app/models/asciicast.rb | 2 +- db/migrate/20141115172153_add_secret_token_to_asciicasts.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/asciicast.rb b/app/models/asciicast.rb index c4469af..f312d0f 100644 --- a/app/models/asciicast.rb +++ b/app/models/asciicast.rb @@ -64,7 +64,7 @@ class Asciicast < ActiveRecord::Base end def self.generate_secret_token - SecureRandom.hex.to_i(16).to_s(36) + SecureRandom.hex.to_i(16).to_s(36).rjust(25, '0') end def stdout diff --git a/db/migrate/20141115172153_add_secret_token_to_asciicasts.rb b/db/migrate/20141115172153_add_secret_token_to_asciicasts.rb index 7984244..07d0757 100644 --- a/db/migrate/20141115172153_add_secret_token_to_asciicasts.rb +++ b/db/migrate/20141115172153_add_secret_token_to_asciicasts.rb @@ -3,7 +3,7 @@ class AddSecretTokenToAsciicasts < ActiveRecord::Migration add_column :asciicasts, :secret_token, :string Asciicast.find_each do |asciicast| - asciicast.update_attribute(:secret_token, SecureRandom.hex.to_i(16).to_s(36)) + asciicast.update_attribute(:secret_token, Asciicast.generate_secret_token) end change_column :asciicasts, :secret_token, :string, null: false From 6aeb8810ad6315e455387ea9b8459fac06dc8658 Mon Sep 17 00:00:00 2001 From: Marcin Kulik Date: Sat, 25 Apr 2015 18:47:52 +0000 Subject: [PATCH 05/15] Refactor AsciicastPolicy --- app/policies/asciicast_policy.rb | 20 +++-------- app/presenters/asciicast_page_presenter.rb | 8 ++--- spec/policies/asciicast_policy_spec.rb | 40 ++-------------------- 3 files changed, 10 insertions(+), 58 deletions(-) diff --git a/app/policies/asciicast_policy.rb b/app/policies/asciicast_policy.rb index b5a985e..e959567 100644 --- a/app/policies/asciicast_policy.rb +++ b/app/policies/asciicast_policy.rb @@ -9,8 +9,8 @@ class AsciicastPolicy < ApplicationPolicy def permitted_attributes if user.admin? || record.owner?(user) attrs = [:title, :description, :theme_name, :snapshot_at] - attrs << :featured if user.admin? - attrs << :private if record.owner?(user) + attrs << :featured if change_featured? + attrs << :private if change_visibility? attrs else @@ -30,25 +30,13 @@ class AsciicastPolicy < ApplicationPolicy user.admin? || record.owner?(user) end - def feature? + def change_featured? return false unless user user.admin? end - def unfeature? - return false unless user - - user.admin? - end - - def make_public? - return false unless user - - record.owner?(user) - end - - def make_private? + def change_visibility? return false unless user record.owner?(user) diff --git a/app/presenters/asciicast_page_presenter.rb b/app/presenters/asciicast_page_presenter.rb index be18b53..9ceeb02 100644 --- a/app/presenters/asciicast_page_presenter.rb +++ b/app/presenters/asciicast_page_presenter.rb @@ -87,19 +87,19 @@ class AsciicastPagePresenter end def show_set_featured_link? - !asciicast.featured? && policy.feature? + !asciicast.featured? && policy.change_featured? end def show_unset_featured_link? - asciicast.featured? && policy.unfeature? + asciicast.featured? && policy.change_featured? end def show_make_private_link? - !asciicast.private? && policy.make_private? + !asciicast.private? && policy.change_visibility? end def show_make_public_link? - asciicast.private? && policy.make_public? + asciicast.private? && policy.change_visibility? end def show_description? diff --git a/spec/policies/asciicast_policy_spec.rb b/spec/policies/asciicast_policy_spec.rb index f333bf7..65bf257 100644 --- a/spec/policies/asciicast_policy_spec.rb +++ b/spec/policies/asciicast_policy_spec.rb @@ -74,7 +74,7 @@ describe AsciicastPolicy do end end - permissions :feature? do + permissions :change_featured? do it "denies access if user is nil" do expect(subject).not_to permit(nil, Asciicast.new) end @@ -90,43 +90,7 @@ describe AsciicastPolicy do end end - permissions :unfeature? do - it "denies access if user is nil" do - expect(subject).not_to permit(nil, Asciicast.new) - end - - it "grants access if user is admin" do - user = stub_model(User, admin?: true) - expect(subject).to permit(user, Asciicast.new) - end - - it "denies access if user isn't admin" do - user = stub_model(User, admin?: false) - expect(subject).not_to permit(user, Asciicast.new) - end - end - - permissions :make_public? do - let(:asciicast) { Asciicast.new } - - it "denies access if user is nil" do - expect(subject).not_to permit(nil, asciicast) - end - - it "grants access if user is owner of the asciicast" do - user = stub_model(User) - asciicast.user = user - expect(subject).to permit(user, asciicast) - end - - it "denies access if user isn't owner of the asciicast" do - user = stub_model(User) - asciicast.user = stub_model(User) - expect(subject).not_to permit(user, asciicast) - end - end - - permissions :make_private? do + permissions :change_visibility? do let(:asciicast) { Asciicast.new } it "denies access if user is nil" do From 2c7d549778917ea2d0f572a9c9516b3231bef8b6 Mon Sep 17 00:00:00 2001 From: Marcin Kulik Date: Sun, 26 Apr 2015 13:30:42 +0000 Subject: [PATCH 06/15] Require private asciicasts to be requested via secret token --- app/controllers/api/asciicasts_controller.rb | 2 +- app/controllers/asciicasts_controller.rb | 3 +- app/models/asciicast.rb | 16 +++++ app/models/user.rb | 20 +++++-- app/presenters/asciicast_page_presenter.rb | 6 +- app/presenters/user_page_presenter.rb | 13 ++-- app/views/asciicasts/show.html.slim | 2 +- spec/api/asciicast_show_spec.rb | 38 +++++++----- .../controllers/asciicasts_controller_spec.rb | 8 +-- spec/models/asciicast_spec.rb | 60 ++++++++++++++++++- spec/models/user_spec.rb | 4 +- .../asciicast_page_presenter_spec.rb | 22 +------ spec/presenters/user_page_presenter_spec.rb | 20 +++++-- 13 files changed, 151 insertions(+), 63 deletions(-) diff --git a/app/controllers/api/asciicasts_controller.rb b/app/controllers/api/asciicasts_controller.rb index 285b5dc..a5afab9 100644 --- a/app/controllers/api/asciicasts_controller.rb +++ b/app/controllers/api/asciicasts_controller.rb @@ -19,7 +19,7 @@ module Api end def show - @asciicast = Asciicast.find(params[:id]) + @asciicast = Asciicast.find_by_id_or_secret_token!(params[:id]) respond_with(asciicast) do |format| format.html do diff --git a/app/controllers/asciicasts_controller.rb b/app/controllers/asciicasts_controller.rb index f0eeea7..6f6fd58 100644 --- a/app/controllers/asciicasts_controller.rb +++ b/app/controllers/asciicasts_controller.rb @@ -15,6 +15,7 @@ class AsciicastsController < ApplicationController end def show + # TODO: filter out private or not (????) respond_to do |format| format.html do view_counter.increment(asciicast, cookies) @@ -69,7 +70,7 @@ class AsciicastsController < ApplicationController private def load_resource - @asciicast = Asciicast.find(params[:id]) + @asciicast = Asciicast.find_by_id_or_secret_token!(params[:id]) end def view_counter diff --git a/app/models/asciicast.rb b/app/models/asciicast.rb index f312d0f..96cf9f1 100644 --- a/app/models/asciicast.rb +++ b/app/models/asciicast.rb @@ -30,6 +30,14 @@ class Asciicast < ActiveRecord::Base before_create :generate_secret_token + def self.find_by_id_or_secret_token!(thing) + if thing.size == 25 + find_by_secret_token!(thing) + else + non_private.find(thing) + end + end + def self.cache_key timestamps = scoped.select(:updated_at).map { |o| o.updated_at.to_i } Digest::MD5.hexdigest timestamps.join('/') @@ -67,6 +75,14 @@ class Asciicast < ActiveRecord::Base SecureRandom.hex.to_i(16).to_s(36).rjust(25, '0') end + def to_param + if private? + secret_token + else + id.to_s + end + end + def stdout return @stdout if @stdout @stdout = Stdout::Buffered.new(get_stdout) diff --git a/app/models/user.rb b/app/models/user.rb index 1bedcd6..457e8f7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -103,16 +103,20 @@ class User < ActiveRecord::Base end end + def public_asciicast_count + asciicasts.non_private.count + end + def asciicast_count asciicasts.count end - def asciicasts_excluding(asciicast, limit) - asciicasts.where('id <> ?', asciicast.id).order('RANDOM()').limit(limit) + def other_asciicasts(asciicast, limit) + asciicasts.non_private.where('id <> ?', asciicast.id).order('RANDOM()').limit(limit) end - def paged_asciicasts(page, per_page) - asciicasts. + def paged_asciicasts(page, per_page, include_private) + asciicasts_scope(include_private). includes(:user). order("created_at DESC"). paginate(page, per_page) @@ -134,4 +138,12 @@ class User < ActiveRecord::Base end while self.class.exists?(auth_token: self[:auth_token]) end + def asciicasts_scope(include_private) + if include_private + asciicasts + else + asciicasts.non_private + end + end + end diff --git a/app/presenters/asciicast_page_presenter.rb b/app/presenters/asciicast_page_presenter.rb index 9ceeb02..d105967 100644 --- a/app/presenters/asciicast_page_presenter.rb +++ b/app/presenters/asciicast_page_presenter.rb @@ -118,12 +118,8 @@ class AsciicastPagePresenter end end - def show_other_asciicasts_by_author? - author.asciicast_count > 1 - end - def other_asciicasts_by_author - author.asciicasts_excluding(asciicast, 3).decorate + @other_asciicasts_by_author ||= author.other_asciicasts(asciicast, 3).decorate end def asciicast_oembed_url(format) diff --git a/app/presenters/user_page_presenter.rb b/app/presenters/user_page_presenter.rb index adebd16..820f1e5 100644 --- a/app/presenters/user_page_presenter.rb +++ b/app/presenters/user_page_presenter.rb @@ -39,15 +39,17 @@ class UserPagePresenter def asciicast_count_text(h) if current_users_profile? - if user.asciicast_count > 0 - count = h.pluralize(user.asciicast_count, 'asciicast') + count = user.asciicast_count + if count > 0 + count = h.pluralize(count, 'asciicast') "You have recorded #{count}" else "Record your first asciicast" end else - if user.asciicast_count > 0 - count = h.pluralize(user.asciicast_count, 'asciicast') + count = user.public_asciicast_count + if count > 0 + count = h.pluralize(count, 'asciicast') "#{count} by #{user.display_name}" else "#{user.display_name} hasn't recorded anything yet" @@ -70,7 +72,8 @@ class UserPagePresenter private def get_asciicasts - PaginatingDecorator.new(user.paged_asciicasts(page, per_page)) + asciicasts = user.paged_asciicasts(page, per_page, current_users_profile?) + PaginatingDecorator.new(asciicasts) end end diff --git a/app/views/asciicasts/show.html.slim b/app/views/asciicasts/show.html.slim index cfe57f2..9e29aba 100644 --- a/app/views/asciicasts/show.html.slim +++ b/app/views/asciicasts/show.html.slim @@ -82,7 +82,7 @@ .container .content = page.description - - if page.show_other_asciicasts_by_author? + - unless page.other_asciicasts_by_author.empty? section.even .container .other-asciicasts diff --git a/spec/api/asciicast_show_spec.rb b/spec/api/asciicast_show_spec.rb index 469d788..15bf364 100644 --- a/spec/api/asciicast_show_spec.rb +++ b/spec/api/asciicast_show_spec.rb @@ -1,5 +1,24 @@ require 'rails_helper' +shared_examples_for "asciicast iframe response" do + it "responds with status 200" do + expect(response.status).to eq(200) + end + + it "responds with html content type" do + expect(response.headers['Content-Type']).to match('text/html') + end + + it "responds without X-Frame-Options header" do + pending "the header is added back by Rails only in tests O_o" + expect(response.headers).to_not have_key('Content-Type') + end + + it "responds with player page using iframe layout" do + expect(response.body).to have_selector('body.iframe div.player') + end +end + describe "Asciicast retrieval" do let(:asciicast) { create(:asciicast) } @@ -18,24 +37,15 @@ describe "Asciicast retrieval" do include Capybara::RSpecMatchers before do - get "/api/asciicasts/#{asciicast.id}", format: 'html' - end - - it "responds with status 200" do - expect(response.status).to eq(200) + get "/api/asciicasts/#{asciicast.to_param}", format: 'html' end - it "responds with html content type" do - expect(response.headers['Content-Type']).to match('text/html') - end + it_behaves_like "asciicast iframe response" - it "responds without X-Frame-Options header" do - pending "the header is added back by Rails only in tests O_o" - expect(response.headers).to_not have_key('Content-Type') - end + context "for private asciicast" do + let(:asciicast) { create(:asciicast, private: true) } - it "responds with player page using iframe layout" do - expect(response.body).to have_selector('body.iframe div.player') + it_behaves_like "asciicast iframe response" end end diff --git a/spec/controllers/asciicasts_controller_spec.rb b/spec/controllers/asciicasts_controller_spec.rb index 68db172..4c0afee 100644 --- a/spec/controllers/asciicasts_controller_spec.rb +++ b/spec/controllers/asciicasts_controller_spec.rb @@ -41,7 +41,7 @@ describe AsciicastsController do before do allow(controller).to receive(:view_counter) { view_counter } - expect(Asciicast).to receive(:find).and_return(asciicast) + expect(Asciicast).to receive(:find_by_id_or_secret_token!).and_return(asciicast) end let(:asciicast_presenter) { double('asciicast_presenter') } @@ -74,7 +74,7 @@ describe AsciicastsController do let(:make_request) { get :edit, :id => asciicast.id } before do - expect(Asciicast).to receive(:find).and_return(asciicast) + expect(Asciicast).to receive(:find_by_id_or_secret_token!).and_return(asciicast) asciicast.user = user end @@ -114,7 +114,7 @@ describe AsciicastsController do before do allow(controller).to receive(:asciicast_updater) { asciicast_updater } - expect(Asciicast).to receive(:find).and_return(asciicast) + expect(Asciicast).to receive(:find_by_id_or_secret_token!).and_return(asciicast) asciicast.user = user end @@ -165,7 +165,7 @@ describe AsciicastsController do let(:make_request) { delete :destroy, :id => asciicast.id } before do - expect(Asciicast).to receive(:find).and_return(asciicast) + expect(Asciicast).to receive(:find_by_id_or_secret_token!).and_return(asciicast) asciicast.user = user end diff --git a/spec/models/asciicast_spec.rb b/spec/models/asciicast_spec.rb index c6de820..6843614 100644 --- a/spec/models/asciicast_spec.rb +++ b/spec/models/asciicast_spec.rb @@ -3,6 +3,44 @@ require 'tempfile' describe Asciicast do + describe '.find_by_id_or_secret_token!' do + subject { Asciicast.find_by_id_or_secret_token!(thing) } + + context 'for public asciicast' do + let(:asciicast) { create(:asciicast, private: false) } + + context 'when looked up by id' do + let(:thing) { asciicast.id } + + it { should eq(asciicast) } + end + + context 'when looked up by secret token' do + let(:thing) { asciicast.secret_token } + + it { should eq(asciicast) } + end + end + + context 'for private asciicast' do + let(:asciicast) { create(:asciicast, private: true) } + + context 'when looked up by id' do + let(:thing) { asciicast.id } + + it 'raises RecordNotFound' do + expect { subject }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context 'when looked up by secret token' do + let(:thing) { asciicast.secret_token } + + it { should eq(asciicast) } + end + end + end + describe '.for_category_ordered' do subject { described_class.for_category_ordered(category, order) } @@ -52,7 +90,27 @@ describe Asciicast do end end - let(:asciicast) { described_class.new } + describe '#to_param' do + subject { asciicast.to_param } + + let(:asciicast) { Asciicast.new(id: 123, secret_token: 'sekrit') } + + context 'for public asciicast' do + before do + asciicast.private = false + end + + it { should eq('123') } + end + + context 'for private asciicast' do + before do + asciicast.private = true + end + + it { should eq('sekrit') } + end + end describe '#stdout' do context 'for single-file, JSON asciicast' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 26c99aa..35b4233 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -148,8 +148,8 @@ describe User do it { should eq(2) } end - describe '#asciicasts_excluding' do - subject { user.asciicasts_excluding(asciicast, 1) } + describe '#other_asciicasts' do + subject { user.other_asciicasts(asciicast, 1) } let(:user) { create(:user) } let(:asciicast) { create(:asciicast, user: user) } diff --git a/spec/presenters/asciicast_page_presenter_spec.rb b/spec/presenters/asciicast_page_presenter_spec.rb index 2d39523..1bded65 100644 --- a/spec/presenters/asciicast_page_presenter_spec.rb +++ b/spec/presenters/asciicast_page_presenter_spec.rb @@ -155,26 +155,6 @@ describe AsciicastPagePresenter do it { should eq('i am description') } end - describe '#show_other_asciicasts_by_author?' do - subject { presenter.show_other_asciicasts_by_author? } - - before do - allow(author).to receive(:asciicast_count) { count } - end - - context "when user has more than 1 asciicast" do - let(:count) { 2 } - - it { should be(true) } - end - - context "when user doesn't have more than 1 asciicasts" do - let(:count) { 1 } - - it { should be(false) } - end - end - describe '#other_asciicasts_by_author' do subject { presenter.other_asciicasts_by_author } @@ -182,7 +162,7 @@ describe AsciicastPagePresenter do let(:decorated_others) { double('decorated_others') } before do - allow(author).to receive(:asciicasts_excluding). + allow(author).to receive(:other_asciicasts). with(asciicast, 3) { others } end diff --git a/spec/presenters/user_page_presenter_spec.rb b/spec/presenters/user_page_presenter_spec.rb index 7bae402..5f2a463 100644 --- a/spec/presenters/user_page_presenter_spec.rb +++ b/spec/presenters/user_page_presenter_spec.rb @@ -117,11 +117,23 @@ describe UserPagePresenter do describe '#asciicast_count_text' do subject { presenter.asciicast_count_text(view_context) } - before do - allow(user).to receive(:asciicast_count) { 3 } + context 'for non author' do + before do + allow(user).to receive(:public_asciicast_count) { 2 } + end + + it { should match(/2.+cartman/) } end - it { should eq('3 asciicasts by cartman') } + context 'for author' do + let(:current_user) { user } + + before do + allow(user).to receive(:asciicast_count) { 3 } + end + + it { should match(/you.+3/i) } + end end describe '#user_username' do @@ -143,7 +155,7 @@ describe UserPagePresenter do it "gets user's asciicasts paged" do subject - expect(user).to have_received(:paged_asciicasts).with(2, 5) + expect(user).to have_received(:paged_asciicasts).with(2, 5, false) end it "wraps the asciicasts with paginating decorator" do From 3f32ee0ef6f499fb10cbaf4c3035d434dbdfbffc Mon Sep 17 00:00:00 2001 From: Marcin Kulik Date: Sun, 26 Apr 2015 13:54:23 +0000 Subject: [PATCH 07/15] No "self" needed here --- app/controllers/asciicasts_controller.rb | 1 - app/models/asciicast.rb | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/asciicasts_controller.rb b/app/controllers/asciicasts_controller.rb index 6f6fd58..3808706 100644 --- a/app/controllers/asciicasts_controller.rb +++ b/app/controllers/asciicasts_controller.rb @@ -15,7 +15,6 @@ class AsciicastsController < ApplicationController end def show - # TODO: filter out private or not (????) respond_to do |format| format.html do view_counter.increment(asciicast, cookies) diff --git a/app/models/asciicast.rb b/app/models/asciicast.rb index 96cf9f1..be772ea 100644 --- a/app/models/asciicast.rb +++ b/app/models/asciicast.rb @@ -48,7 +48,7 @@ class Asciicast < ActiveRecord::Base end def self.for_category_ordered(category, order, page = nil, per_page = nil) - collection = self.non_private + collection = non_private if category == :featured collection = collection.featured From 510f16768066c02587fa4a6e960fbef5fc8d8085 Mon Sep 17 00:00:00 2001 From: Marcin Kulik Date: Sun, 26 Apr 2015 13:55:15 +0000 Subject: [PATCH 08/15] Test for Asciicast.generate_secret_token --- spec/models/asciicast_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/models/asciicast_spec.rb b/spec/models/asciicast_spec.rb index 6843614..a823fb1 100644 --- a/spec/models/asciicast_spec.rb +++ b/spec/models/asciicast_spec.rb @@ -41,6 +41,12 @@ describe Asciicast do end end + describe '.generate_secret_token' do + subject { Asciicast.generate_secret_token } + + it { should match(/^[a-z0-9]{25}$/) } + end + describe '.for_category_ordered' do subject { described_class.for_category_ordered(category, order) } From 833c68ad69b6274e3e97309f3d6bb85e80d0a69a Mon Sep 17 00:00:00 2001 From: Marcin Kulik Date: Sun, 26 Apr 2015 13:55:24 +0000 Subject: [PATCH 09/15] Ensure private asciicast doesn't show up on public lists --- spec/models/asciicast_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/models/asciicast_spec.rb b/spec/models/asciicast_spec.rb index a823fb1..1f5b4fb 100644 --- a/spec/models/asciicast_spec.rb +++ b/spec/models/asciicast_spec.rb @@ -62,6 +62,7 @@ describe Asciicast do let!(:asciicast_4) { create(:asciicast, created_at: 3.hours.ago, views_count: 40, featured: true) } + let!(:asciicast_5) { create(:asciicast, private: true) } context "when category is :all" do let(:category) { :all } From c45ee93370e79d37d9026e7bf965d95925ffaad9 Mon Sep 17 00:00:00 2001 From: Marcin Kulik Date: Sun, 26 Apr 2015 14:16:42 +0000 Subject: [PATCH 10/15] More high level specs for private asciicasts --- spec/features/asciicast_spec.rb | 11 +++++++++++ spec/features/playback_spec.rb | 26 +++++++++++++++++++------- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/spec/features/asciicast_spec.rb b/spec/features/asciicast_spec.rb index a6d02b4..951bdc1 100644 --- a/spec/features/asciicast_spec.rb +++ b/spec/features/asciicast_spec.rb @@ -15,4 +15,15 @@ feature "Asciicast page", :js => true do expect(page).to have_selector('.cinema .play-button') end + scenario 'Visiting as guest when asciicast is private' do + asciicast.update(private: true) + + visit asciicast_path(asciicast) + + expect(page).to have_content('the title') + expect(page).to have_link('aaron') + expect(page).to have_link('Embed') + expect(page).to have_selector('.cinema .play-button') + end + end diff --git a/spec/features/playback_spec.rb b/spec/features/playback_spec.rb index 2b30cb4..6fe924a 100644 --- a/spec/features/playback_spec.rb +++ b/spec/features/playback_spec.rb @@ -1,8 +1,6 @@ require 'rails_helper' -describe 'Asciicast playback', :js => true, :slow => true do - - let(:asciicast) { create(:asciicast) } +describe 'Asciicast playback', js: true, slow: true do describe "from fixture" do before do @@ -14,10 +12,24 @@ describe 'Asciicast playback', :js => true, :slow => true do Capybara.default_wait_time = @old_wait_time end - it "is successful" do - visit asciicast_path(asciicast, speed: 5) - find(".start-prompt .play-button").click - expect(page).to have_css('.time-remaining', visible: false, text: '-00:0') + context "for public asciicast" do + let(:asciicast) { create(:asciicast, private: false) } + + it "is successful" do + visit asciicast_path(asciicast, speed: 5) + find(".start-prompt .play-button").click + expect(page).to have_css('.time-remaining', visible: false, text: '-00:0') + end + end + + context "for private asciicast" do + let(:asciicast) { create(:asciicast, private: true) } + + it "is successful" do + visit asciicast_path(asciicast, speed: 5) + find(".start-prompt .play-button").click + expect(page).to have_css('.time-remaining', visible: false, text: '-00:0') + end end end From ae95697a9a73b755bbdfac458aa2c9862dc97147 Mon Sep 17 00:00:00 2001 From: Marcin Kulik Date: Sun, 26 Apr 2015 15:09:30 +0000 Subject: [PATCH 11/15] Add meta tags for hiding referrer on private asciicast pages --- app/serializers/asciicast_serializer.rb | 5 +++++ app/views/asciicasts/_player.html.erb | 7 +++++++ app/views/layouts/bare.html.slim | 1 + app/views/layouts/screenshot.html.slim | 1 + 4 files changed, 14 insertions(+) diff --git a/app/serializers/asciicast_serializer.rb b/app/serializers/asciicast_serializer.rb index add532f..e52b926 100644 --- a/app/serializers/asciicast_serializer.rb +++ b/app/serializers/asciicast_serializer.rb @@ -4,4 +4,9 @@ class AsciicastSerializer < ActiveModel::Serializer attributes :id, :duration, :stdout_frames_url, :snapshot attribute :terminal_columns, key: :width attribute :terminal_lines, key: :height + + def private? + object.private? + end + end diff --git a/app/views/asciicasts/_player.html.erb b/app/views/asciicasts/_player.html.erb index 4d82cbb..2e25b70 100644 --- a/app/views/asciicasts/_player.html.erb +++ b/app/views/asciicasts/_player.html.erb @@ -1,3 +1,10 @@ +<% if asciicast.private? %> + <% content_for(:head) do %> + + + <% end %> +<% end %> +