diff --git a/app/channels/application_cable/connection.rb b/app/channels/application_cable/connection.rb index 251cbd8..8e8a8b7 100644 --- a/app/channels/application_cable/connection.rb +++ b/app/channels/application_cable/connection.rb @@ -11,7 +11,7 @@ def connect protected def find_verified_user - if verified_user = env['warden'].user + if (verified_user = env['warden'].user) verified_user else reject_unauthorized_connection diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 3b9390c..3431203 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,6 +1,6 @@ class UsersController < ApplicationController def show - @user = User.find(params[:id]) + @user = params[:id].to_i == -1 ? System.new : User.find(params[:id]) end def index diff --git a/app/models/activity.rb b/app/models/activity.rb index 55147d6..5bcce28 100644 --- a/app/models/activity.rb +++ b/app/models/activity.rb @@ -13,6 +13,54 @@ class Activity vote.create ].freeze + # outer key: defines the key of an activity ('.' replaced with '_') + # => this way each key can cache different data + # inner key: caching_key + # inner value: method(s) to call on an activity (via send) to get the cache value + CACHE_CONFIG = { + app_create: { + app_name: 'trackable.name', + user_username: 'owner.username' + }, + comment_create: { + comment_content: 'trackable.content', + user_username: 'owner.username', + stampable_name: 'recipient.stampable_name' + }, + domain_create: { + domain_name: 'trackable.name', + user_username: 'owner.username' + }, + stamp_accept: { + stampable_name: 'trackable.stampable_name', + stampable_type: 'trackable.type' + }, + stamp_archive: { + stampable_name: 'trackable.stampable_name', + stampable_type: 'trackable.type' + }, + stamp_create: { + stampable_name: 'trackable.stampable_name', + stampable_type: 'trackable.type' + }, + stamp_deny: { + stampable_name: 'trackable.stampable_name', + stampable_type: 'trackable.type' + }, + stamp_dispute: { + stampable_name: 'trackable.stampable_name', + stampable_type: 'trackable.type' + }, + user_signup: { + user_username: 'trackable.username' + }, + vote_create: { + user_username: 'owner.username', + stampable_name: 'recipient.stampable_name', + stampable_type: 'recipient.type' + } + }.freeze + def persisted? false end diff --git a/app/models/comment.rb b/app/models/comment.rb index ce6283c..23ee2b4 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -1,6 +1,9 @@ class Comment < ApplicationRecord include PublicActivity::Common + # :commentable can currently only be used with stamps + # comment.create activities caches data of stamps, so this would fail if used on another object + # in cache config => comment_create => stampable_name is cached belongs_to :commentable, polymorphic: true belongs_to :user diff --git a/app/models/concerns/votable/state.rb b/app/models/concerns/votable/state.rb index 8c50e90..fe831c2 100644 --- a/app/models/concerns/votable/state.rb +++ b/app/models/concerns/votable/state.rb @@ -2,6 +2,7 @@ module Votable module State extend ActiveSupport::Concern + # rubocop:disable Metrics/BlockLength included do # used so boosts can reference the transition_activity attr_accessor :transition_activity @@ -54,6 +55,7 @@ module State end end end + # rubocop:enable Metrics/BlockLength def scheduled_at created_at + ENVProxy.required_integer('STAMP_CONCLUDE_IN_HOURS').hours diff --git a/app/models/notification.rb b/app/models/notification.rb index 64297e0..6e08b45 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -4,7 +4,7 @@ class Notification < ApplicationRecord belongs_to :recipient, class_name: 'User' belongs_to :reference, polymorphic: true - after_create { NotificationBroadcastWorker.perform_async(self.id) } + after_create { NotificationBroadcastWorker.perform_async(id) } validates_presence_of %i[activity actor recipient reference] diff --git a/app/models/stamp.rb b/app/models/stamp.rb index e3675ec..5ca84c4 100644 --- a/app/models/stamp.rb +++ b/app/models/stamp.rb @@ -27,7 +27,12 @@ def peers? end # must be implemented @ each subclass - # can describe a stronger connection than peers + # -> used to describe the stampable object in the UI + def stampable_name + raise NotImplementedError + end + + # -> can describe a stronger connection than peers def siblings raise NotImplementedError end diff --git a/app/models/stamp/flag.rb b/app/models/stamp/flag.rb index f63862c..82e3daa 100644 --- a/app/models/stamp/flag.rb +++ b/app/models/stamp/flag.rb @@ -12,6 +12,10 @@ def app stampable end + def stampable_name + app.name + end + def siblings peers end diff --git a/app/models/stamp/label.rb b/app/models/stamp/label.rb index de44d08..f76fc94 100644 --- a/app/models/stamp/label.rb +++ b/app/models/stamp/label.rb @@ -14,6 +14,10 @@ def domain stampable end + def stampable_name + domain.name + end + def initial_stamp_cannot_be_0 return true if percentage != 0 return true if siblings.accepted.present? diff --git a/app/models/system.rb b/app/models/system.rb index 3e2337b..4c907ce 100644 --- a/app/models/system.rb +++ b/app/models/system.rb @@ -9,4 +9,12 @@ class System < User def self.polymorphic_name 'System' end + + def created_at + '01-01-2019'.to_date + end + + def updated_at + created_at + end end diff --git a/app/views/activities/_comment.create.html.haml b/app/views/activities/_comment.create.html.haml index 2855de0..2e35841 100644 --- a/app/views/activities/_comment.create.html.haml +++ b/app/views/activities/_comment.create.html.haml @@ -1,4 +1,4 @@ commented on a = link_to activity.recipient_type, activity.recipient of -= link_to activity.recipient.stampable.name, activity.recipient.stampable += link_to activity.recipient.stampable_name, activity.recipient.stampable diff --git a/app/views/activities/_vote.create.html.haml b/app/views/activities/_vote.create.html.haml index 5782d3c..89b68ba 100644 --- a/app/views/activities/_vote.create.html.haml +++ b/app/views/activities/_vote.create.html.haml @@ -1,6 +1,6 @@ voted for a = link_to activity.recipient_type, activity.recipient of -= link_to activity.recipient.stampable.name, activity.recipient.stampable += link_to activity.recipient.stampable_name, activity.recipient.stampable its = activity.recipient.state diff --git a/app/views/notifications/_notification.html.haml b/app/views/notifications/_notification.html.haml index 7973b7a..1fd623c 100644 --- a/app/views/notifications/_notification.html.haml +++ b/app/views/notifications/_notification.html.haml @@ -6,4 +6,4 @@ = render partial: "notifications/#{notification.activity.key}", locals: { notification: notification } = link_to notification.reference_type, notification.reference of - = link_to notification.reference.stampable.name, notification.reference.stampable + = link_to notification.reference.stampable_name, notification.reference.stampable diff --git a/cable/config.ru b/cable/config.ru index 341e724..e049ff1 100644 --- a/cable/config.ru +++ b/cable/config.ru @@ -1,4 +1,4 @@ -require ::File.expand_path('../../config/environment', __FILE__) +require ::File.expand_path('../../config/environment', __FILE__) Rails.application.eager_load! run ActionCable.server diff --git a/config/initializers/public_activity.rb b/config/initializers/public_activity.rb index ce00be5..0ea5abc 100644 --- a/config/initializers/public_activity.rb +++ b/config/initializers/public_activity.rb @@ -3,4 +3,41 @@ has_many :boosts, foreign_key: :trigger_id # a trigger can cause multiple boosts, iE stamp.accept validates :key, presence: true, inclusion: { in: Activity::KEYS } + + # key can only have one dot (.) as defined in Activity::KEYS + def config_key + key.sub('.', '_').to_sym + end + + # if it is not set yet, it will fetch and set the data + # second call already retrieves the cache + # => adding caches in Activity::CACHE_CONFIG is no big deal & performant + def cache(cache_key) + value = parameters[cache_key.to_s] + + return value if value.present? || !cache_key_set?(cache_key) + + set_cache! + cache(cache_key) + end + + def cache_key_set?(cache_key) + Activity::CACHE_CONFIG[config_key][cache_key.to_sym].present? + end + + # if you want to reload all of the caches, use PublicActivity::Activity.all.each(&:set_cache!) + # though it is still performant to just get the caches whenever needed + def set_cache! + Activity::CACHE_CONFIG[config_key]&.each do |cache_key, retrievers| + parameters[cache_key.to_s] = fetch_value(retrievers) + end + + save if changed? + end + + def fetch_value(retrievers) + retrievers.split('.').inject(self) do |cache_value, retriever| + cache_value.send(retriever) + end + end end diff --git a/spec/factories/activity.rb b/spec/factories/activity.rb index 2035f7b..91cc922 100644 --- a/spec/factories/activity.rb +++ b/spec/factories/activity.rb @@ -3,18 +3,22 @@ association :owner, factory: :user end - factory :domain_activity, parent: :activity do - association :trackable, factory: :domain - - key { 'domain.create' } - end - factory :app_activity, parent: :activity do association :trackable, factory: :app - key { 'app.create' } end + factory :comment_activity, parent: :activity do + association :trackable, factory: :comment + recipient { trackable.commentable } + key { 'comment.create' } + end + + factory :domain_activity, parent: :activity do + association :trackable, factory: :domain + key { 'domain.create' } + end + factory :signup_activity, parent: :activity do association :trackable, factory: :user key { 'user.signup' } @@ -34,6 +38,13 @@ key { 'stamp.accept' } end + factory :user_activity, parent: :activity do + association :trackable, factory: :user + owner { trackable } + recipient { nil } + key { 'user.create' } + end + factory :vote_activity, parent: :activity do transient do vote { create(:vote) } diff --git a/spec/factories/comments.rb b/spec/factories/comments.rb index 7fbf122..a2d1395 100644 --- a/spec/factories/comments.rb +++ b/spec/factories/comments.rb @@ -6,7 +6,7 @@ association :commentable, factory: :label_stamp end - # meaning: the comment & commentable were created by the same user + # meaning: the comment & commentable were created by the same user factory :initial_comment, parent: :comment do commentable { create(:label_stamp, creator: user) } end diff --git a/spec/factories/stamps.rb b/spec/factories/stamps.rb index 2ee4b95..51b8298 100644 --- a/spec/factories/stamps.rb +++ b/spec/factories/stamps.rb @@ -26,7 +26,7 @@ end trait :with_comments do - after(:create) do |stamp, evaluator| + after(:create) do |stamp, _| stamp.comments << FactoryBot.build(:comment, commentable: stamp, user_id: stamp.user_id) stamp.comments << FactoryBot.build_list(:comment, 2, commentable: stamp) stamp.save diff --git a/spec/models/activities_spec.rb b/spec/models/activities_spec.rb deleted file mode 100644 index 65a323e..0000000 --- a/spec/models/activities_spec.rb +++ /dev/null @@ -1,25 +0,0 @@ -RSpec.describe PublicActivity::Activity, type: :model do - describe 'relations' do - it { is_expected.to have_one(:boost).with_foreign_key(:cause_id) } - it { is_expected.to have_many(:boosts).with_foreign_key(:trigger_id) } - end - - describe 'database' do - it { is_expected.to have_db_index(%i[trackable_id trackable_type]) } - it { is_expected.to have_db_index(%i[owner_id owner_type]) } - it { is_expected.to have_db_index(%i[recipient_id recipient_type]) } - end - - describe 'validations' do - it { is_expected.to validate_presence_of(:key) } - it { is_expected.to validate_inclusion_of(:key).in_array(Activity::KEYS) } - end - - describe 'view partials' do - Activity::KEYS.each do |key| - it "#{key}.html.haml partial exists" do - expect(File).to exist("app/views/activities/_#{key}.html.haml") - end - end - end -end diff --git a/spec/models/activity_spec.rb b/spec/models/activity_spec.rb new file mode 100644 index 0000000..b03072f --- /dev/null +++ b/spec/models/activity_spec.rb @@ -0,0 +1,131 @@ +RSpec.describe PublicActivity::Activity, type: :model do + describe 'relations' do + it { is_expected.to have_one(:boost).with_foreign_key(:cause_id) } + it { is_expected.to have_many(:boosts).with_foreign_key(:trigger_id) } + end + + describe 'database' do + it { is_expected.to have_db_index(%i[trackable_id trackable_type]) } + it { is_expected.to have_db_index(%i[owner_id owner_type]) } + it { is_expected.to have_db_index(%i[recipient_id recipient_type]) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:key) } + it { is_expected.to validate_inclusion_of(:key).in_array(Activity::KEYS) } + end + + describe '#config_key' do + subject { activity.config_key } + let(:activity) { FactoryBot.create(:signup_activity) } + + it 'replaces the key´s . with _' do + expect(subject).to eq(:user_signup) + end + + it 'returns a symbol' do + expect(subject.class).to eq(Symbol) + end + end + + describe '#cache_key_set?(cache_key)' do + subject { activity.cache_key_set?(cache_key) } + let(:activity) { FactoryBot.create(:app_activity) } + + context 'cache_key is configured' do + let(:cache_key) { :app_name } + + it 'returns true' do + expect(subject).to be true + end + end + + context 'cache_key is not configured' do + let(:cache_key) { :unset_key } + + it 'returns false' do + expect(subject).to be false + end + end + end + + describe '#cache(cache_key)' do + subject { activity.cache(cache_key) } + + let(:activity) { FactoryBot.create(:signup_activity, parameters: parameters, trackable: user) } + let(:user) { FactoryBot.create(:user) } + let(:parameters) { {} } + + context 'cache_key is set' do + let(:cache_key) { :user_username } + + context 'cache is already set' do + let(:parameters) { { user_username: user.username } } + + it 'returns the cached value' do + expect(subject).to eq(user.username) + end + end + + context 'cache is not set yet' do + it 'sets the cache' do + expect { subject }.to change { activity.parameters }.from({}).to( + 'user_username' => user.username + ) + end + + it 'recalls #cache(cache_key) to return the value' do + expect(activity).to receive(:cache).twice.with(cache_key).and_call_original + subject + end + end + end + + context 'cache_key is not set' do + let(:cache_key) { :unset_key } + + it 'returns nil' do + expect(subject).to eq nil + end + end + end + + describe '#fetch_value(retrievers)' do + subject { activity.fetch_value(retrievers) } + + describe 'CACHE_CONFIG' do + Activity::CACHE_CONFIG.each do |config_key, config_hash| + model_type = config_key.to_s.split('_').first + factory_key = "#{model_type}_activity".to_sym + activity_key = config_key.to_s.sub('_', '.') + + context "for '#{activity_key}' activity" do + config_hash.each do |cache_key, retrievers| + let(:activity) { FactoryBot.create(factory_key, key: activity_key) } + let(:retrievers) { retrievers } + + describe "cache_key: #{cache_key}" do + describe "#fetch_value('#{retrievers}')" do + it 'does not raise an error' do + expect { subject }.not_to raise_error + end + + it 'does not return nil' do + expect(subject).not_to eq(nil) + end + end + end + end + end + end + end + end + + describe 'view partials' do + Activity::KEYS.each do |key| + it "#{key}.html.haml partial exists" do + expect(File).to exist("app/views/activities/_#{key}.html.haml") + end + end + end +end diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index e9e4e4e..5246f85 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -60,7 +60,9 @@ expect { subject }.to change { Notification.count }.by(2) end + # rubocop:disable LineLength it 'creates notifications {activity: self, actor: actor, recipient: other commenters, reference: stamp}' do + # rubocop:enable LineLength subject activity = PublicActivity::Activity.last diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 1744f39..752d289 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -21,7 +21,7 @@ it { is_expected.to have_db_index(:activity_id) } it { is_expected.to have_db_index(:actor_id) } it { is_expected.to have_db_index(:recipient_id) } - it { is_expected.to have_db_index([:reference_type, :reference_id]) } + it { is_expected.to have_db_index(%i[reference_type reference_id]) } end describe '#create' do @@ -37,7 +37,7 @@ subject { notification.actor } context 'actor_id is -1' do - let(:notification) { FactoryBot.create(:notification, :system_actor) } + let(:notification) { FactoryBot.create(:notification, :system_actor) } it 'returns the System object' do expect(subject).to eq(System.new) diff --git a/spec/views/semantic/header.html.haml_spec.rb b/spec/views/semantic/header.html.haml_spec.rb index b3458f9..af7c6f1 100644 --- a/spec/views/semantic/header.html.haml_spec.rb +++ b/spec/views/semantic/header.html.haml_spec.rb @@ -13,7 +13,6 @@ def rendered context 'user has notifications' do let(:user) { FactoryBot.create(:user, :with_notifications) } - it 'shows the notification inbox as a purple icon' do expect(rendered).to have_css('.inbox.purple.large.icon') end diff --git a/spec/workers/notification_broadcast_worker_spec.rb b/spec/workers/notification_broadcast_worker_spec.rb index 4e2a908..ca2bd97 100644 --- a/spec/workers/notification_broadcast_worker_spec.rb +++ b/spec/workers/notification_broadcast_worker_spec.rb @@ -2,7 +2,7 @@ describe '#perform' do subject { worker.perform(notification.id) } - let(:worker) { NotificationBroadcastWorker.new() } + let(:worker) { NotificationBroadcastWorker.new } let(:notification) { FactoryBot.create(:notification) } it 'calls NotificationsChannel.broadcast_to' do