From 9c05e618db9aad1334c25ff01d73ee366ddaadff Mon Sep 17 00:00:00 2001 From: rhymes Date: Mon, 9 Dec 2019 22:33:48 +0100 Subject: [PATCH] Improve speed of model tests by around 50/60 seconds (#5016) --- app/models/chat_channel.rb | 2 +- app/models/comment.rb | 40 +- app/models/display_ad.rb | 6 +- app/models/job_opportunity.rb | 20 +- app/models/mention.rb | 9 +- app/models/notification.rb | 4 +- app/models/page.rb | 3 +- app/models/podcast.rb | 3 +- app/models/podcast_episode.rb | 20 +- app/models/poll_skip.rb | 3 +- app/models/rating_vote.rb | 10 +- app/models/reaction.rb | 5 +- app/models/role.rb | 35 +- app/models/tag_adjustment.rb | 8 +- app/models/user.rb | 16 +- app/models/user_block.rb | 3 + spec/decorators/user_decorator_spec.rb | 101 ++ spec/factories/poll_skips.rb | 6 - spec/models/article_spec.rb | 2 +- spec/models/audit_log_spec.rb | 3 - spec/models/badge_achievement_spec.rb | 6 +- spec/models/badge_spec.rb | 4 +- spec/models/block_spec.rb | 7 +- spec/models/buffer_update_spec.rb | 3 +- spec/models/chat_channel_spec.rb | 64 +- spec/models/classified_listing_spec.rb | 6 +- spec/models/collection_spec.rb | 19 +- spec/models/comment_spec.rb | 409 +++---- spec/models/credit_spec.rb | 73 +- spec/models/display_ad_event_spec.rb | 30 +- spec/models/display_ad_spec.rb | 58 +- spec/models/event_spec.rb | 12 +- spec/models/feedback_message_spec.rb | 3 - spec/models/html_variant_spec.rb | 15 +- spec/models/identity_spec.rb | 3 +- spec/models/job_opportunity_spec.rb | 55 +- spec/models/mention_spec.rb | 14 +- spec/models/message_spec.rb | 134 ++- spec/models/notification_spec.rb | 377 +++--- spec/models/notification_subscription_spec.rb | 6 +- spec/models/organization_spec.rb | 57 +- spec/models/page_spec.rb | 47 +- spec/models/page_view_spec.rb | 20 +- spec/models/podcast_episode_spec.rb | 94 +- spec/models/podcast_spec.rb | 99 +- spec/models/poll_option_spec.rb | 21 +- spec/models/poll_skip_spec.rb | 48 +- spec/models/poll_spec.rb | 31 +- spec/models/profile_pin_spec.rb | 65 +- spec/models/rating_vote_spec.rb | 29 +- spec/models/reaction_create_spec.rb | 52 - spec/models/reaction_destroy_spec.rb | 10 - spec/models/reaction_spec.rb | 106 +- spec/models/role_spec.rb | 13 +- spec/models/search_keyword_spec.rb | 2 +- spec/models/sponsorship_spec.rb | 2 +- spec/models/tag_adjustment_spec.rb | 94 +- spec/models/user_block_spec.rb | 9 +- spec/models/user_spec.rb | 1055 ++++++++--------- spec/models/webhook/event_spec.rb | 14 +- 60 files changed, 1846 insertions(+), 1619 deletions(-) create mode 100644 spec/decorators/user_decorator_spec.rb delete mode 100644 spec/models/reaction_create_spec.rb delete mode 100644 spec/models/reaction_destroy_spec.rb diff --git a/app/models/chat_channel.rb b/app/models/chat_channel.rb index d52e56d71..9a2be086c 100644 --- a/app/models/chat_channel.rb +++ b/app/models/chat_channel.rb @@ -44,7 +44,7 @@ class ChatChannel < ApplicationRecord end def clear_channel - messages.delete_all + messages.destroy_all Pusher.trigger(pusher_channels, "channel-cleared", { chat_channel_id: id }.to_json) true rescue Pusher::Error => e diff --git a/app/models/comment.rb b/app/models/comment.rb index 85f5573dc..917c94dc6 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -97,6 +97,19 @@ class Comment < ApplicationRecord end end + def self.users_with_number_of_comments(user_ids, before_date) + joins(:user). + select("users.username, COUNT(comments.user_id) AS number_of_comments"). + where(user_id: user_ids). + where(arel_table[:created_at].gt(before_date)). + group(User.arel_table[:username]). + order("number_of_comments DESC") + end + + def self.tree_for(commentable, limit = 0) + commentable.comments.includes(:user).arrange(order: "score DESC").to_a[0..limit - 1].to_h + end + def self.trigger_index(record, remove) # record is removed from index synchronously in before_destroy_actions return if remove @@ -108,28 +121,12 @@ class Comment < ApplicationRecord end end - def self.users_with_number_of_comments(user_ids, before_date) - joins(:user). - select("users.username, COUNT(comments.user_id) AS number_of_comments"). - where(user_id: user_ids). - where(arel_table[:created_at].gt(before_date)). - group(User.arel_table[:username]). - order("number_of_comments DESC") - end - + # this should remain public because it's called by AlgoliaSearch::AlgoliaJob in .trigger_index def remove_algolia_index remove_from_index! Search::RemoveFromIndexJob.perform_now("ordered_comments_#{Rails.env}", index_id) end - def index_id - "comments-#{id}" - end - - def self.tree_for(commentable, limit = 0) - commentable.comments.includes(:user).arrange(order: "score DESC").to_a[0..limit - 1].to_h - end - def path "/#{user.username}/comment/#{id_code_generated}" rescue StandardError @@ -151,6 +148,8 @@ class Comment < ApplicationRecord end def id_code_generated + # 26 is the conversion base + # eg. 1000.to_s(26) would be "1cc" id.to_s(26) end @@ -164,7 +163,8 @@ class Comment < ApplicationRecord return "[deleted]" if deleted text = ActionController::Base.helpers.strip_tags(processed_html).strip - HTMLEntities.new.decode ActionController::Base.helpers.truncate(text, length: length).gsub("'", "'").gsub("&", "&") + truncated_text = ActionController::Base.helpers.truncate(text, length: length).gsub("'", "'").gsub("&", "&") + HTMLEntities.new.decode(truncated_text) end def video @@ -185,6 +185,10 @@ class Comment < ApplicationRecord private + def index_id + "comments-#{id}" + end + def update_notifications Notification.update_notifications(self) end diff --git a/app/models/display_ad.rb b/app/models/display_ad.rb index 0e36bdf07..7ba594a2d 100644 --- a/app/models/display_ad.rb +++ b/app/models/display_ad.rb @@ -11,10 +11,12 @@ class DisplayAd < ApplicationRecord scope :approved_and_published, -> { where(approved: true, published: true) } def self.for_display(area) + relation = approved_and_published.where(placement_area: area).order(success_rate: :desc) + if rand(8) == 1 - approved_and_published.where(placement_area: area).order("success_rate DESC").sample + relation.sample else - approved_and_published.where(placement_area: area).order("success_rate DESC").limit(rand(1..15)).sample + relation.limit(rand(1..15)).sample end end diff --git a/app/models/job_opportunity.rb b/app/models/job_opportunity.rb index 52bdfd4ab..4e9044475 100644 --- a/app/models/job_opportunity.rb +++ b/app/models/job_opportunity.rb @@ -1,14 +1,16 @@ class JobOpportunity < ApplicationRecord + REMOTENESS_PHRASES = { + "on_premise" => "In Office", + "fully_remote" => "Fully Remote", + "remote_optional" => "Remote Optional", + "on_premise_flexible" => "Mostly in Office but Flexible" + }.freeze + has_many :articles - validates :remoteness, - inclusion: { in: %w[on_premise fully_remote remote_optional on_premise_flexible] } + + validates :remoteness, inclusion: { in: REMOTENESS_PHRASES.keys } + def remoteness_in_words - phrases = { - "on_premise" => "In Office", - "fully_remote" => "Fully Remote", - "remote_optional" => "Remote Optional", - "on_premise_flexible" => "Mostly in Office but Flexible" - } - phrases[remoteness] + REMOTENESS_PHRASES[remoteness] end end diff --git a/app/models/mention.rb b/app/models/mention.rb index 0905deceb..013322c30 100644 --- a/app/models/mention.rb +++ b/app/models/mention.rb @@ -8,14 +8,15 @@ class Mention < ApplicationRecord validates :mentionable_id, presence: true validates :mentionable_type, presence: true validate :permission + after_create :send_email_notification - class << self - def create_all(notifiable) - Mentions::CreateAllJob.perform_later(notifiable.id, notifiable.class.name) - end + def self.create_all(notifiable) + Mentions::CreateAllJob.perform_later(notifiable.id, notifiable.class.name) end + private + def send_email_notification user = User.find(user_id) return unless user.email.present? && user.email_mention_notifications diff --git a/app/models/notification.rb b/app/models/notification.rb index 18e121b70..5c93072ea 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -165,12 +165,12 @@ class Notification < ApplicationRecord end end - # instance methods - def aggregated? action == "Reaction" || action == "Follow" end + private + def mark_notified_at_time self.notified_at = Time.current end diff --git a/app/models/page.rb b/app/models/page.rb index a51cb68e4..74ea373fe 100644 --- a/app/models/page.rb +++ b/app/models/page.rb @@ -36,7 +36,8 @@ class Page < ApplicationRecord end def unique_slug_including_users_and_orgs - errors.add(:slug, "is taken.") if User.find_by(username: slug) || Organization.find_by(slug: slug) || Podcast.find_by(slug: slug) + slug_exists = User.exists?(username: slug) || Organization.exists?(slug: slug) || Podcast.exists?(slug: slug) + errors.add(:slug, "is taken.") if slug_exists end def bust_cache diff --git a/app/models/podcast.rb b/app/models/podcast.rb index 1b0784311..1699209fa 100644 --- a/app/models/podcast.rb +++ b/app/models/podcast.rb @@ -40,7 +40,8 @@ class Podcast < ApplicationRecord private def unique_slug_including_users_and_orgs - errors.add(:slug, "is taken.") if User.find_by(username: slug) || Organization.find_by(slug: slug) || Page.find_by(slug: slug) + slug_exists = User.exists?(username: slug) || Organization.exists?(slug: slug) || Page.exists?(slug: slug) + errors.add(:slug, "is taken.") if slug_exists end def bust_cache diff --git a/app/models/podcast_episode.rb b/app/models/podcast_episode.rb index cf8e5d673..6050a4008 100644 --- a/app/models/podcast_episode.rb +++ b/app/models/podcast_episode.rb @@ -23,7 +23,7 @@ class PodcastEpisode < ApplicationRecord after_destroy :purge, :purge_all after_save :bust_cache - before_validation :prefix_all_images + before_validation :process_html_and_prefix_all_images scope :reachable, -> { where(reachable: true) } scope :published, -> { joins(:podcast).where(podcasts: { published: true }) } @@ -68,10 +68,6 @@ class PodcastEpisode < ApplicationRecord comments.pluck(:body_markdown).join(" ") end - def index_id - "podcast_episodes-#{id}" - end - def path return nil unless podcast&.slug @@ -117,10 +113,6 @@ class PodcastEpisode < ApplicationRecord alias search_score zero_method alias positive_reactions_count zero_method - def bust_cache - PodcastEpisodes::BustCacheJob.perform_later(id, path, podcast_slug) - end - def class_name self.class.name end @@ -143,7 +135,15 @@ class PodcastEpisode < ApplicationRecord private - def prefix_all_images + def index_id + "podcast_episodes-#{id}" + end + + def bust_cache + PodcastEpisodes::BustCacheJob.perform_later(id, path, podcast_slug) + end + + def process_html_and_prefix_all_images return if body.blank? self.processed_html = body. diff --git a/app/models/poll_skip.rb b/app/models/poll_skip.rb index ed2894cd1..6eefafe99 100644 --- a/app/models/poll_skip.rb +++ b/app/models/poll_skip.rb @@ -7,6 +7,7 @@ class PollSkip < ApplicationRecord private def one_vote_per_poll_per_user - errors.add(:base, "cannot vote more than once in one poll") if poll.poll_votes.where(user_id: user_id).any? || poll.poll_skips.where(user_id: user_id).any? + already_voted = poll.poll_votes.where(user_id: user_id).any? || poll.poll_skips.where(user_id: user_id).any? + errors.add(:base, "cannot vote more than once in one poll") if already_voted end end diff --git a/app/models/rating_vote.rb b/app/models/rating_vote.rb index d9d89e8d2..7469719cd 100644 --- a/app/models/rating_vote.rb +++ b/app/models/rating_vote.rb @@ -6,15 +6,19 @@ class RatingVote < ApplicationRecord validates :group, inclusion: { in: %w[experience_level] } validates :rating, numericality: { greater_than: 0.0, less_than_or_equal_to: 10.0 } validate :permissions + counter_culture :article counter_culture :user def assign_article_rating ratings = article.rating_votes.where(group: group).pluck(:rating) average = ratings.sum / ratings.size - article.update_column(:experience_level_rating, average) - article.update_column(:experience_level_rating_distribution, ratings.sort.max - ratings.sort.min) - article.update_column(:last_experience_level_rating_at, Time.current) + + article.update_columns( + experience_level_rating: average, + experience_level_rating_distribution: ratings.max - ratings.min, + last_experience_level_rating_at: Time.current, + ) end private diff --git a/app/models/reaction.rb b/app/models/reaction.rb index 8d5bbb542..4e85f4c22 100644 --- a/app/models/reaction.rb +++ b/app/models/reaction.rb @@ -1,5 +1,6 @@ class Reaction < ApplicationRecord include AlgoliaSearch + CATEGORIES = %w[like readinglist unicorn thinking hands thumbsdown vomit].freeze belongs_to :reactable, polymorphic: true @@ -78,10 +79,6 @@ class Reaction < ApplicationRecord private - def cache_buster - CacheBuster - end - def touch_user Users::TouchJob.perform_later(user_id) end diff --git a/app/models/role.rb b/app/models/role.rb index 20d667f68..d187a5349 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -1,4 +1,20 @@ class Role < ApplicationRecord + ROLES = %w[ + admin + banned + chatroom_beta_tester + comment_banned + podcast_admin + pro + single_resource_admin + super_admin + tag_moderator + tech_admin + trusted + warned + workshop_pass + ].freeze + has_and_belongs_to_many :users, join_table: :users_roles belongs_to :resource, @@ -9,22 +25,7 @@ class Role < ApplicationRecord allow_nil: true validates :name, - inclusion: { - in: %w[ - super_admin - admin - single_resource_admin - tech_admin - tag_moderator - trusted - banned - warned - workshop_pass - chatroom_beta_tester - comment_banned - pro - podcast_admin - ] - } + inclusion: { in: ROLES } + scopify end diff --git a/app/models/tag_adjustment.rb b/app/models/tag_adjustment.rb index f028563a2..bb99fea21 100644 --- a/app/models/tag_adjustment.rb +++ b/app/models/tag_adjustment.rb @@ -20,8 +20,10 @@ class TagAdjustment < ApplicationRecord end def has_privilege_to_adjust? - user&.has_role?(:tag_moderator, tag) || - user&.has_role?(:admin) || - user&.has_role?(:super_admin) + return false unless user + + user.has_role?(:tag_moderator, tag) || + user.has_role?(:admin) || + user.has_role?(:super_admin) end end diff --git a/app/models/user.rb b/app/models/user.rb index 5d1e01306..09d4aa838 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -211,19 +211,11 @@ class User < ApplicationRecord summary end - def index_id - "users-#{id}" - end - def set_remember_fields self.remember_token ||= self.class.remember_token if respond_to?(:remember_token) self.remember_created_at ||= Time.now.utc end - def estimate_default_language - Users::EstimateDefaultLanguageJob.perform_later(id) - end - def calculate_score score = (articles.where(featured: true).size * 100) + comments.sum(:score) update_column(:score, score) @@ -461,6 +453,14 @@ class User < ApplicationRecord private + def index_id + "users-#{id}" + end + + def estimate_default_language + Users::EstimateDefaultLanguageJob.perform_later(id) + end + def set_default_language language_settings["preferred_languages"] ||= ["en"] end diff --git a/app/models/user_block.rb b/app/models/user_block.rb index 1ff2fc830..7c0ef09f9 100644 --- a/app/models/user_block.rb +++ b/app/models/user_block.rb @@ -15,6 +15,9 @@ class UserBlock < ApplicationRecord exists?(blocker_id: blocker_id, blocked_id: blocked_id) end end + + private + def blocker_cannot_be_same_as_blocked errors.add(:blocker_id, "can't be the same as the blocked_id") if blocker_id == blocked_id end diff --git a/spec/decorators/user_decorator_spec.rb b/spec/decorators/user_decorator_spec.rb new file mode 100644 index 000000000..0cea03c55 --- /dev/null +++ b/spec/decorators/user_decorator_spec.rb @@ -0,0 +1,101 @@ +require "rails_helper" + +RSpec.describe UserDecorator, type: :decorator do + let(:user) { build_stubbed(:user) } + + describe "#cached_followed_tags" do + let_it_be(:user) { create(:user) } + let(:tag1) { create(:tag) } + let(:tag2) { create(:tag) } + let(:tag3) { create(:tag) } + + it "returns empty if no tags followed" do + expect(user.decorate.cached_followed_tags.size).to eq(0) + end + + it "returns array of tags if user follows them" do + user.follow(tag1) + user.follow(tag2) + user.follow(tag3) + expect(user.decorate.cached_followed_tags.size).to eq(3) + end + + it "returns tag object with name" do + user.follow(tag1) + expect(user.decorate.cached_followed_tags.first.name).to eq(tag1.name) + end + + it "returns follow points for tag" do + user.follow(tag1) + expect(user.decorate.cached_followed_tags.first.points).to eq(1.0) + end + + it "returns adjusted points for tag" do + follow = user.follow(tag1) + follow.update(points: 0.1) + expect(user.decorate.cached_followed_tags.first.points).to eq(0.1) + end + end + + describe "#config_body_class" do + it "creates proper body class with defaults" do + expect(user.decorate.config_body_class).to eq("default default-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} #{user.config_navbar}-navbar-config") + end + + it "creates proper body class with sans serif config" do + user.config_font = "sans_serif" + expect(user.decorate.config_body_class).to eq("default sans-serif-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} #{user.config_navbar}-navbar-config") + end + + it "creates proper body class with night theme" do + user.config_theme = "night_theme" + expect(user.decorate.config_body_class).to eq("night-theme default-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} #{user.config_navbar}-navbar-config") + end + + it "creates proper body class with pink theme" do + user.config_theme = "pink_theme" + expect(user.decorate.config_body_class).to eq("pink-theme default-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} #{user.config_navbar}-navbar-config") + end + + it "creates proper body class with minimal light theme" do + user.config_theme = "minimal_light_theme" + expect(user.decorate.config_body_class).to eq("minimal-light-theme default-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} #{user.config_navbar}-navbar-config") + end + + it "works with static navbar" do + user.config_navbar = "static" + expect(user.decorate.config_body_class).to eq("default default-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} static-navbar-config") + end + + context "when user with roles" do + let(:user) { create(:user) } + + it "creates proper body class with pro user" do + user.add_role(:pro) + expect(user.decorate.config_body_class).to eq("default default-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} #{user.config_navbar}-navbar-config") + end + + it "creates proper body class with trusted user" do + user.add_role(:trusted) + expect(user.decorate.config_body_class).to eq("default default-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} #{user.config_navbar}-navbar-config") + end + end + end + + describe "#dark_theme?" do + it "determines dark theme if night theme" do + user.config_theme = "night_theme" + expect(user.decorate.dark_theme?).to eq(true) + end + + it "determines dark theme if ten x hacker" do + user.config_theme = "ten_x_hacker_theme" + expect(user.decorate.dark_theme?).to eq(true) + end + + it "determines not dark theme if not one of the dark themes" do + user.config_theme = "default" + expect(user.decorate.dark_theme?).to eq(false) + end + end +end diff --git a/spec/factories/poll_skips.rb b/spec/factories/poll_skips.rb index 17b242431..48d359ba2 100644 --- a/spec/factories/poll_skips.rb +++ b/spec/factories/poll_skips.rb @@ -1,10 +1,4 @@ FactoryBot.define do factory :poll_skip do - # prompt_markdown { Faker::Hipster.words(number: 5) } - # factory :poll_with_options do - # after(:create) do |poll| - # create_list(:poll_option, poll: poll) - # end - # end end end diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index 7f82700f1..3c4dceea1 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -330,7 +330,7 @@ RSpec.describe Article, type: :model do end end - describe "published_timestamp" do + describe "#published_timestamp" do it "returns empty string if the article is not published" do article.published = false expect(article.published_timestamp).to be_empty diff --git a/spec/models/audit_log_spec.rb b/spec/models/audit_log_spec.rb index 622ac3017..077642696 100644 --- a/spec/models/audit_log_spec.rb +++ b/spec/models/audit_log_spec.rb @@ -1,9 +1,6 @@ require "rails_helper" RSpec.describe AuditLog, type: :model do - let(:user) { create(:user) } - let(:audit_log) { create(:audit_log, user_id: user.id) } - it { is_expected.to belong_to(:user) } it { is_expected.to validate_presence_of(:user_id) } end diff --git a/spec/models/badge_achievement_spec.rb b/spec/models/badge_achievement_spec.rb index c23d6520d..6e3c76db8 100644 --- a/spec/models/badge_achievement_spec.rb +++ b/spec/models/badge_achievement_spec.rb @@ -1,9 +1,9 @@ require "rails_helper" RSpec.describe BadgeAchievement, type: :model do - describe "validations" do - subject { create(:badge_achievement) } + let_it_be(:achievement) { create(:badge_achievement) } + describe "validations" do it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:badge) } it { is_expected.to belong_to(:rewarder).class_name("User").optional } @@ -11,12 +11,10 @@ RSpec.describe BadgeAchievement, type: :model do end it "turns rewarding_context_message_markdown into rewarding_context_message HTML" do - achievement = create(:badge_achievement) expect(achievement.rewarding_context_message).to include("") end it "awards credits after create" do - achievement = create(:badge_achievement) expect(achievement.user.credits.size).to eq(5) end end diff --git a/spec/models/badge_spec.rb b/spec/models/badge_spec.rb index be76bd2fc..dad1e6364 100644 --- a/spec/models/badge_spec.rb +++ b/spec/models/badge_spec.rb @@ -1,9 +1,9 @@ require "rails_helper" RSpec.describe Badge, type: :model do - describe "validations" do - subject { create(:badge) } + let_it_be(:badge) { create(:badge) } + describe "validations" do it { is_expected.to have_many(:users).through(:badge_achievements) } it { is_expected.to have_many(:badge_achievements) } it { is_expected.to validate_presence_of(:title) } diff --git a/spec/models/block_spec.rb b/spec/models/block_spec.rb index 04d2607fc..999b5a760 100644 --- a/spec/models/block_spec.rb +++ b/spec/models/block_spec.rb @@ -1,8 +1,8 @@ require "rails_helper" RSpec.describe Block, type: :model do - let(:user) { create(:user) } - let(:block) { described_class.new(user: user, input_html: "hello") } + let_it_be(:user) { create(:user) } + let_it_be(:block) { described_class.new(user: user, input_html: "hello") } it "creates processed_html after published!" do user.add_role(:super_admin) @@ -11,8 +11,7 @@ RSpec.describe Block, type: :model do end it "is not valid without user" do - block.user = nil - expect(block).not_to be_valid + expect(described_class.new(user: nil)).not_to be_valid end it "is not valid with non-admin user" do diff --git a/spec/models/buffer_update_spec.rb b/spec/models/buffer_update_spec.rb index 4dc017a02..763493322 100644 --- a/spec/models/buffer_update_spec.rb +++ b/spec/models/buffer_update_spec.rb @@ -1,8 +1,7 @@ require "rails_helper" RSpec.describe BufferUpdate, type: :model do - let(:user) { create(:user) } - let(:article) { create(:article, user_id: user.id) } + let_it_be(:article) { create(:article) } it "creates update" do described_class.buff!(article.id, "twitter_buffer_text", "CODE", "twitter") diff --git a/spec/models/chat_channel_spec.rb b/spec/models/chat_channel_spec.rb index e9e9226e8..e836d74fb 100644 --- a/spec/models/chat_channel_spec.rb +++ b/spec/models/chat_channel_spec.rb @@ -2,44 +2,56 @@ require "rails_helper" RSpec.describe ChatChannel, type: :model do let(:chat_channel) { create(:chat_channel) } - let(:message) { create(:chat_channel, message_id: chat_channel.id) } + + let_it_be(:users) { create_list(:user, 2) } it { is_expected.to have_many(:messages) } it { is_expected.to validate_presence_of(:channel_type) } - it "clears chat" do - allow(Pusher).to receive(:trigger) - chat_channel.clear_channel - expect(chat_channel.messages.size).to eq(0) + describe "#clear_channel" do + before { allow(Pusher).to receive(:trigger) } + + it "clears chat" do + create(:message, chat_channel: chat_channel, user: create(:user)) + chat_channel.reload + expect(chat_channel.messages.size).to be_positive + chat_channel.clear_channel + expect(chat_channel.messages.size).to eq(0) + end end - it "creates channel with users" do - chat_channel = described_class.create_with_users([create(:user), create(:user)]) - expect(chat_channel.users.size).to eq(2) - expect(chat_channel.has_member?(User.first)).to eq(true) + describe "#create_with_users" do + it "creates channel with users" do + chat_channel = described_class.create_with_users(users) + expect(chat_channel.users.size).to eq(users.size) + expect(chat_channel.has_member?(users.first)).to be(true) + expect(chat_channel.has_member?(users.last)).to be(true) + end + + it "lists active memberships" do + chat_channel = described_class.create_with_users(users) + expect(chat_channel.active_users.size).to eq(users.size) + expect(chat_channel.channel_users.size).to eq(users.size) + end end - it "lists active memberships" do - chat_channel = described_class.create_with_users([create(:user), create(:user)]) - expect(chat_channel.active_users.size).to eq(2) - expect(chat_channel.channel_users.size).to eq(2) - end - - it "decreases active users if one leaves" do - chat_channel = described_class.create_with_users([create(:user), create(:user)]) - ChatChannelMembership.last.update(status: "left_channel") - expect(chat_channel.active_users.size).to eq(1) - expect(chat_channel.channel_users.size).to eq(1) + describe "#active_users" do + it "decreases active users if one leaves" do + chat_channel = described_class.create_with_users(users) + expect(chat_channel.active_users.size).to eq(users.size) + expect(chat_channel.channel_users.size).to eq(users.size) + ChatChannelMembership.last.update(status: "left_channel") + expect(chat_channel.active_users.size).to eq(users.size - 1) + expect(chat_channel.channel_users.size).to eq(users.size - 1) + end end describe "#remove_user" do - let(:user) { create(:user) } - it "removes a user from a channel" do - chat_channel.add_users(user) - expect(chat_channel.chat_channel_memberships.exists?(user_id: user.id)).to be(true) - chat_channel.remove_user(user) - expect(chat_channel.chat_channel_memberships.exists?(user_id: user.id)).to be(false) + chat_channel.add_users(users.first) + expect(chat_channel.chat_channel_memberships.exists?(user_id: users.first.id)).to be(true) + chat_channel.remove_user(users.first) + expect(chat_channel.chat_channel_memberships.exists?(user_id: users.first.id)).to be(false) end end end diff --git a/spec/models/classified_listing_spec.rb b/spec/models/classified_listing_spec.rb index 51f3991e1..3a2ba9949 100644 --- a/spec/models/classified_listing_spec.rb +++ b/spec/models/classified_listing_spec.rb @@ -1,9 +1,9 @@ require "rails_helper" RSpec.describe ClassifiedListing, type: :model do - let(:classified_listing) { create(:classified_listing, user_id: user.id) } - let(:user) { create(:user) } - let(:organization) { create(:organization) } + let_it_be(:user) { create(:user) } + let_it_be(:organization) { create(:organization) } + let(:classified_listing) { create(:classified_listing, user: user) } it { is_expected.to validate_presence_of(:title) } it { is_expected.to validate_presence_of(:body_markdown) } diff --git a/spec/models/collection_spec.rb b/spec/models/collection_spec.rb index 934f2e998..b8cb33c56 100644 --- a/spec/models/collection_spec.rb +++ b/spec/models/collection_spec.rb @@ -1,12 +1,10 @@ require "rails_helper" RSpec.describe Collection, type: :model do - let(:user) { create(:user) } - let(:collection) { create(:collection, :with_articles, user: user) } + let_it_be(:user) { create(:user) } + let_it_be(:collection) { create(:collection, :with_articles, user: user) } describe "validations" do - subject { described_class.new } - it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:organization).optional } it { is_expected.to have_many(:articles) } @@ -17,11 +15,10 @@ RSpec.describe Collection, type: :model do end describe ".find_series" do - let(:user) { create(:user) } - let(:series) { create(:collection, user: user) } + let_it_be(:other_user) { create(:user) } + let_it_be(:series) { collection } it "returns an existing series" do - series # the series has to be created before the following expect expect do expect(described_class.find_series(series.slug, series.user)).to eq(series) end.not_to change(described_class, :count) @@ -29,13 +26,11 @@ RSpec.describe Collection, type: :model do it "creates a new series for a user if an existing one is not found" do slug = Faker::Books::CultureSeries.book - expect { described_class.find_series(slug, user) }.to change(described_class, :count).by(1) + expect { described_class.find_series(slug, other_user) }.to change(described_class, :count).by(1) end it "creates a new series with an existing slug for a new user" do - user = create(:user) - series # the series has to be created before the following expect - expect { described_class.find_series(series.slug, user) }.to change(described_class, :count).by(1) + expect { described_class.find_series(series.slug, other_user) }.to change(described_class, :count).by(1) end end @@ -44,7 +39,7 @@ RSpec.describe Collection, type: :model do Timecop.freeze(DateTime.parse("2019/10/24")) do allow(collection.articles).to receive(:update_all) collection.touch_articles - expect(collection.articles).to have_received(:update_all).with(updated_at: Time.zone.now) + expect(collection.articles).to have_received(:update_all).with(updated_at: Time.current) end end end diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index fd26e5e39..249076389 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -1,26 +1,13 @@ require "rails_helper" RSpec.describe Comment, type: :model do - let(:user) { create(:user, created_at: 3.weeks.ago) } - let(:user2) { create(:user) } - let(:article) { create(:article, user_id: user.id, published: true) } - let(:article_with_video) { create(:article, :video, user_id: user.id, published: true) } - let(:comment) { create(:comment, user_id: user2.id, commentable_id: article.id) } - let(:video_comment) { create(:comment, user_id: user2.id, commentable_id: article_with_video.id) } - let(:comment_2) { create(:comment, user_id: user2.id, commentable_id: article.id) } - let(:child_comment) do - build(:comment, user_id: user.id, commentable_id: article.id, parent_id: comment.id) - end + let_it_be(:user) { create(:user) } + let_it_be(:article) { create(:article, user: user) } + let_it_be(:comment) { create(:comment, user: user, commentable: article) } + + include_examples "#sync_reactions_count", :article_comment describe "validations" do - subject { described_class.new(user: user, commentable: article) } - - let(:article) { Article.new(user: user2) } - - before do - allow(article).to receive(:touch).and_return(true) - end - it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:commentable) } it { is_expected.to have_many(:reactions).dependent(:destroy) } @@ -28,135 +15,158 @@ RSpec.describe Comment, type: :model do it { is_expected.to have_many(:notifications).dependent(:delete_all) } it { is_expected.to have_many(:notification_subscriptions).dependent(:destroy) } it { is_expected.to validate_presence_of(:commentable_id) } - it { is_expected.to validate_presence_of(:body_markdown) } - it { is_expected.to validate_uniqueness_of(:body_markdown).scoped_to(:user_id, :ancestry, :commentable_id, :commentable_type) } + + it do + # rubocop:disable RSpec/NamedSubject + subject.commentable = article + subject.user = user + expect(subject).to( + validate_uniqueness_of(:body_markdown).scoped_to(:user_id, :ancestry, :commentable_id, :commentable_type), + ) + # rubocop:enable RSpec/NamedSubject + end + it { is_expected.to validate_length_of(:body_markdown).is_at_least(1).is_at_most(25_000) } it { is_expected.to validate_inclusion_of(:commentable_type).in_array(%w[Article PodcastEpisode]) } - end - it "gets proper generated ID code" do - comment = described_class.new(id: 1) - expect(comment.id_code_generated).to eq(comment.id.to_s(26)) - end + it "is invalid if commentable is unpublished article" do + # rubocop:disable RSpec/NamedSubject + subject.commentable = build(:article, published: false) + expect(subject).not_to be_valid + # rubocop:enable RSpec/NamedSubject + end - it "generates character count before saving" do - expect(comment.markdown_character_count).to eq(comment.body_markdown.size) - end + describe "#processed_html" do + let(:comment) { build(:comment, user: user, commentable: article) } - describe "#processed_html" do - let(:comment) { create(:comment, commentable: article, body_markdown: "# hello\n\nhy hey hey") } + it "converts body_markdown to proper processed_html" do + comment.body_markdown = "# hello\n\nhy hey hey" + comment.validate! + expect(comment.processed_html.include?("

")).to be(true) + end - it "converts body_markdown to proper processed_html" do - expect(comment.processed_html.include?("

")).to eq(true) + it "adds rel=nofollow to links" do + comment.body_markdown = "this is a comment with a link: http://dev.to" + comment.validate! + expect(comment.processed_html.include?('rel="nofollow"')).to be(true) + end + + it "adds a mention url if user is mentioned like @mention" do + comment.body_markdown = "Hello @#{user.username}, you are cool." + comment.validate! + expect(comment.processed_html.include?("/#{user.username}")).to be(true) + expect(comment.processed_html.include?("href")).to be(true) + expect(comment.processed_html.include?("Hello ")).to be(true) + expect(comment.processed_html.size < 450).to be(true) + end + + it "adds timestamp url if commentable has video and timestamp", :aggregate_failures do + article.video = "https://example.com" + + comment.body_markdown = "I like the part at 4:30" + comment.validate! + expect(comment.processed_html.include?(">4:30")).to be(true) + + comment.body_markdown = "I like the part at 4:30 and 5:50" + comment.validate! + expect(comment.processed_html.include?(">5:50")).to eq(true) + + comment.body_markdown = "I like the part at 5:30 and :55" + comment.validate! + expect(comment.processed_html.include?(">:55")).to eq(true) + + comment.body_markdown = "I like the part at 52:30" + comment.validate! + expect(comment.processed_html.include?(">52:30")).to eq(true) + + comment.body_markdown = "I like the part at 1:52:30 and 1:20" + comment.validate! + expect(comment.processed_html.include?(">1:52:30")).to eq(true) + expect(comment.processed_html.include?(">1:20")).to eq(true) + end + + it "does not add timestamp if commentable does not have video" do + article.video = nil + + comment.body_markdown = "I like the part at 1:52:30 and 1:20" + comment.validate! + expect(comment.processed_html.include?(">1:52:30")).to eq(false) + end end end - it "adds timestamp url if commentable has video and timestamp" do - video_comment.body_markdown = "I like the part at 4:30" - video_comment.save - expect(video_comment.processed_html.include?(">4:30")).to eq(true) - video_comment.body_markdown = "I like the part at 4:30 and 5:50" - video_comment.save - expect(video_comment.processed_html.include?(">5:50")).to eq(true) - video_comment.body_markdown = "I like the part at 5:30 and :55" - video_comment.save - expect(video_comment.processed_html.include?(">:55")).to eq(true) - video_comment.body_markdown = "I like the part at 52:30" - video_comment.save - expect(video_comment.processed_html.include?(">52:30")).to eq(true) - video_comment.body_markdown = "I like the part at 1:52:30 and 1:20" - video_comment.save - expect(video_comment.processed_html.include?(">1:52:30")).to eq(true) - expect(video_comment.processed_html.include?(">1:20")).to eq(true) + describe "#id_code_generated" do + it "gets proper generated ID code" do + expect(described_class.new(id: 1000).id_code_generated).to eq("1cc") + end end - it "does not add timestamp if commentable does not have video" do - comment.body_markdown = "I like the part at 1:52:30 and 1:20" - comment.save - expect(comment.processed_html.include?(">1:52:30")).to eq(false) + describe "#readable_publish_date" do + it "does not show year in readable time if not current year" do + expect(comment.readable_publish_date).to eq(comment.created_at.strftime("%b %e")) + end + + it "shows year in readable time if not current year" do + comment.created_at = 1.year.ago + last_year = 1.year.ago.year % 100 + expect(comment.readable_publish_date.include?("'#{last_year}")).to eq(true) + end end - it "adds rel=nofollow to links" do - comment.body_markdown = "this is a comment with a link: http://dev.to" - comment.save - expect(comment.processed_html.include?('rel="nofollow"')).to eq(true) + describe "#path" do + it "returns the properly formed path" do + expect(comment.path).to eq("/#{comment.user.username}/comment/#{comment.id_code_generated}") + end end - it "adds a mention url if user is mentioned like @mention" do - comment.body_markdown = "Hello @#{user.username}, you are cool." - comment.save - expect(comment.processed_html.include?("/#{user.username}")).to eq(true) - expect(comment.processed_html.include?("href")).to eq(true) - expect(comment.processed_html.include?("Hello ")).to eq(true) - expect(comment.processed_html.size).to be < 450 - end - - it "does not show year in readable time if not current year" do - expect(comment.readable_publish_date).to eq(comment.created_at.strftime("%b %e")) - end - - it "shows year in readable time if not current year" do - comment.created_at = 1.year.ago - last_year = 1.year.ago.year % 100 - expect(comment.readable_publish_date.include?("'#{last_year}")).to eq(true) - end - - it "returns a path" do - expect(comment.path).not_to be(nil) - end - - it "returns the properly formed path" do - expect(comment.path).to eq("/#{comment.user.username}/comment/#{comment.id_code_generated}") - end - - it "returns root article if no parent comment" do - expect(comment.parent_or_root_article).to eq(comment.commentable) - end - - it "returns root parent comment if exists" do - expect(child_comment.parent_or_root_article).to eq(comment) - end - - it "properly indexes" do - comment.index! + it "returns root parent comment if exists" do + child_comment = build(:comment, parent: comment) + expect(child_comment.parent_or_root_article).to eq(comment) + end end describe "#parent_user" do @@ -165,7 +175,10 @@ RSpec.describe Comment, type: :model do end it "returns the root parent comment's user if root parent comment exists" do - expect(child_comment.parent_user).to eq(user2) + child_comment_user = build(:user) + child_comment = build(:comment, parent: comment, user: child_comment_user) + expect(child_comment.parent_user).not_to eq(child_comment_user) + expect(child_comment.parent_user).to eq(comment.user) end end @@ -179,129 +192,123 @@ RSpec.describe Comment, type: :model do end it "retains content from #processed_html" do - comment.update_column(:processed_html, "Hello this is a post.") # Remove randomness + comment.processed_html = "Hello this is a post." # Remove randomness + comment.validate! text = comment.title.gsub("...", "").delete("\n") - expect(comment.processed_html).to include CGI.unescapeHTML(text) + expect(comment.processed_html).to include(CGI.unescapeHTML(text)) end it "is converted to deleted if the comment is deleted" do - comment.update_column(:deleted, true) - expect(comment.title).to eq "[deleted]" + comment.deleted = true + expect(comment.title).to eq("[deleted]") end it "does not contain the wrong encoding" do comment.body_markdown = "It's the best post ever. It's so great." - comment.save - expect(comment.title).not_to include "'" + comment.validate! + expect(comment.title).not_to include("'") + end + end + + describe "#index_id" do + it "is equal to comments-ID" do + # NOTE: we shouldn't test private things but cheating a bit for Algolia here + expect(comment.send(:index_id)).to eq("comments-#{comment.id}") end end describe "#custom_css" do - it "returns nothing when no ltag was used" do - expect(comment.custom_css).to eq("") + it "returns nothing when no liquid tag was used" do + expect(comment.custom_css).to be_blank end it "returns proper liquid tag classes if used" do text = "{% devcomment #{comment.id_code_generated} %}" - ltag_comment = create(:comment, commentable_id: create(:article).id, body_markdown: text) - expect(ltag_comment.custom_css).not_to eq("") + comment.body_markdown = text + expect(comment.custom_css).to be_present end end - describe "validity" do - it "is invalid if commentable is unpublished article" do - article.update_column(:published, false) - comment = build(:comment, user_id: user.id, commentable_id: article.id) - expect(comment).not_to be_valid - end - end + describe ".tree_for" do + let_it_be(:other_comment) { create(:comment, commentable: article, user: user) } + let_it_be(:child_comment) { create(:comment, commentable: article, parent: comment, user: user) } - describe "tree" do - let(:article2) { create(:article) } - let!(:tree_comment) { create(:comment, commentable: article2) } - let!(:child) { create(:comment, commentable: article2, parent: tree_comment) } - let!(:tree_comment2) { create(:comment, commentable: article2) } - - before { tree_comment.update_column(:score, 1) } + before { comment.update_column(:score, 1) } it "returns a full tree" do - comments = described_class.tree_for(article2) - expect(comments).to eq(tree_comment => { child => {} }, tree_comment2 => {}) + comments = described_class.tree_for(article) + expect(comments).to eq(comment => { child_comment => {} }, other_comment => {}) end it "returns part of the tree" do - comments = described_class.tree_for(article2, 1) - expect(comments).to eq(tree_comment => { child => {} }) + comments = described_class.tree_for(article, 1) + expect(comments).to eq(comment => { child_comment => {} }) end end - describe "deleted" do - let(:child_comment) { create(:comment, commentable: article, parent: comment, user: user) } + context "when callbacks are triggered before save" do + it "generates character count before saving" do + comment.save + expect(comment.markdown_character_count).to eq(comment.body_markdown.size) + end + end - it "deletes the comment's notifications" do - create(:notification, notifiable: comment, user: user2) - create(:notification, notifiable: child_comment, user: user) + context "when callbacks are triggered after save" do + it "updates user last comment date" do + expect { comment.save }.to change(user, :last_comment_at) + end + end + + context "when callbacks are triggered after update" do + it "deletes the comment's notifications when deleted is set to true" do + create(:notification, notifiable: comment, user: user) perform_enqueued_jobs do - child_comment.update(deleted: true) - expect(child_comment.notifications).to be_empty + comment.update(deleted: true) end + expect(comment.notifications).to be_empty end - it "updates the notifications of the ancestors and descendants" do - create(:notification, notifiable: comment, user: user2) + it "updates the notifications of the descendants with [deleted]" do + comment = create(:comment, commentable: article) + child_comment = create(:comment, parent: comment, commentable: article, user: user) create(:notification, notifiable: child_comment, user: user) perform_enqueued_jobs do comment.update(deleted: true) - expect(child_comment.notifications.first.json_data["comment"]["ancestors"][0]["title"]).to eq "[deleted]" end + notification = child_comment.notifications.first + expect(notification.json_data["comment"]["ancestors"][0]["title"]).to eq("[deleted]") end end - describe "when algolia auto-indexing/removal is triggered" do + context "when callbacks are triggered after destroy" do + it "updates user's last_comment_at" do + expect { comment.destroy }.to change(user, :last_comment_at) + end + end + + describe "when indexing and deindexing" do + let!(:comment) { create(:comment, commentable: article) } + context "when destroying" do - it "doesn't schedule an ActiveJob on destroy" do - comment = create(:comment, commentable: article) - expect do - comment.destroy - end.not_to have_enqueued_job.on_queue("algoliasearch") + it "doesn't trigger auto removal from index" do + expect { comment.destroy }.not_to have_enqueued_job.on_queue("algoliasearch") end end - context "when record.deleted == false" do + context "when deleted is false" do it "checks auto-indexing" do expect do - create(:comment, user_id: user2.id, commentable_id: article.id) + create(:comment, commentable: article) end.to have_enqueued_job.with(kind_of(described_class), "index!").on_queue("algoliasearch") end end - context "when record.deleted == true" do - it "checks auto-indexing" do - comment.deleted = true + context "when deleted is true" do + it "checks auto-deindexing" do expect do - comment.save! + comment.update(deleted: true) end.to have_enqueued_job.with(kind_of(described_class), "remove_algolia_index").on_queue("algoliasearch") end end end - - describe "when a comment is destroyed" do - it "updates user last comment date" do - expect do - comment.destroy - user2.reload - end.to change(user2, :last_comment_at) - end - end - - describe "when a comment is created" do - it "updates user last comment date" do - expect do - comment.save - user2.reload - end.to change(user2, :last_comment_at) - end - end - - include_examples "#sync_reactions_count", :article_comment end diff --git a/spec/models/credit_spec.rb b/spec/models/credit_spec.rb index 542d81e44..8240cddcc 100644 --- a/spec/models/credit_spec.rb +++ b/spec/models/credit_spec.rb @@ -1,58 +1,63 @@ require "rails_helper" RSpec.describe Credit, type: :model do - let(:user) { create(:user) } - let(:organization) { create(:organization) } - let(:random_number) { rand(100) } + let_it_be(:user) { create(:user) } + let_it_be(:organization) { create(:organization) } it { is_expected.to belong_to(:user).optional } it { is_expected.to belong_to(:organization).optional } it { is_expected.to belong_to(:purchase).optional } - xit "counts credits for user" do - # See https://github.com/magnusvk/counter_culture/issues/259 + context "when caching counters" do + let_it_be(:user_credits) { create_list(:credit, 2, user: user) } + let_it_be(:org_credits) { create_list(:credit, 1, organization: organization) } - create_list(:credit, random_number, user: user) - described_class.counter_culture_fix_counts - expect(user.reload.credits_count).to eq(random_number) - end + describe "#credits_count" do + it "counts credits for user" do + # See https://github.com/magnusvk/counter_culture/issues/259 + described_class.counter_culture_fix_counts + expect(user.reload.credits_count).to eq(user.credits.size) + end - it "counts credits for organization" do - create_list(:credit, random_number, organization: organization) - described_class.counter_culture_fix_counts - expect(organization.reload.credits_count).to eq(random_number) - end + it "counts credits for organization" do + described_class.counter_culture_fix_counts + expect(organization.reload.credits_count).to eq(organization.credits.size) + end + end - it "counts the number of unspent credits for a user" do - create_list(:credit, random_number, user: user) - expect(user.reload.unspent_credits_count).to eq(random_number) - end + describe "#unspent_credits_count" do + it "counts the number of unspent credits for a user" do + expect(user.reload.unspent_credits_count).to eq(user.credits.unspent.size) + end - it "counts the number of spent credits for a user" do - create_list(:credit, random_number, user: user, spent: true) - expect(user.reload.spent_credits_count).to eq(random_number) - end + it "counts the number of unspent credits for an organization" do + expect(organization.reload.unspent_credits_count).to eq(organization.credits.unspent.size) + end + end - it "counts the number of unspent credits for an organization" do - create_list(:credit, random_number, organization: organization) - expect(organization.reload.unspent_credits_count).to eq(random_number) - end + describe "#spent_credits_count" do + it "counts the number of spent credits for a user" do + create_list(:credit, 1, user: user, spent: true) + expect(user.reload.spent_credits_count).to eq(user.credits.spent.size) + end - it "counts the number of spent credits for an organization" do - create_list(:credit, random_number, organization: organization, spent: true) - expect(organization.reload.spent_credits_count).to eq(random_number) + it "counts the number of spent credits for an organization" do + create_list(:credit, 1, organization: organization, spent: true) + expect(organization.reload.spent_credits_count).to eq(organization.credits.spent.size) + end + end end describe "#purchase" do - let(:listing) { create(:classified_listing) } + let_it_be(:credit) { build(:credit) } - it "associates to a purchase" do - credit = create(:credit, purchase: listing) - expect(credit.purchase).to eq(listing) + it "is valid with a purchase" do + credit.purchase = build(:classified_listing) + expect(credit).to be_valid end it "is valid without a purchase" do - credit = create(:credit, purchase: nil) + credit.purchase = nil expect(credit).to be_valid end end diff --git a/spec/models/display_ad_event_spec.rb b/spec/models/display_ad_event_spec.rb index 9cbd807bd..f67747ecd 100644 --- a/spec/models/display_ad_event_spec.rb +++ b/spec/models/display_ad_event_spec.rb @@ -1,22 +1,24 @@ require "rails_helper" RSpec.describe DisplayAdEvent, type: :model do - let(:user) { create(:user) } - let(:organization) { create(:organization) } - let(:display_ad) { create(:display_ad, organization_id: organization.id) } + let_it_be(:user) { build(:user) } + let_it_be(:organization) { build(:organization) } + let_it_be(:display_ad) { build(:display_ad, organization: organization) } - it "creates a click event" do - event = build(:display_ad_event, category: "click", user_id: user.id, display_ad_id: display_ad.id) - expect(event).to be_valid - end + describe "#category" do + it "is valid with a click category" do + event = build(:display_ad_event, category: "click", user: user, display_ad: display_ad) + expect(event).to be_valid + end - it "creates an impression event" do - event = build(:display_ad_event, category: "impression", user_id: user.id, display_ad_id: display_ad.id) - expect(event).to be_valid - end + it "is valid with an impression category" do + event = build(:display_ad_event, category: "impression", user: user, display_ad: display_ad) + expect(event).to be_valid + end - it "does not create an invalid event" do - event = build(:display_ad_event, category: "wazoo", user_id: user.id, display_ad_id: display_ad.id) - expect(event).not_to be_valid + it "is not valid with an uknown category" do + event = build(:display_ad_event, category: "wazoo", user: user, display_ad: display_ad) + expect(event).not_to be_valid + end end end diff --git a/spec/models/display_ad_spec.rb b/spec/models/display_ad_spec.rb index 963563ee6..20307f228 100644 --- a/spec/models/display_ad_spec.rb +++ b/spec/models/display_ad_spec.rb @@ -1,38 +1,52 @@ require "rails_helper" RSpec.describe DisplayAd, type: :model do - let(:display_ad) { create(:display_ad, organization_id: organization.id) } - let(:organization) { create(:organization) } + let_it_be(:organization) { create(:organization) } + let_it_be(:display_ad) { create(:display_ad, organization_id: organization.id) } it { is_expected.to validate_presence_of(:organization_id) } it { is_expected.to validate_presence_of(:placement_area) } it { is_expected.to validate_presence_of(:body_markdown) } - it "generates processed_html before save" do - expect(display_ad.processed_html).to eq("Hello hey Hey hey") + describe "validations" do + it "allows sidebar_right" do + display_ad.placement_area = "sidebar_right" + expect(display_ad).to be_valid + end + + it "allows sidebar_left" do + display_ad.placement_area = "sidebar_left" + expect(display_ad).to be_valid + end + + it "disallows unacceptable placement_area" do + display_ad.placement_area = "tsdsdsdds" + expect(display_ad).not_to be_valid + end end - it "only disallows unacceptable placement_area" do - display_ad.placement_area = "tsdsdsdds" - expect(display_ad).not_to be_valid + context "when callbacks are triggered before save" do + it "generates #processed_html from #body_markdown" do + expect(display_ad.processed_html).to eq("Hello hey Hey hey") + end end - it "allows sidebar_right" do - display_ad.placement_area = "sidebar_right" - expect(display_ad).to be_valid - end + describe ".for_display" do + let!(:display_ad) { create(:display_ad, organization_id: organization.id) } - it "allows sidebar_left" do - display_ad.placement_area = "sidebar_left" - expect(display_ad).to be_valid - end + it "does not return unpublished ads" do + display_ad.update!(published: false, approved: true) + expect(described_class.for_display(display_ad.placement_area)).to be_nil + end - it "displays published and approved posts" do - create(:display_ad, organization_id: organization.id, published: true, approved: true) - create(:display_ad, organization_id: organization.id, published: true, approved: true) - create(:display_ad, organization_id: organization.id, published: false, approved: true) - create(:display_ad, organization_id: organization.id, published: true, approved: false) - expect(described_class.for_display(described_class.last.placement_area).published).to eq(true) - expect(described_class.for_display(described_class.last.placement_area).approved).to eq(true) + it "does not return unapproved ads" do + display_ad.update!(published: true, approved: false) + expect(described_class.for_display(display_ad.placement_area)).to be_nil + end + + it "returns published and approved ads" do + display_ad.update!(published: true, approved: true) + expect(described_class.for_display(display_ad.placement_area)).to eq(display_ad) + end end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 093f9c4f4..cccfa7d20 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" RSpec.describe Event, type: :model do - let(:event) { create(:event) } + let(:event) { build(:event) } it "rejects title with over 90 characters" do event.title = Faker::Lorem.characters(number: 100) @@ -14,16 +14,18 @@ RSpec.describe Event, type: :model do end it "rejects ends times that are earlier than start times" do - event.ends_at = 14.hours.ago + event.ends_at = event.starts_at - 1.minute expect(event).not_to be_valid end it "creates slug for published events" do - event.published = true - expect(event).to be_valid + event = build(:event, category: "ama", title: "yo", published: true) + event.validate! + expected_slug = "#{event.category}-#{event.title}-#{event.starts_at.strftime('%m-%d-%Y')}" + expect(event.slug).to eq(expected_slug) end it "triggers cache busting on save" do - expect { build(:event).save }.to have_enqueued_job.on_queue("events_bust_cache") + expect { event.save }.to have_enqueued_job.on_queue("events_bust_cache") end end diff --git a/spec/models/feedback_message_spec.rb b/spec/models/feedback_message_spec.rb index bbf758403..0e1991cfd 100644 --- a/spec/models/feedback_message_spec.rb +++ b/spec/models/feedback_message_spec.rb @@ -1,9 +1,6 @@ require "rails_helper" RSpec.describe FeedbackMessage, type: :model do - let(:user) { create(:user) } - let(:abuse_report) { create(:feedback_message, :abuse_report) } - describe "validations for an abuse report" do subject(:feedback_message) do described_class.new( diff --git a/spec/models/html_variant_spec.rb b/spec/models/html_variant_spec.rb index d2a056991..e87508143 100644 --- a/spec/models/html_variant_spec.rb +++ b/spec/models/html_variant_spec.rb @@ -8,11 +8,9 @@ RSpec.describe HtmlVariant, type: :model do it { is_expected.to belong_to(:user).optional } it "calculates success rate" do - HtmlVariantTrial.create!(html_variant_id: html_variant.id) - HtmlVariantTrial.create!(html_variant_id: html_variant.id) - HtmlVariantTrial.create!(html_variant_id: html_variant.id) - HtmlVariantTrial.create!(html_variant_id: html_variant.id) + 4.times { HtmlVariantTrial.create!(html_variant_id: html_variant.id) } HtmlVariantSuccess.create!(html_variant_id: html_variant.id) + html_variant.calculate_success_rate! expect(html_variant.success_rate).to eq(0.025) end @@ -35,14 +33,13 @@ RSpec.describe HtmlVariant, type: :model do end it "finds if no tag targeted and tag given" do - html_variant.save! + html_variant.update(target_tag: nil) expect(described_class.find_for_test(["hello"]).id).to eq(html_variant.id) end - it "creates an html variant with img in it" do - html_variant = create(:html_variant, approved: false, published: true) - html_variant.html = "
" - html_variant.save! + it "prefixes an image with cloudinary" do + html = "
" + html_variant.update(approved: false, html: html) expect(html_variant.html).to include("cloudinary") end end diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb index f84b46e6d..9169131c6 100644 --- a/spec/models/identity_spec.rb +++ b/spec/models/identity_spec.rb @@ -8,8 +8,7 @@ RSpec.describe Identity, type: :model do it do # rubocop:disable RSpec/NamedSubject - # see - subject.user = create(:user) + subject.user = build(:user) expect(subject).to validate_uniqueness_of(:user_id).scoped_to(:provider) # rubocop:enable RSpec/NamedSubject end diff --git a/spec/models/job_opportunity_spec.rb b/spec/models/job_opportunity_spec.rb index 6d4f3c4c5..b73df9219 100644 --- a/spec/models/job_opportunity_spec.rb +++ b/spec/models/job_opportunity_spec.rb @@ -1,9 +1,56 @@ require "rails_helper" RSpec.describe JobOpportunity, type: :model do - it "returns remoteness in words for remoteness" do - jo = described_class.new - jo.remoteness = "on_premise" - expect(jo.remoteness_in_words).to eq("In Office") + let(:job_opportunity) { described_class.new(remoteness: "on_premise") } + + context "when validations" do + describe "#remoteness" do + it "is valid with on_premise" do + job_opportunity.remoteness = "on_premise" + expect(job_opportunity).to be_valid + end + + it "is valid with fully_remote" do + job_opportunity.remoteness = "fully_remote" + expect(job_opportunity).to be_valid + end + + it "is valid with remote_optional" do + job_opportunity.remoteness = "remote_optional" + expect(job_opportunity).to be_valid + end + + it "is valid with on_premise_flexible" do + job_opportunity.remoteness = "on_premise_flexible" + expect(job_opportunity).to be_valid + end + + it "is not valid an arbitrary word" do + job_opportunity.remoteness = "foobar" + expect(job_opportunity).not_to be_valid + end + end + end + + describe "#remoteness_in_words" do + it "returns remoteness in words for on_premise" do + job_opportunity.remoteness = "on_premise" + expect(job_opportunity.remoteness_in_words).to eq("In Office") + end + + it "returns remoteness in words for fully_remote" do + job_opportunity.remoteness = "fully_remote" + expect(job_opportunity.remoteness_in_words).to eq("Fully Remote") + end + + it "returns remoteness in words for remote_optional" do + job_opportunity.remoteness = "remote_optional" + expect(job_opportunity.remoteness_in_words).to eq("Remote Optional") + end + + it "returns remoteness in words for on_premise_flexible" do + job_opportunity.remoteness = "on_premise_flexible" + expect(job_opportunity.remoteness_in_words).to eq("Mostly in Office but Flexible") + end end end diff --git a/spec/models/mention_spec.rb b/spec/models/mention_spec.rb index 32dee5a91..de5f35109 100644 --- a/spec/models/mention_spec.rb +++ b/spec/models/mention_spec.rb @@ -1,20 +1,22 @@ require "rails_helper" RSpec.describe Mention, type: :model do - let(:user) { create(:user) } - let(:article) { create(:article, user_id: user.id) } - let(:comment) { create(:comment, user_id: user.id, commentable_id: article.id) } + let(:comment) { create(:comment, commentable: create(:podcast_episode)) } - it "calls on Mentions::CreateAllJob" do - described_class.create_all(comment) do - expect(Mentions::CreateAllJob).to have_received(:perform_later).with(comment.id, "Comment") + describe "#create_all" do + it "enqueues a job to create mentions" do + assert_enqueued_with(job: Mentions::CreateAllJob, args: [comment.id, "Comment"], queue: "mentions_create_all") do + described_class.create_all(comment) + end end end + # TODO: Replace this test with validation spec it "creates a valid mention" do expect(create(:mention)).to be_valid end + # TODO: Replace this test with validation spec it "doesn't raise undefined method for NilClass on valid?" do expect(described_class.new.valid?).to eq(false) end diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 7c624664b..5644d41f3 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -2,84 +2,90 @@ require "rails_helper" RSpec.describe Message, type: :model do let(:user) { create(:user) } - let(:chat_channel) { create(:chat_channel) } let(:user2) { create(:user) } - let(:long_text) { Faker::Hipster.words(number: 1500) } + let(:chat_channel) { create(:chat_channel) } + let(:message) { create(:message, user: user) } describe "validations" do - before do - allow(ChatChannel).to receive(:find).and_return(ChatChannel.new) + context "with automatic validations" do + before do + allow(ChatChannel).to receive(:find).and_return(ChatChannel.new) + end + + it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:chat_channel) } + it { is_expected.to validate_presence_of(:message_html) } + it { is_expected.to validate_presence_of(:message_markdown) } end - it { is_expected.to belong_to(:user) } - it { is_expected.to belong_to(:chat_channel) } - it { is_expected.to validate_presence_of(:message_html) } - it { is_expected.to validate_presence_of(:message_markdown) } + it "is invalid without channel permission for invite only channels" do + chat_channel.update(channel_type: "invite_only") + message = build(:message, chat_channel: chat_channel, user: user) + expect(message).not_to be_valid + end + + it "is valid with channel permission" do + chat_channel.add_users([user]) + message = build(:message, chat_channel: chat_channel, user: user) + expect(message).to be_valid + end + + it "is invalid with text over 1024 chars" do + message = build(:message, chat_channel: chat_channel, user: user, message_markdown: "x" * 1025) + expect(message).not_to be_valid + end end - it "is invalid without channel permission for non-open channels" do - chat_channel.update(channel_type: "invite_only") - message = build(:message, chat_channel_id: chat_channel.id, user_id: user2.id) - expect(message).not_to be_valid + context "when callbacks are triggered before validation" do + let_it_be(:article) { create(:article) } + + describe "#message_html" do + it "creates rich link with proper link" do + message.message_markdown = "hello http://#{ApplicationConfig['APP_DOMAIN']}#{article.path}" + message.validate! + + expect(message.message_html).to include(article.title) + expect(message.message_html).to include("data-content") + end + + it "creates rich link with non-rich link" do + message.message_markdown = "hello http://#{ApplicationConfig['APP_DOMAIN']}/report-abuse" + message.validate! + + expect(message.message_html).not_to include("data-content") + end + end end - it "is valid with channel permission" do - chat_channel.add_users([user2]) - message = build(:message, chat_channel_id: chat_channel.id, user_id: user2.id) - expect(message).to be_valid - end + context "when callbacks are triggered after create" do + before do + chat_channel.add_users([user, user2]) + end - it "is invalid if over 1024 chars" do - message = build(:message, chat_channel_id: chat_channel.id, user_id: user.id, - message_markdown: long_text) - expect(message).not_to be_valid - end + it "sends email if user not recently active on /connect" do + chat_channel.update_column(:channel_type, "direct") + user2.update_column(:updated_at, 1.day.ago) + user2.chat_channel_memberships.last.update_column(:last_opened_at, 2.days.ago) - it "is valid if under 1024 chars" do - message = build(:message, chat_channel_id: chat_channel.id, user_id: user.id, - message_markdown: "hello") - expect(message).to be_valid - end + create(:message, chat_channel: chat_channel, user: user) - it "creates rich link in connect with proper link" do - article = create(:article) - message = create(:message, chat_channel_id: chat_channel.id, user_id: user.id, - message_markdown: "hello http://#{ApplicationConfig['APP_DOMAIN']}#{article.path}") - expect(message.message_html).to include(article.title) - expect(message.message_html).to include("data-content") - end + expect(EmailMessage.last.subject).to start_with("#{user.name} just messaged you") + end - it "creates rich link in connect with non-rich link" do - message = create(:message, chat_channel_id: chat_channel.id, user_id: user.id, - message_markdown: "hello http://#{ApplicationConfig['APP_DOMAIN']}/report-abuse") - expect(message.message_html).not_to include("data-content") - end + it "does not send email if user has been recently active" do + expect do + create(:message, chat_channel: chat_channel, user: user) + end.to change(EmailMessage, :count).by(0) + end - it "sends email if user not recently active on /connect" do - chat_channel.add_users([user, user2]) - chat_channel.update_column(:channel_type, "direct") - user2.update_column(:updated_at, 1.day.ago) - user2.chat_channel_memberships.last.update_column(:last_opened_at, 2.days.ago) - create(:message, chat_channel_id: chat_channel.id, user_id: user.id, - message_markdown: "hello http://#{ApplicationConfig['APP_DOMAIN']}/report-abuse") - expect(EmailMessage.last.subject).to start_with("#{user.name} just messaged you") - end + it "does not send email if user has email_messages turned off" do + chat_channel.update_column(:channel_type, "direct") + user2.update_columns(updated_at: 1.day.ago, email_connect_messages: false) + user2.chat_channel_memberships.last.update_column(:last_opened_at, 2.days.ago) - it "does not send email if user has been recently active" do - chat_channel.add_users([user, user2]) - create(:message, chat_channel_id: chat_channel.id, user_id: user.id, - message_markdown: "hello http://#{ApplicationConfig['APP_DOMAIN']}/report-abuse") - expect(EmailMessage.all.size).to eq(0) - end - - it "does not send email if user has email_messages turned off" do - chat_channel.add_users([user, user2]) - chat_channel.update_column(:channel_type, "direct") - user2.update_column(:updated_at, 1.day.ago) - user2.update_column(:email_connect_messages, false) - user2.chat_channel_memberships.last.update_column(:last_opened_at, 2.days.ago) - create(:message, chat_channel_id: chat_channel.id, user_id: user.id, - message_markdown: "hello http://#{ApplicationConfig['APP_DOMAIN']}/report-abuse") - expect(EmailMessage.all.size).to eq(0) + expect do + create(:message, chat_channel: chat_channel, user: user) + end.to change(EmailMessage, :count).by(0) + end end end diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 20b4d9bcd..b0f426b48 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -1,15 +1,23 @@ require "rails_helper" RSpec.describe Notification, type: :model do - let(:user) { create(:user) } - let(:user2) { create(:user) } - let(:user3) { create(:user) } - let(:organization) { create(:organization) } - let(:article) { create(:article, :with_notification_subscription, user_id: user.id, page_views_count: 4000, positive_reactions_count: 70) } - let(:follow_instance) { user.follow(user2) } - let(:badge_achievement) { create(:badge_achievement) } + let_it_be(:user) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be(:user3) { create(:user) } + let_it_be(:organization) { create(:organization) } + let_it_be(:article) do + create(:article, :with_notification_subscription, user: user, page_views_count: 4000, positive_reactions_count: 70) + end + let_it_be(:user_follows_user2) { user.follow(user2) } + let_it_be(:comment) { create(:comment, user: user2, commentable: article) } + let_it_be(:badge_achievement) { create(:badge_achievement) } - it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(%i[organization_id notifiable_id notifiable_type action]) } + it do + scopes = %i[organization_id notifiable_id notifiable_type action] + # rubocop:disable RSpec/NamedSubject + expect(subject).to validate_uniqueness_of(:user_id).scoped_to(scopes) + # rubocop:enable RSpec/NamedSubject + end describe "when trying to create duplicate notifications" do # Duplicate notifications are not allowed even when validations are skipped @@ -27,8 +35,12 @@ RSpec.describe Notification, type: :model do end it "updates when trying to create a duplicate notification via import" do - notification = create(:notification, user: user, notifiable: article, action: "Reaction", json_data: { "user_id" => 1 }) - duplicate_notification = build(:notification, user: user, notifiable: article, action: "Reaction", json_data: { "user_id" => 2 }) + notification = create( + :notification, user: user, notifiable: article, action: "Reaction", json_data: { "user_id" => 1 } + ) + duplicate_notification = build( + :notification, user: user, notifiable: article, action: "Reaction", json_data: { "user_id" => 2 } + ) described_class.import([duplicate_notification], on_duplicate_key_update: { conflict_target: %i[notifiable_id notifiable_type user_id action], @@ -85,170 +97,198 @@ RSpec.describe Notification, type: :model do end end - describe "when trying to #send_new_follower_notification after following a tag" do - let(:tag) { create(:tag) } - let(:tag_follow) { user.follow(tag) } - - it "runs fine" do - perform_enqueued_jobs do - described_class.send_new_follower_notification(tag_follow) - end - end - - it "doesn't create a notification" do - perform_enqueued_jobs do - expect do - described_class.send_new_follower_notification(tag_follow) - end.not_to change(described_class, :count) - end + context "when callbacks are triggered after create" do + it "sets the notified_at column" do + notification = create(:notification, notifiable: article, user: user2) + expect(notification.notified_at).to be_present end end describe "#send_new_follower_notification" do - before do - perform_enqueued_jobs do - described_class.send_new_follower_notification(follow_instance) + context "when trying to a send notification after following a tag" do + it "does not enqueue a notification job" do + assert_no_enqueued_jobs(only: Notifications::NewFollowerJob) do + tag_follow = user.follow(create(:tag)) + described_class.send_new_follower_notification(tag_follow) + end end end - it "sets the notifiable_at column upon creation" do - expect(described_class.last.notified_at).not_to eq nil - end - - context "when a user follows another user" do + context "when trying to send a notification after following a user" do it "creates a notification belonging to the person being followed" do - expect(described_class.first.user_id).to eq user2.id + perform_enqueued_jobs do + expect do + described_class.send_new_follower_notification(user_follows_user2) + end.to change(user2.notifications, :count).by(1) + end end it "creates a notification from the follow instance" do - notifiable_data = { notifiable_id: described_class.first.notifiable_id, notifiable_type: described_class.first.notifiable_type } - follow_data = { notifiable_id: follow_instance.id, notifiable_type: follow_instance.class.name } - expect(notifiable_data).to eq follow_data + perform_enqueued_jobs do + described_class.send_new_follower_notification(user_follows_user2) + end + + notification = user2.notifications.last + notifiable_data = { notifiable_id: notification.notifiable_id, notifiable_type: notification.notifiable_type } + follow_data = { notifiable_id: user_follows_user2.id, notifiable_type: user_follows_user2.class.name } + expect(notifiable_data).to eq(follow_data) end end context "when a user follows an organization" do - let(:follow_instance) { user.follow(organization) } + let_it_be(:user_follows_organization) { user.follow(organization) } it "creates a notification belonging to the organization" do - expect(described_class.first.organization_id).to eq organization.id + perform_enqueued_jobs do + expect do + described_class.send_new_follower_notification(user_follows_organization) + end.to change(organization.notifications, :count).by(1) + end end it "does not create a notification belonging to a user" do - expect(described_class.first.user_id).to eq nil + perform_enqueued_jobs do + described_class.send_new_follower_notification(user_follows_organization) + expect(organization.notifications.last.user_id).to be(nil) + end end it "creates a notification from the follow instance" do - notifiable_data = { notifiable_id: described_class.first.notifiable_id, notifiable_type: described_class.first.notifiable_type } - follow_data = { notifiable_id: follow_instance.id, notifiable_type: follow_instance.class.name } - expect(notifiable_data).to eq follow_data + perform_enqueued_jobs do + described_class.send_new_follower_notification(user_follows_organization) + end + + notification = organization.notifications.last + notifiable_data = { notifiable_id: notification.notifiable_id, notifiable_type: notification.notifiable_type } + follow_data = { + notifiable_id: user_follows_organization.id, + notifiable_type: user_follows_organization.class.name + } + expect(notifiable_data).to eq(follow_data) end end context "when a user unfollows another user" do it "destroys the follow notification" do - follow_instance = user.stop_following(user2) + # first we follow the user perform_enqueued_jobs do - described_class.send_new_follower_notification(follow_instance) + described_class.send_new_follower_notification(user_follows_user2) + end + + # then we stop following them + perform_enqueued_jobs do + expect do + user_stops_following_user2 = user.stop_following(user2) + described_class.send_new_follower_notification(user_stops_following_user2) + end.to change(user2.notifications, :count).by(-1) end - expect(described_class.count).to eq 0 end end end describe "#send_new_comment_notifications" do + let_it_be(:comment) { create(:comment, user: user2, commentable: article) } + let_it_be(:child_comment) { create(:comment, user: user3, commentable: article, parent: comment) } + context "when all commenters are subscribed" do it "sends a notification to the author of the article" do - comment = create(:comment, user: user2, commentable: article) - described_class.send_new_comment_notifications_without_delay(comment) - expect(user.notifications.count).to eq 1 + expect do + described_class.send_new_comment_notifications_without_delay(comment) + end.to change(user.notifications, :count).by(1) end it "does not send a notification to the author of the article if the commenter is the author" do comment = create(:comment, user: user, commentable: article) - described_class.send_new_comment_notifications_without_delay(comment) - expect(user.notifications.count).to eq 0 + expect do + described_class.send_new_comment_notifications_without_delay(comment) + end.to change(user.notifications, :count).by(0) end it "does not send a notification to the author of the comment" do - comment = create(:comment, user: user2, commentable: article) - described_class.send_new_comment_notifications_without_delay(comment) - expect(user2.notifications.count).to eq 0 + expect do + described_class.send_new_comment_notifications_without_delay(comment) + end.to change(user2.notifications, :count).by(0) end it "sends a notification to the author of the article about the child comment" do - parent_comment = create(:comment, user: user2, commentable: article) - child_comment = create(:comment, user: user3, commentable: article, ancestry: parent_comment.id.to_s) - described_class.send_new_comment_notifications_without_delay(child_comment) - expect(user.notifications.count).to eq(1) + expect do + described_class.send_new_comment_notifications_without_delay(child_comment) + end.to change(user.notifications, :count).by(1) end it "sends a notification to the organization" do - org = create(:organization) - create(:organization_membership, user: user, organization: org) - article.update(organization: org) + create(:organization_membership, user: user, organization: organization) + article.update(organization: organization) comment = create(:comment, user: user2, commentable: article) - described_class.send_new_comment_notifications_without_delay(comment) - expect(org.notifications.count).to eq 1 + + expect do + described_class.send_new_comment_notifications_without_delay(comment) + end.to change(organization.notifications, :count).by(1) end end context "when the author of the article is not subscribed" do - let!(:comment) { create(:comment, user: user2, commentable: article) } - before do article.update(receive_notifications: false) article.notification_subscriptions.delete_all end it "does not send a notification to the author of the article" do - described_class.send_new_comment_notifications_without_delay(comment) - expect(user.notifications.count).to eq 0 + expect do + described_class.send_new_comment_notifications_without_delay(comment) + end.to change(user.notifications, :count).by(0) end it "doesn't send a notification to the author of the article about the child comment" do - child_comment = create(:comment, user: user3, commentable: article, ancestry: comment.id.to_s) - described_class.send_new_comment_notifications_without_delay(child_comment) - expect(user.notifications.count).to eq 0 + expect do + described_class.send_new_comment_notifications_without_delay(child_comment) + end.to change(user.notifications, :count).by(0) end end context "when the author of a comment is not subscribed" do - let(:parent_comment) { create(:comment, user: user2, commentable: article) } - let!(:child_comment) { create(:comment, user: user3, commentable: article, ancestry: parent_comment.id.to_s) } - before do - parent_comment.update(receive_notifications: false) + comment.update(receive_notifications: false) end it "does not send a notification to the author of the comment" do - described_class.send_new_comment_notifications_without_delay(child_comment) - expect(user2.notifications.count).to eq 0 + expect do + described_class.send_new_comment_notifications_without_delay(child_comment) + end.to change(child_comment.user.notifications, :count).by(0) end it "sends a notification to the author of the article" do - described_class.send_new_comment_notifications_without_delay(child_comment) - expect(user.notifications.count).to eq(1) + expect do + described_class.send_new_comment_notifications_without_delay(child_comment) + end.to change(user.notifications, :count).by(1) end end end describe "#send_reaction_notification" do + before do + article.update_columns(receive_notifications: true) + comment.update_columns(receive_notifications: true) + end + context "when reactable is receiving notifications" do it "sends a notification to the author of a comment" do - comment = create(:comment, user: user2, commentable: article) reaction = create(:reaction, reactable: comment, user: user) + perform_enqueued_jobs do - described_class.send_reaction_notification(reaction, reaction.reactable.user) - expect(user2.notifications.count).to eq 1 + expect do + described_class.send_reaction_notification(reaction, reaction.reactable.user) + end.to change(comment.user.notifications, :count).by(1) end end it "sends a notification to the author of an article" do reaction = create(:reaction, reactable: article, user: user2) + perform_enqueued_jobs do - described_class.send_reaction_notification(reaction, reaction.reactable.user) - expect(user.notifications.count).to eq 1 + expect do + described_class.send_reaction_notification(reaction, reaction.reactable.user) + end.to change(article.user.notifications, :count).by(1) end end end @@ -256,17 +296,18 @@ RSpec.describe Notification, type: :model do context "when a reaction is destroyed" do let(:comment) { create(:comment, user: user2, commentable: article) } let!(:notification) { create(:notification, user: user, notifiable: comment, action: "Reaction") } + let(:sibling_reaction) { create(:reaction, reactable: comment, user: user3) } it "destroys the notification if it exists" do reaction = create(:reaction, reactable: comment, user: user) reaction.destroy described_class.send_reaction_notification_without_delay(reaction, article.user) - expect(described_class.where(id: notification.id)).not_to be_any + expect(described_class.exists?(id: notification.id)).to be(false) end it "keeps the notification if siblings exist" do reaction = create(:reaction, reactable: comment, user: user) - create(:reaction, reactable: comment, user: user3) + sibling_reaction # to create it reaction.destroy described_class.send_reaction_notification_without_delay(reaction, article.user) notification.reload @@ -275,7 +316,7 @@ RSpec.describe Notification, type: :model do it "doesn't keep data of the destroyed reaction in the notification" do reaction = create(:reaction, reactable: comment, user: user) - create(:reaction, reactable: comment, user: user3) + sibling_reaction # to create it reaction.destroy described_class.send_reaction_notification_without_delay(reaction, article.user) notification.reload @@ -283,6 +324,22 @@ RSpec.describe Notification, type: :model do # not the user of the destroyed reaction! expect(notification.json_data["user"]["id"]).to eq(user3.id) end + + it "creates and destroys the notification properly" do + reaction = create(:reaction, user: user2, reactable: article, category: "like") + + perform_enqueued_jobs do + expect do + described_class.send_reaction_notification(reaction, reaction.reactable.user) + end.to change(user.notifications, :count).by(1) + + reaction.destroy! + + expect do + described_class.send_reaction_notification(reaction, reaction.reactable.user) + end.to change(user.notifications, :count).by(-1) + end + end end context "when reactable is not receiving notifications" do @@ -290,96 +347,94 @@ RSpec.describe Notification, type: :model do comment = create(:comment, user: user2, commentable: article) comment.update(receive_notifications: false) reaction = create(:reaction, reactable: comment, user: user) - described_class.send_reaction_notification_without_delay(reaction, reaction.reactable.user) - expect(user2.notifications.count).to eq 0 + + expect do + described_class.send_reaction_notification_without_delay(reaction, reaction.reactable.user) + end.to change(user.notifications, :count).by(0) end it "does not send a notification to the author of an article" do article.update(receive_notifications: false) reaction = create(:reaction, reactable: article, user: user2) - described_class.send_reaction_notification_without_delay(reaction, reaction.reactable.user) - expect(user.notifications.count).to eq 0 + + expect do + described_class.send_reaction_notification_without_delay(reaction, reaction.reactable.user) + end.to change(user.notifications, :count).by(0) end end context "when the reactable is an organization's article" do - let(:org) { create(:organization) } - before do - create(:organization_membership, user: user, organization: org, type_of_user: "admin") - article.update(organization: org) + create(:organization_membership, user: user, organization: organization, type_of_user: "admin") + article.update(organization: organization) end it "creates a notification with the organization's ID" do reaction = create(:reaction, reactable: article, user: user2) - described_class.send_reaction_notification_without_delay(reaction, reaction.reactable.organization) - expect(org.notifications.count).to eq 1 - end - end - - it "creates positive reaction notification" do - reaction = article.reactions.create!( - user_id: user2.id, - category: "like", - ) - perform_enqueued_jobs do expect do - described_class.send_reaction_notification(reaction, reaction.reactable.user) - end.to change(described_class, :count).by(1) + described_class.send_reaction_notification_without_delay(reaction, reaction.reactable.organization) + end.to change(organization.notifications, :count).by(1) end end - it "does not create negative notification" do - user2.add_role(:trusted) - reaction = article.reactions.create!( - user_id: user2.id, - category: "vomit", - ) - perform_enqueued_jobs do - expect do - described_class.send_reaction_notification(reaction, reaction.reactable.user) - end.not_to change(described_class, :count) - end - end + context "when dealing with positive and negative reactions" do + it "creates a notification for a positive reaction" do + reaction = create(:reaction, reactable: article, user: user2, category: "like") - it "destroys the notification properly" do - reaction = create(:reaction, user: user2, reactable: article, category: "like") - perform_enqueued_jobs do - described_class.send_reaction_notification(reaction, reaction.reactable.user) - reaction.destroy! - described_class.send_reaction_notification(reaction, reaction.reactable.user) - expect(described_class.count).to eq 0 + perform_enqueued_jobs do + expect do + described_class.send_reaction_notification(reaction, reaction.reactable.user) + end.to change(article.notifications, :count).by(1) + end + end + + it "does not create a notification for a negative reaction" do + user2.add_role(:trusted) + reaction = create(:reaction, reactable: article, user: user2, category: "vomit") + + perform_enqueued_jobs do + expect do + described_class.send_reaction_notification(reaction, reaction.reactable.user) + end.to change(article.notifications, :count).by(0) + end end end end describe "#send_to_followers" do context "when the notifiable is an article from a user" do - before do - user2.follow(user) - perform_enqueued_jobs { described_class.send_to_followers(article, "Published") } - end - it "sends a notification to the author's followers" do - expect(described_class.first.user_id).to eq user2.id + user2.follow(user) + + perform_enqueued_jobs do + expect do + described_class.send_to_followers(article, "Published") + end.to change(user2.notifications, :count).by(1) + end end end context "when the notifiable is an article from an organization" do - let(:article) { create(:article, organization_id: organization.id, user_id: user.id) } - - before do - user2.follow(user) - user3.follow(organization) - perform_enqueued_jobs { described_class.send_to_followers(article, "Published") } - end + let_it_be(:org_article) { create(:article, organization: organization, user: user) } it "sends a notification to author's followers" do - expect(user2.notifications.count).to eq 1 + user2.follow(user) + + perform_enqueued_jobs do + expect do + described_class.send_to_followers(org_article, "Published") + end.to change(user2.notifications, :count).by(1) + end end it "sends a notification to the organization's followers" do - expect(user3.notifications.count).to eq 1 + user3.follow(organization) + + perform_enqueued_jobs do + expect do + described_class.send_to_followers(org_article, "Published") + end.to change(user3.notifications, :count).by(1) + end end end end @@ -395,39 +450,42 @@ RSpec.describe Notification, type: :model do new_title = "hehehe hohoho!" article.update_column(:title, new_title) article.reload + perform_enqueued_jobs do described_class.update_notifications(article, "Published") - first_notification_article_title = described_class.first.json_data["article"]["title"] - expect(first_notification_article_title).to eq new_title + expected_notification_article_title = user2.notifications.last.json_data["article"]["title"] + expect(expected_notification_article_title).to eq(new_title) end end it "adds organization data when the article now belongs to an org" do article.update_column(:organization_id, organization.id) article.reload + perform_enqueued_jobs do - described_class.update_notifications(article.reload, "Published") - first_notification_organization_id = described_class.first.json_data["organization"]["id"] - expect(first_notification_organization_id).to eq organization.id + described_class.update_notifications(article, "Published") + expected_notification_organization_id = described_class.last.json_data["organization"]["id"] + expect(expected_notification_organization_id).to eq(organization.id) end end end end describe "#aggregated?" do + let_it_be(:notification) { build(:notification) } it "returns true if a notification's action is 'Reaction'" do - notification = build(:notification, action: "Reaction") - expect(notification.aggregated?).to eq true + notification.action = "Reaction" + expect(notification.aggregated?).to be(true) end it "returns true if a notification's action is 'Follow'" do - notification = build(:notification, action: "Follow") - expect(notification.aggregated?).to eq true + notification.action = "Follow" + expect(notification.aggregated?).to be(true) end it "returns false if a notification's action is not 'Reaction' or 'Follow'" do - notification = build(:notification, action: "Published") - expect(notification.aggregated?).to eq false + notification.action = "Published" + expect(notification.aggregated?).to be(false) end end @@ -448,19 +506,14 @@ RSpec.describe Notification, type: :model do end describe "#remove_all" do - let(:mention) { create(:mention, user_id: user.id, mentionable_id: comment.id, mentionable_type: "Comment") } - let(:comment) { create(:comment, user_id: user.id, commentable_id: article.id) } - let(:notifiable_collection) { [mention] } - - before do - create(:notification, user_id: mention.user_id, notifiable_id: mention.id, notifiable_type: "Mention") - end - it "removes all mention related notifications" do + mention = create(:mention, user: user, mentionable: comment) + create(:notification, user: mention.user, notifiable: mention) + perform_enqueued_jobs do expect do described_class.remove_all(notifiable_ids: mention.id, notifiable_type: "Mention") - end.to change(described_class, :count).by(-1) + end.to change(user.notifications, :count).by(-1) end end end diff --git a/spec/models/notification_subscription_spec.rb b/spec/models/notification_subscription_spec.rb index 3a695a0e8..9efd31df9 100644 --- a/spec/models/notification_subscription_spec.rb +++ b/spec/models/notification_subscription_spec.rb @@ -1,10 +1,10 @@ require "rails_helper" RSpec.describe NotificationSubscription, type: :model do - subject { create(:notification_subscription, user: user, notifiable: article) } + let_it_be(:user) { create(:user) } + let_it_be(:article) { create(:article, user: user) } - let(:user) { create(:user) } - let(:article) { create(:article, user: user) } + subject { create(:notification_subscription, user: user, notifiable: article) } it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(%i[notifiable_type notifiable_id]) } end diff --git a/spec/models/organization_spec.rb b/spec/models/organization_spec.rb index 934bd0b89..1dcb63a40 100644 --- a/spec/models/organization_spec.rb +++ b/spec/models/organization_spec.rb @@ -1,7 +1,6 @@ require "rails_helper" RSpec.describe Organization, type: :model do - let(:user) { create(:user) } let(:organization) { create(:organization) } it { is_expected.to have_many(:sponsorships) } @@ -9,22 +8,24 @@ RSpec.describe Organization, type: :model do describe "#name" do it "rejects names with over 50 characters" do - organization.name = Faker::Lorem.characters(number: 51) + organization.name = "x" * 51 expect(organization).not_to be_valid end it "accepts names with 50 or less characters" do + organization.name = "x" * 50 expect(organization).to be_valid end end describe "#summary" do - it "rejects summaries with over 1000 characters" do - organization.summary = Faker::Lorem.characters(number: 1001) + it "rejects summaries with over 250 characters" do + organization.summary = "x" * 251 expect(organization).not_to be_valid end - it "accepts summaries with 1000 or less characters" do + it "accepts summaries with 250 or less characters" do + organization.summary = "x" * 250 expect(organization).to be_valid end end @@ -46,7 +47,7 @@ RSpec.describe Organization, type: :model do end it "rejects wrong color format" do - organization.text_color_hex = "##{Faker::Lorem.words(number: 4)}" + organization.text_color_hex = "#FOOBAR" expect(organization).not_to be_valid end end @@ -128,44 +129,46 @@ RSpec.describe Organization, type: :model do end describe "#check_for_slug_change" do - def create_article_for_organization - user.update(organization_id: organization.id, org_admin: true) - create(:article, organization_id: organization.id, user_id: user.id) - end + let(:user) { create(:user) } it "properly updates the slug/username" do random_new_slug = "slug_#{rand(10_000)}" organization.update(slug: random_new_slug) - expect(organization.slug).to eq random_new_slug + expect(organization.slug).to eq(random_new_slug) end it "updates old_slug to original slug if slug was changed" do original_slug = organization.slug organization.update(slug: "slug_#{rand(10_000)}") - expect(organization.old_slug).to eq original_slug + expect(organization.old_slug).to eq(original_slug) end it "updates old_old_slug properly if slug was changed and there was an old_slug" do original_slug = organization.slug organization.update(slug: "something_else") organization.update(slug: "another_slug") - expect(organization.old_old_slug).to eq original_slug + expect(organization.old_old_slug).to eq(original_slug) end - it "updates the paths of the organization's articles" do - create_article_for_organization - new_slug = "slug_#{rand(10_000)}" - organization.update(slug: new_slug) - article = Article.find_by(organization_id: organization.id) - expect(article.path).to include new_slug - end + context "when dealing with organization articles" do + before do + create(:organization_membership, user: user, organization: organization, type_of_user: "admin") + create(:article, organization: organization, user: user) + end - it "updates article cached_organizations" do - create_article_for_organization - new_slug = "slug_#{rand(10_000)}" - organization.update(slug: new_slug) - article = Article.find_by(organization_id: organization.id) - expect(article.cached_organization.slug).to eq new_slug + it "updates the paths of the organization's articles" do + new_slug = "slug_#{rand(10_000)}" + organization.update(slug: new_slug) + article = Article.find_by(organization_id: organization.id) + expect(article.path).to include(new_slug) + end + + it "updates article cached_organizations" do + new_slug = "slug_#{rand(10_000)}" + organization.update(slug: new_slug) + article = Article.find_by(organization_id: organization.id) + expect(article.cached_organization.slug).to eq(new_slug) + end end end @@ -187,7 +190,7 @@ RSpec.describe Organization, type: :model do describe "#pro?" do it "always returns false" do - expect(organization.pro?).to be(false) + expect(build(:organization).pro?).to be(false) end end end diff --git a/spec/models/page_spec.rb b/spec/models/page_spec.rb index 329d3464b..8770e411a 100644 --- a/spec/models/page_spec.rb +++ b/spec/models/page_spec.rb @@ -1,34 +1,14 @@ require "rails_helper" RSpec.describe Page, type: :model do - let(:page) { create(:page) } - - describe "#processed_html" do - it "accepts body markdown and turns it into html" do - page.body_markdown = "Hello `heyhey`" - page.save - expect(page.processed_html).to include("") - end - - it "accepts body html" do - page.body_html = "Hello `heyhey`" - page.body_markdown = nil - page.save - expect(page.processed_html).to eq(page.body_html) - end - + describe "#validations" do it "requires either body_markdown or body_html" do + page = build(:page) page.body_html = nil page.body_markdown = nil expect(page).not_to be_valid end - it "triggers cache busting on save" do - expect { build(:page).save }.to have_enqueued_job.on_queue("pages_bust_cache") - end - end - - describe "#validations" do it "takes organization slug into account" do create(:organization, slug: "benandfriends") page = build(:page, slug: "benandfriends") @@ -50,4 +30,27 @@ RSpec.describe Page, type: :model do expect(page.errors[:slug].to_s.include?("taken")).to be true end end + + context "when callbacks are triggered before save" do + let(:page) { create(:page) } + + describe "#processed_html" do + it "accepts body markdown and turns it into html" do + page.update(body_markdown: "Hello `heyhey`") + expect(page.processed_html).to include("") + end + + it "accepts body html without changing it" do + html = "Hello `heyhey`" + page.update(body_html: html, body_markdown: "") + expect(page.processed_html).to eq(html) + end + end + end + + context "when callbacks are triggered after save" do + it "triggers cache busting on save" do + expect { build(:page).save }.to have_enqueued_job.on_queue("pages_bust_cache") + end + end end diff --git a/spec/models/page_view_spec.rb b/spec/models/page_view_spec.rb index 87a5e6507..28cebadc9 100644 --- a/spec/models/page_view_spec.rb +++ b/spec/models/page_view_spec.rb @@ -1,22 +1,22 @@ require "rails_helper" RSpec.describe PageView, type: :model do - let(:article) { create(:article) } + let(:page_view) { create(:page_view, referrer: "http://example.com/page") } it { is_expected.to belong_to(:user).optional } it { is_expected.to belong_to(:article) } - describe "#domain" do - it "is automatically set when a new page view is created" do - pv = create(:page_view, referrer: "http://example.com/page") - expect(pv.reload.domain).to eq("example.com") + context "when callbacks are triggered before create" do + describe "#domain" do + it "is automatically set when a new page view is created" do + expect(page_view.domain).to eq("example.com") + end end - end - describe "#path" do - it "is automatically set when a new page view is created" do - pv = create(:page_view, referrer: "http://example.com/page") - expect(pv.reload.path).to eq("/page") + describe "#path" do + it "is automatically set when a new page view is created" do + expect(page_view.path).to eq("/page") + end end end end diff --git a/spec/models/podcast_episode_spec.rb b/spec/models/podcast_episode_spec.rb index 0ec78c52d..1af8f024a 100644 --- a/spec/models/podcast_episode_spec.rb +++ b/spec/models/podcast_episode_spec.rb @@ -27,55 +27,71 @@ RSpec.describe PodcastEpisode, type: :model do expect(ep2).not_to be_valid expect(ep2.errors[:media_url]).to be_present end - - it "accepts valid podcast episode" do - expect(podcast_episode).to be_valid - end - end - - describe "#available" do - let(:podcast) { create(:podcast) } - let(:unpodcast) { create(:podcast, published: false) } - let!(:episode) { create(:podcast_episode, podcast: podcast) } - - before do - create(:podcast_episode, podcast: unpodcast) - create(:podcast_episode, podcast: podcast, reachable: false) - end - - it "is available when reachable and published" do - available_ids = described_class.available.pluck(:id) - expect(available_ids).to eq([episode.id]) - end end describe "#description" do it "strips tags from the body" do - podcast_episode.body = "

Body with HTML tags

" - expect(podcast_episode.description).to eq("Body with HTML tags") + ep2 = build(:podcast_episode, guid: podcast_episode.guid) + + ep2.body = "

Body with HTML tags

" + expect(ep2.description).to eq("Body with HTML tags") end end - describe "image cleanup during validation" do - it "removes empty paragraphs" do - podcast_episode.body = "

\r\n

 

\r\n

" - podcast_episode.validate! - expect(podcast_episode.processed_html).to eq("

") + describe "#index_id" do + it "is equal to articles-ID" do + # NOTE: we shouldn't test private things but cheating a bit for Algolia here + expect(podcast_episode.send(:index_id)).to eq("podcast_episodes-#{podcast_episode.id}") + end + end + + describe ".available" do + let_it_be(:podcast) { create(:podcast) } + + it "is available when reachable and published" do + expect do + create(:podcast_episode, podcast: podcast) + end.to change(described_class.available, :count).by(1) end - it "adds a wrapping paragraph" do - podcast_episode.body = "the body" - podcast_episode.validate! - expect(podcast_episode.processed_html).to eq("

the body

") + it "is not available when unreachable" do + expect do + create(:podcast_episode, podcast: podcast, reachable: false) + end.to change(described_class.available, :count).by(0) end - it "does not add a wrapping paragraph if already present" do - podcast_episode.body = "

the body

" - podcast_episode.validate! - expect(podcast_episode.processed_html).to eq("

the body

") + it "is not available when podcast is unpublished" do + expect do + podcast = create(:podcast, published: false) + create(:podcast_episode, podcast: podcast) + end.to change(described_class.available, :count).by(0) + end + end + + context "when callbacks are triggered before validation" do + let_it_be(:podcast_episode) { build(:podcast_episode) } + + describe "paragraphs cleanup" do + it "removes empty paragraphs" do + podcast_episode.body = "

\r\n

 

\r\n

" + podcast_episode.validate! + expect(podcast_episode.processed_html).to eq("

") + end + + it "adds a wrapping paragraph" do + podcast_episode.body = "the body" + podcast_episode.validate! + expect(podcast_episode.processed_html).to eq("

the body

") + end + + it "does not add a wrapping paragraph if already present" do + podcast_episode.body = "

the body

" + podcast_episode.validate! + expect(podcast_episode.processed_html).to eq("

the body

") + end end - describe "Cloudinary configuration" do + describe "Cloudinary configuration and processing" do it "prefixes an image URL with a path" do image_url = "https://dummyimage.com/10x10" podcast_episode.body = "" @@ -99,7 +115,9 @@ RSpec.describe PodcastEpisode, type: :model do end end - it "triggers cache busting on save" do - expect { build(:podcast_episode).save }.to have_enqueued_job.on_queue("podcast_episodes_bust_cache") + context "when callbacks are triggered after save" do + it "triggers cache busting on save" do + expect { build(:podcast_episode).save }.to have_enqueued_job.on_queue("podcast_episodes_bust_cache") + end end end diff --git a/spec/models/podcast_spec.rb b/spec/models/podcast_spec.rb index c387996ae..b05961fa1 100644 --- a/spec/models/podcast_spec.rb +++ b/spec/models/podcast_spec.rb @@ -1,23 +1,21 @@ require "rails_helper" RSpec.describe Podcast, type: :model do + let(:podcast) { create(:podcast) } + it { is_expected.to validate_presence_of(:image) } it { is_expected.to validate_presence_of(:slug) } it { is_expected.to validate_presence_of(:title) } it { is_expected.to validate_presence_of(:main_color_hex) } it { is_expected.to validate_presence_of(:feed_url) } - describe "validations" do - let(:podcast) { create(:podcast) } - - it "is valid" do - expect(podcast).to be_valid - end - + context "when callbacks are triggered after save" do it "triggers cache busting on save" do - expect { podcast.save }.to have_enqueued_job.on_queue("podcasts_bust_cache").twice + expect { build(:podcast).save }.to have_enqueued_job.on_queue("podcasts_bust_cache").once end + end + describe "validations" do # Couldn't use shoulda uniqueness matchers for these tests because: # Shoulda uses `save(validate: false)` which skips validations # So an invalid record is trying to be saved but fails because of the db constraints @@ -65,36 +63,52 @@ RSpec.describe Podcast, type: :model do end end - describe "#reachable and #available" do - let(:podcast) { create(:podcast, reachable: false, published: true) } - let!(:unpodcast) { create(:podcast, reachable: false, published: true) } - let!(:unpodcast2) { create(:podcast, reachable: false, published: true) } - let!(:cool_podcast) { create(:podcast, reachable: true, published: false) } - let!(:reachable_podcast) { create(:podcast, reachable: true, published: true) } - - before do - create(:podcast_episode, reachable: true, podcast: podcast) - create(:podcast_episode, reachable: false, podcast: unpodcast2) - create(:podcast_episode, reachable: true, podcast: cool_podcast) - create(:podcast_episode, reachable: true, podcast: reachable_podcast) + describe ".reachable" do + it "is reachable when it has a reachable episode" do + expect do + create(:podcast_episode, reachable: true) + create(:podcast, published: true) + end.to change(described_class.reachable, :count).by(1) end - it "is reachable when the feed is unreachable but the podcast has reachable podcasts" do - reachable_ids = described_class.reachable.pluck(:id) - expect(reachable_ids).to include(podcast.id) - expect(reachable_ids).to include(cool_podcast.id) - expect(reachable_ids).not_to include(unpodcast.id) - expect(reachable_ids).not_to include(unpodcast2.id) + it "is reachable when it has a reachable episode even if it is unpublished" do + expect do + create(:podcast_episode, reachable: true) + create(:podcast, published: false) + end.to change(described_class.reachable, :count).by(1) end - it "is available only when reachable and published" do - available_ids = described_class.available.pluck(:id) - expect(available_ids.sort).to eq([podcast.id, reachable_podcast.id].sort) + it "is not reachable when it has an unreachable episode" do + expect do + create(:podcast_episode, reachable: false) + create(:podcast, published: true) + end.to change(described_class.reachable, :count).by(0) + end + end + + describe ".available" do + it "is available when it has a reachable episode and it is published" do + expect do + create(:podcast_episode, reachable: true) + create(:podcast, published: true) + end.to change(described_class.available, :count).by(1) + end + + it "is not available when it has an unreachable episode" do + expect do + create(:podcast_episode, reachable: false) + create(:podcast, published: true) + end.to change(described_class.available, :count).by(0) + end + + it "is not available when it is not published" do + expect do + create(:podcast, published: false) + end.to change(described_class.available, :count).by(0) end end describe "#existing_episode" do - let(:podcast) { create(:podcast) } let(:guid) { "http://podcast.example/file.mp3" } let(:item) do @@ -130,35 +144,24 @@ RSpec.describe Podcast, type: :model do end it "doesn't determine existing episode by non-unique website_url" do - podcast.update_columns(unique_website_url?: false) + podcast.update_attribute(:unique_website_url?, false) create(:podcast_episode, podcast: podcast, website_url: "https://litealloy.ru") expect(podcast.existing_episode(item)).to eq(nil) end end describe "#admins" do - let(:podcast) { create(:podcast) } - let(:podcast2) { create(:podcast) } - let(:podcast3) { create(:podcast) } let(:user) { create(:user) } - let(:user2) { create(:user) } - let(:user3) { create(:user) } - before do + it "returns podcast admins" do user.add_role(:podcast_admin, podcast) - user2.add_role(:podcast_admin, podcast) - user.add_role(:podcast_admin, podcast3) - user3.add_role(:podcast_admin, podcast2) - user3.add_role(:podcast_admin, podcast2) - [user, user2, user3].each(&:save) + expect(podcast.admins).to include(user) end - it "returns proper admins" do - expect(podcast.admins.sort).to eq([user, user2].sort) - end - - it "returns proper admins for podcast3" do - expect(podcast3.admins).to eq([user]) + it "does not return admins for other podcasts" do + other_podcast = create(:podcast) + user.add_role(:podcast_admin, other_podcast) + expect(podcast.admins).to be_empty end end end diff --git a/spec/models/poll_option_spec.rb b/spec/models/poll_option_spec.rb index 3dfb738ee..20ed5438f 100644 --- a/spec/models/poll_option_spec.rb +++ b/spec/models/poll_option_spec.rb @@ -1,16 +1,19 @@ require "rails_helper" RSpec.describe PollOption, type: :model do - let(:article) { create(:article, featured: true) } - let(:poll) { create(:poll, article_id: article.id) } + let_it_be(:article) { build(:article, featured: true) } + let_it_be(:poll) { build(:poll, article: article) } + let_it_be(:poll_option) { build(:poll_option, poll: poll) } - it "allows up to 128 markdown characters" do - poll_option = described_class.create(markdown: "0" * 30, poll_id: poll.id) - expect(poll_option).to be_valid - end + describe "validations" do + it "allows up to 128 markdown characters" do + poll_option.markdown = "0" * 128 + expect(poll_option).to be_valid + end - it "disallows over 128 markdown characters" do - poll_option = described_class.create(markdown: "0" * 200, poll_id: poll.id) - expect(poll_option).not_to be_valid + it "disallows over 128 markdown characters" do + poll_option.markdown = "0" * 129 + expect(poll_option).not_to be_valid + end end end diff --git a/spec/models/poll_skip_spec.rb b/spec/models/poll_skip_spec.rb index 533a07c5c..8f55ac095 100644 --- a/spec/models/poll_skip_spec.rb +++ b/spec/models/poll_skip_spec.rb @@ -1,35 +1,29 @@ require "rails_helper" RSpec.describe PollSkip, type: :model do - let(:article) { create(:article, featured: true) } - let(:user) { create(:user) } - let(:poll) { create(:poll, article_id: article.id) } + let_it_be(:article) { create(:article, featured: true) } + let_it_be(:user) { create(:user) } + let_it_be(:poll) { create(:poll, article: article) } - it "is unique across poll and user" do - described_class.create(user_id: user.id, poll_id: poll.id) - described_class.create(user_id: user.id, poll_id: poll.id) - described_class.create(user_id: user.id, poll_id: poll.id) - expect(described_class.all.size).to eq(1) - second_poll = create(:poll, article_id: article.id) - described_class.create(user_id: user.id, poll_id: second_poll.id) - expect(described_class.all.size).to eq(2) - end + describe "validations" do + context "when checking against poll" do + before do + create(:poll_skip, user: user, poll: poll) + end - it "is unique across user and poll votes for the poll" do - PollVote.create(user_id: user.id, poll_id: poll.id, poll_option_id: poll.poll_options.last.id) - described_class.create(user_id: user.id, poll_id: poll.id) - described_class.create(user_id: user.id, poll_id: poll.id) - expect(described_class.all.size).to eq(0) - second_poll = create(:poll, article_id: article.id) - described_class.create(user_id: user.id, poll_id: second_poll.id) - expect(described_class.all.size).to eq(1) - end + it "is unique across poll and user" do + expect(build(:poll_skip, user: user, poll: poll)).not_to be_valid + end - it "is prevents a poll vote from being cast" do - described_class.create(user_id: user.id, poll_id: poll.id) - described_class.create(user_id: user.id, poll_id: poll.id) - PollVote.create(user_id: user.id, poll_id: poll.id, poll_option_id: poll.poll_options.last.id) - expect(described_class.all.size).to eq(1) - expect(PollVote.all.size).to eq(0) + it "is valid if it belongs to the same user but to a different poll" do + second_poll = create(:poll, article: article) + expect(build(:poll_skip, user: user, poll: second_poll)).to be_valid + end + end + + it "is unique across user and poll votes for the poll" do + create(:poll_vote, user: user, poll: poll, poll_option: poll.poll_options.last) + expect(build(:poll_skip, user: user, poll: poll)).not_to be_valid + end end end diff --git a/spec/models/poll_spec.rb b/spec/models/poll_spec.rb index 656d04d8c..e951a3aa0 100644 --- a/spec/models/poll_spec.rb +++ b/spec/models/poll_spec.rb @@ -1,17 +1,30 @@ require "rails_helper" RSpec.describe Poll, type: :model do - let(:article) { create(:article, featured: true) } - let(:poll) { create(:poll, article_id: article.id) } + let_it_be(:article) { create(:article, featured: true) } - it "limits length of prompt" do - long_string = "0" * 200 - poll.prompt_markdown = long_string - expect(poll).not_to be_valid + describe "validations" do + let_it_be(:poll) { build(:poll, article: article) } + + describe "#prompt_markdown" do + it "is valid up to 128 chars" do + poll.prompt_markdown = "x" * 128 + expect(poll).to be_valid + end + + it "is not valid with more than 128 chars" do + poll.prompt_markdown = "x" * 129 + expect(poll).not_to be_valid + end + end end - it "creates options from input" do - poll = create(:poll, article_id: article.id, poll_options_input_array: %w[hello goodbye heyheyhey]) - expect(poll.poll_options.size).to eq(3) + context "when callbacks are triggered after create" do + it "creates options from input" do + options = %w[hello goodbye heyheyhey] + expect do + create(:poll, article: article, poll_options_input_array: options) + end.to change(PollOption, :count).by(options.size) + end end end diff --git a/spec/models/profile_pin_spec.rb b/spec/models/profile_pin_spec.rb index 95a1f3663..c3504c87d 100644 --- a/spec/models/profile_pin_spec.rb +++ b/spec/models/profile_pin_spec.rb @@ -1,45 +1,44 @@ require "rails_helper" RSpec.describe ProfilePin, type: :model do - let(:user) { create(:user) } - let(:second_user) { create(:user) } - let(:article) { create(:article, user_id: user.id) } - let(:second_article) { create(:article, user_id: user.id) } - let(:third_article) { create(:article, user_id: user.id) } - let(:fourth_article) { create(:article, user_id: user.id) } - let(:fifth_article) { create(:article, user_id: user.id) } - let(:sixth_article) { create(:article, user_id: user.id) } + let_it_be(:user) { create(:user) } describe "validations" do - it "allows up to five pins per user" do - create(:profile_pin, pinnable_id: article.id, profile_id: user.id) - create(:profile_pin, pinnable_id: second_article.id, profile_id: user.id) - create(:profile_pin, pinnable_id: third_article.id, profile_id: user.id) - create(:profile_pin, pinnable_id: fourth_article.id, profile_id: user.id) - last_pin = create(:profile_pin, pinnable_id: fifth_article.id, profile_id: user.id) - expect(last_pin).to be_valid + describe "number of pins" do + let_it_be(:articles) { create_list(:article, 4, user: user) } + let_it_be(:pins) do + articles.each { |article| create(:profile_pin, pinnable_id: article.id, profile_id: user.id) } + end + + let(:fifth_article) { create(:article, user: user) } + let(:sixth_article) { create(:article, user: user) } + + it "allows up to five pins per user" do + pin = build(:profile_pin, pinnable_id: fifth_article.id, profile_id: user.id) + expect(pin).to be_valid + end + + it "disallows the sixth pin" do + create(:profile_pin, pinnable_id: fifth_article.id, profile_id: user.id) + expect do + create(:profile_pin, pinnable_id: sixth_article.id, profile_id: user.id) + end.to raise_error(ActiveRecord::RecordInvalid) + end end - it "disallows the sixth pin" do - create(:profile_pin, pinnable_id: article.id, profile_id: user.id) - create(:profile_pin, pinnable_id: second_article.id, profile_id: user.id) - create(:profile_pin, pinnable_id: third_article.id, profile_id: user.id) - create(:profile_pin, pinnable_id: fourth_article.id, profile_id: user.id) - create(:profile_pin, pinnable_id: fifth_article.id, profile_id: user.id) - expect do - create(:profile_pin, pinnable_id: sixth_article.id, profile_id: user.id) - end.to raise_error(ActiveRecord::RecordInvalid) - end + describe "#profile" do + let_it_be(:article) { create(:article, user: user) } - it "ensures pinnable belongs to profile" do - pin = build(:profile_pin, pinnable_id: article.id, profile_id: second_user.id) - expect(pin).not_to be_valid - end + it "ensures pinnable belongs to the same profile" do + pin = build(:profile_pin, pinnable_id: article.id, profile_id: create(:user).id) + expect(pin).not_to be_valid + end - it "ensures one pin per pinnable per profile" do - create(:profile_pin, pinnable_id: article.id, profile_id: user.id) - last_pin = build(:profile_pin, pinnable_id: article.id, profile_id: user.id) - expect(last_pin).not_to be_valid + it "ensures one pin per pinnable per profile" do + create(:profile_pin, pinnable_id: article.id, profile_id: user.id) + other_pin = build(:profile_pin, pinnable_id: article.id, profile_id: user.id) + expect(other_pin).not_to be_valid + end end end end diff --git a/spec/models/rating_vote_spec.rb b/spec/models/rating_vote_spec.rb index ec08b2cc8..b671ac075 100644 --- a/spec/models/rating_vote_spec.rb +++ b/spec/models/rating_vote_spec.rb @@ -2,9 +2,8 @@ require "rails_helper" RSpec.describe RatingVote, type: :model do let(:user) { create(:user, :trusted) } - let(:user2) { create(:user, :trusted) } - let(:user3) { create(:user, :trusted) } - let(:article) { create(:article, user_id: user.id) } + let(:user2) { create(:user, :trusted) } + let(:article) { create(:article, user: user) } describe "validations" do it { is_expected.to validate_numericality_of(:rating).is_greater_than(0.0).is_less_than_or_equal_to(10.0) } @@ -28,36 +27,42 @@ RSpec.describe RatingVote, type: :model do it "assigns article rating" do rating = create(:rating_vote, article_id: article.id, user_id: user.id, rating: 2.0) create(:rating_vote, article_id: article.id, user_id: user2.id, rating: 3.0) + rating.assign_article_rating - expect(article.reload.experience_level_rating).to eq(2.5) - expect(article.reload.experience_level_rating_distribution).to eq(1.0) + article.reload + + expect(article.experience_level_rating).to eq(2.5) + expect(article.experience_level_rating_distribution).to eq(1.0) end it "assigns article rating with larger distribution" do rating = create(:rating_vote, article_id: article.id, user_id: user.id, rating: 1.0) create(:rating_vote, article_id: article.id, user_id: user2.id, rating: 7.0) + rating.assign_article_rating - expect(article.reload.experience_level_rating).to eq(4.0) - expect(article.reload.experience_level_rating_distribution).to eq(6.0) + article.reload + + expect(article.experience_level_rating).to eq(4.0) + expect(article.experience_level_rating_distribution).to eq(6.0) end end describe "permissions" do + let_it_be(:untrusted_user) { create(:user) } + it "allows trusted users to make rating" do rating = build(:rating_vote, article_id: article.id, user_id: user.id) expect(rating).to be_valid end it "does not allow non-trusted users to make rating" do - nontrusted_user = create(:user) - rating = build(:rating_vote, article_id: article.id, user_id: nontrusted_user.id) + rating = build(:rating_vote, article_id: article.id, user_id: untrusted_user.id) expect(rating).not_to be_valid end it "does allows author to make rating on own post" do - nontrusted_user = create(:user) - article = create(:article, user_id: nontrusted_user.id) - rating = build(:rating_vote, article_id: article.id, user_id: nontrusted_user.id) + article = create(:article, user: untrusted_user) + rating = build(:rating_vote, article_id: article.id, user_id: untrusted_user.id) expect(rating).to be_valid end end diff --git a/spec/models/reaction_create_spec.rb b/spec/models/reaction_create_spec.rb deleted file mode 100644 index 5440139a1..000000000 --- a/spec/models/reaction_create_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -require "rails_helper" - -RSpec.describe Reaction, type: :model do - let(:article) { create(:article, featured: true) } - let(:user) { create(:user) } - - context "when creating and enqueueing" do - it "enqueues the Users::TouchJob" do - expect do - create(:reaction, reactable: article, user: user) - end.to have_enqueued_job(Users::TouchJob).exactly(:once).with(user.id) - end - - it "enqueues the Reactions::UpdateReactableJob" do - expect do - create(:reaction, reactable: article, user: user) - end.to have_enqueued_job(Reactions::UpdateReactableJob).exactly(:once) - end - - it "enqueues the Reactions::BustReactableCacheJob" do - expect do - create(:reaction, reactable: article, user: user) - end.to have_enqueued_job(Reactions::BustReactableCacheJob).exactly(:once) - end - - it "enqueues the Reactions::BustHomepageCacheJob" do - expect do - create(:reaction, reactable: article, user: user) - end.to have_enqueued_job(Reactions::BustHomepageCacheJob).exactly(:once) - end - end - - context "when creating and performing jobs" do - it "updates the reactable Comment" do - perform_enqueued_jobs do - updated_at = 1.day.ago - comment = create(:comment, commentable: article, updated_at: updated_at) - create(:reaction, reactable: comment, user: user) - expect(comment.reload.updated_at).to be > updated_at - end - end - - it "touches the user" do - perform_enqueued_jobs do - updated_at = 1.day.ago - user.update_columns(updated_at: updated_at) - create(:reaction, reactable: article, user: user) - expect(user.reload.updated_at).to be > updated_at - end - end - end -end diff --git a/spec/models/reaction_destroy_spec.rb b/spec/models/reaction_destroy_spec.rb deleted file mode 100644 index d31a30237..000000000 --- a/spec/models/reaction_destroy_spec.rb +++ /dev/null @@ -1,10 +0,0 @@ -require "rails_helper" - -RSpec.describe Reaction, type: :model do - let(:article) { create(:article, featured: true) } - let!(:reaction) { create(:reaction, reactable: article) } - - it "creates a ScoreCalcJob on article reaction destroy" do - expect { reaction.destroy }.to have_enqueued_job(Articles::ScoreCalcJob).exactly(:once) - end -end diff --git a/spec/models/reaction_spec.rb b/spec/models/reaction_spec.rb index 71936d836..a27f0b5bf 100644 --- a/spec/models/reaction_spec.rb +++ b/spec/models/reaction_spec.rb @@ -2,21 +2,15 @@ require "rails_helper" RSpec.describe Reaction, type: :model do let(:user) { create(:user) } - let(:article) { create(:article, featured: true) } - let(:comment) { create(:comment, user: user, commentable: article) } - let(:reaction) { build(:reaction, reactable: comment) } + let(:article) { create(:article, user: user) } + let(:reaction) { build(:reaction, reactable: article) } - describe "actual validation" do - subject { described_class.new(reactable: article, reactable_type: "Article", user: user) } - - before { user.add_role(:trusted) } + describe "builtin validations" do + subject { build(:reaction, reactable: article, user: user) } it { is_expected.to belong_to(:user) } it { is_expected.to validate_inclusion_of(:category).in_array(Reaction::CATEGORIES) } it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(%i[reactable_id reactable_type category]) } - - # Thumbsdown and Vomits test needed - # it { is_expected.to validate_inclusion_of(:reactable_type).in_array(%w(Comment Article)) } end describe "validations" do @@ -74,37 +68,87 @@ RSpec.describe Reaction, type: :model do end end - describe "async callbacks" do - it "runs async jobs effectively" do - u2 = create(:user) - c2 = create(:comment, commentable_id: article.id) - create(:reaction, user: u2, reactable: c2) - create(:reaction, user: u2, reactable: article) - expect(reaction).to be_valid + describe "#skip_notification_for?" do + let_it_be(:receiver) { build(:user) } + let_it_be(:reaction) { build(:reaction, reactable: build(:article), user: nil) } + + context "when false" do + it "is false when points are positive" do + reaction.points = 1 + expect(reaction.skip_notification_for?(receiver)).to be(false) + end + + it "is false when the person who reacted is not the same as the reactable owner" do + user_id = User.maximum(:id).to_i + 1 + reaction.user_id = user_id + reaction.reactable.user_id = user_id + 1 + expect(reaction.skip_notification_for?(user)).to be(false) + end + + it "is false when receive_notifications is true" do + reaction.reactable.receive_notifications = true + expect(reaction.skip_notification_for?(receiver)).to be(false) + end + end + + context "when true" do + it "is true when points are negative" do + reaction.points = -2 + expect(reaction.skip_notification_for?(receiver)).to be(true) + end + + it "is true when the person who reacted is the same as the reactable owner" do + user_id = User.maximum(:id).to_i + 1 + reaction.user_id = user_id + reaction.reactable.user_id = user_id + expect(reaction.skip_notification_for?(user)).to be(true) + end + + it "is true when the receive_notifications is false" do + reaction.reactable.receive_notifications = false + expect(reaction.skip_notification_for?(receiver)).to be(true) + end end end - describe "#skip_notification_for?" do - let(:receiver) { build(:user) } - let(:reaction2) { build(:reaction, reactable: comment, user_id: user.id + 1) } + context "when callbacks are called after save" do + let!(:reaction) { build(:reaction, category: "like", reactable: article, user: user) } - it "is false by default" do - expect(reaction2.skip_notification_for?(receiver)).to be(false) + it "enqueues the correct jobs" do + expect do + reaction.save + end.to( + have_enqueued_job(Users::TouchJob).with(user.id).exactly(:once). + and(have_enqueued_job(Reactions::UpdateReactableJob).exactly(:once)). + and(have_enqueued_job(Reactions::BustReactableCacheJob).exactly(:once)). + and(have_enqueued_job(Reactions::BustHomepageCacheJob).exactly(:once)), + ) end - it "is true when points are negative" do - reaction2.points = -2 - expect(reaction2.skip_notification_for?(receiver)).to be(true) + it "updates updated_at if the reactable is a comment" do + perform_enqueued_jobs do + updated_at = 1.day.ago + comment = create(:comment, commentable: article, updated_at: updated_at) + reaction.update(reactable: comment) + expect(comment.reload.updated_at).to be > updated_at + end end - it "is true when the receiver is the same user as the one who reacted" do - reaction.user = user - expect(reaction.skip_notification_for?(user)).to be(true) + it "updates updated_at for the user" do + perform_enqueued_jobs do + updated_at = user.updated_at + Timecop.travel(1.day.from_now) do + reaction.save + expect(user.reload.updated_at).to be > updated_at + end + end end + end - it "is true when the receive_notifications is false" do - comment.receive_notifications = false - expect(reaction.skip_notification_for?(receiver)).to be(true) + context "when callbacks are called before destroy" do + it "enqueues a ScoreCalcJob on article reaction destroy" do + reaction = create(:reaction, reactable: article, user: user) + expect { reaction.destroy }.to have_enqueued_job(Articles::ScoreCalcJob).exactly(:once) end end end diff --git a/spec/models/role_spec.rb b/spec/models/role_spec.rb index 09cc6a3d8..abbcb1efc 100644 --- a/spec/models/role_spec.rb +++ b/spec/models/role_spec.rb @@ -3,5 +3,16 @@ require "rails_helper" RSpec.describe Role, type: :model do it { is_expected.to belong_to(:resource).optional } it { is_expected.to validate_inclusion_of(:resource_type).in_array(Rolify.resource_types) } - it { is_expected.to validate_inclusion_of(:name).in_array(%w[super_admin admin single_resource_admin tech_admin tag_moderator trusted banned warned workshop_pass chatroom_beta_tester comment_banned pro podcast_admin]) } + it { is_expected.to validate_inclusion_of(:name).in_array(described_class::ROLES) } + + describe "::ROLES" do + it "contains the correct values" do + expected_roles = %w[ + admin banned chatroom_beta_tester comment_banned + podcast_admin pro single_resource_admin super_admin + tag_moderator tech_admin trusted warned workshop_pass + ] + expect(described_class::ROLES).to eq(expected_roles) + end + end end diff --git a/spec/models/search_keyword_spec.rb b/spec/models/search_keyword_spec.rb index ee06297e6..951e91dc1 100644 --- a/spec/models/search_keyword_spec.rb +++ b/spec/models/search_keyword_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" RSpec.describe SearchKeyword, type: :model do - let(:search_keyword) { create(:search_keyword) } + let(:search_keyword) { build(:search_keyword) } it { is_expected.to validate_presence_of(:keyword) } it { is_expected.to validate_presence_of(:google_result_path) } diff --git a/spec/models/sponsorship_spec.rb b/spec/models/sponsorship_spec.rb index d53bee685..4f7e148de 100644 --- a/spec/models/sponsorship_spec.rb +++ b/spec/models/sponsorship_spec.rb @@ -49,7 +49,7 @@ RSpec.describe Sponsorship, type: :model do end describe "validations" do - let(:org) { create(:organization) } + let(:org) { build(:organization) } it "forbids an org to have multiple 'expiring' sponsorships" do create(:sponsorship, level: :gold, organization: org) diff --git a/spec/models/tag_adjustment_spec.rb b/spec/models/tag_adjustment_spec.rb index 14d59efd8..296534921 100644 --- a/spec/models/tag_adjustment_spec.rb +++ b/spec/models/tag_adjustment_spec.rb @@ -1,75 +1,57 @@ require "rails_helper" RSpec.describe TagAdjustment, type: :model do - before do - mod_user.add_role(:tag_moderator, tag) - admin_user.add_role(:admin) - end - - let(:article) { create(:article) } - let(:tag) { create(:tag) } - let(:admin_user) { create(:user) } - let(:mod_user) { create(:user) } - let(:regular_user) { create(:user) } + let_it_be(:article) { create(:article) } + let_it_be(:admin_user) { create(:user, :admin) } + let_it_be(:regular_user) { create(:user) } it { is_expected.to validate_presence_of(:user_id) } it { is_expected.to validate_presence_of(:article_id) } it { is_expected.to validate_presence_of(:tag_id) } it { is_expected.to validate_presence_of(:tag_name) } it { is_expected.to validate_presence_of(:adjustment_type) } + it { is_expected.to validate_inclusion_of(:adjustment_type).in_array(%w[removal addition]) } it { is_expected.to validate_presence_of(:status) } - it { is_expected.to have_many(:notifications).dependent(:delete_all) } - describe "privileges" do - it "allows tag mods to create for their tags" do - tag_adjustment = build(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: tag.id) - expect(tag_adjustment).to be_valid - end - - it "does not allow tag mods to create for other tags" do - another_tag = create(:tag) - tag_adjustment = build(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: another_tag.id) - expect(tag_adjustment).to be_invalid - end - - it "allows admins to create for any tags" do - tag_adjustment = build(:tag_adjustment, user_id: admin_user.id, article_id: article.id, tag_id: tag.id) - expect(tag_adjustment).to be_valid - end - - it "does not allow normal users to create for any tags" do - tag_adjustment = build(:tag_adjustment, user_id: regular_user.id, article_id: article.id, tag_id: tag.id) - expect(tag_adjustment).to be_invalid - end + it do + # rubocop:disable RSpec/NamedSubject + expect(subject).to validate_inclusion_of(:status).in_array( + %w[committed pending committed_and_resolvable resolved], + ) + # rubocop:enable RSpec/NamedSubject end - describe "allowed attribute states" do - it "allows proper adjustment_types" do - tag_adjustment = build(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: tag.id, adjustment_type: "removal") - expect(tag_adjustment).to be_valid - end + it { is_expected.to have_many(:notifications).dependent(:delete_all) } - it "disallows improper adjustment_types" do - tag_adjustment = build(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: tag.id, adjustment_type: "slushie") - expect(tag_adjustment).to be_invalid - end + describe "validations" do + let(:tag) { create(:tag) } + let(:mod_user) { create(:user) } - it "allows proper status" do - tag_adjustment = build(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: tag.id, status: "committed") - expect(tag_adjustment).to be_valid - end + describe "privileges" do + before do + mod_user.add_role(:tag_moderator, tag) + end - it "disallows improper status" do - tag_adjustment = build(:tag_adjustment, user_id: mod_user.id, article_id: article.id, tag_id: tag.id, status: "slushiemonkey") - expect(tag_adjustment).to be_invalid + it "allows tag mods to create for their tags" do + tag_adjustment = build(:tag_adjustment, user: mod_user, article: article, tag: tag) + expect(tag_adjustment).to be_valid + end + + it "does not allow tag mods to create for other tags" do + another_tag = create(:tag) + tag_adjustment = build(:tag_adjustment, user: mod_user, article: article, tag: another_tag) + expect(tag_adjustment).to be_invalid + end + + it "allows admins to create for any tags" do + tag_adjustment = build(:tag_adjustment, user: admin_user, article: article, tag: tag) + expect(tag_adjustment).to be_valid + end + + it "does not allow normal users to create for any tags" do + tag_adjustment = build(:tag_adjustment, user: regular_user, article: article, tag: tag) + expect(tag_adjustment).to be_invalid + end end end end - -# t.integer :user_id -# t.integer :article_id -# t.integer :tag_id -# t.string :tag_name -# t.string :adjustment_type -# t.string :status -# t.string :reason_for_adjustment diff --git a/spec/models/user_block_spec.rb b/spec/models/user_block_spec.rb index 08dd6f2ff..7fafb87ac 100644 --- a/spec/models/user_block_spec.rb +++ b/spec/models/user_block_spec.rb @@ -1,16 +1,15 @@ require "rails_helper" RSpec.describe UserBlock, type: :model do - let(:blocker) { create(:user) } - let(:blocked) { create(:user) } + let(:blocker) { build(:user) } describe "validations" do it { is_expected.to validate_inclusion_of(:config).in_array(%w[default]) } it "prevents the blocker from blocking itself" do - user_block = described_class.new(blocker_id: 1, blocked_id: 1, config: "default") - expect(user_block.valid?).to eq false - expect(user_block.errors.full_messages).to include "Blocker can't be the same as the blocked_id" + user_block = build(:user_block, blocker: blocker, blocked: blocker, config: "default") + expect(user_block).not_to be_valid + expect(user_block.errors.full_messages).to include("Blocker can't be the same as the blocked_id") end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 04d9b3e11..054c8d0db 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1,45 +1,66 @@ require "rails_helper" +def user_from_authorization_service(service_name, signed_in_resource, cta_variant) + auth = OmniAuth.config.mock_auth[service_name] + service = AuthorizationService.new(auth, signed_in_resource, cta_variant) + service.get_user +end + RSpec.describe User, type: :model do - let!(:user) { create(:user) } - let(:returning_user) { create(:user, signup_cta_variant: nil) } - let(:second_user) { create(:user) } - let(:article) { create(:article, user_id: user.id) } - let(:tag) { create(:tag) } - let(:org) { create(:organization) } - let(:second_org) { create(:organization) } + let(:user) { create(:user) } + let(:other_user) { create(:user) } + let(:org) { create(:organization) } before { mock_auth_hash } describe "validations" do - it { is_expected.to have_many(:api_secrets) } - it { is_expected.to have_many(:articles) } - it { is_expected.to have_many(:badge_achievements).dependent(:destroy) } - it { is_expected.to have_many(:badges).through(:badge_achievements) } - it { is_expected.to have_many(:collections).dependent(:destroy) } - it { is_expected.to have_many(:comments) } - it { is_expected.to have_many(:email_messages).class_name("Ahoy::Message") } - it { is_expected.to have_many(:identities).dependent(:destroy) } - it { is_expected.to have_many(:mentions).dependent(:destroy) } - it { is_expected.to have_many(:notes) } - it { is_expected.to have_many(:notifications).dependent(:destroy) } - it { is_expected.to have_many(:reactions).dependent(:destroy) } - it { is_expected.to have_many(:tweets).dependent(:destroy) } - it { is_expected.to have_many(:github_repos).dependent(:destroy) } - it { is_expected.to have_many(:chat_channel_memberships).dependent(:destroy) } - it { is_expected.to have_many(:chat_channels).through(:chat_channel_memberships) } - it { is_expected.to have_many(:push_notification_subscriptions).dependent(:destroy) } - it { is_expected.to have_many(:notification_subscriptions).dependent(:destroy) } - it { is_expected.to have_one(:pro_membership).dependent(:destroy) } - it { is_expected.to validate_uniqueness_of(:username).case_insensitive } - it { is_expected.to validate_uniqueness_of(:github_username).allow_nil } - it { is_expected.to validate_uniqueness_of(:twitter_username).allow_nil } - it { is_expected.to validate_presence_of(:username) } - it { is_expected.to validate_length_of(:username).is_at_most(30).is_at_least(2) } - it { is_expected.to validate_length_of(:name).is_at_most(100) } - it { is_expected.to validate_inclusion_of(:inbox_type).in_array(%w[open private]) } - it { is_expected.to have_many(:access_grants).class_name("Doorkeeper::AccessGrant").with_foreign_key("resource_owner_id").dependent(:delete_all) } - it { is_expected.to have_many(:access_tokens).class_name("Doorkeeper::AccessToken").with_foreign_key("resource_owner_id").dependent(:delete_all) } + describe "builtin validations" do + it { is_expected.to have_many(:api_secrets) } + it { is_expected.to have_many(:articles) } + it { is_expected.to have_many(:badge_achievements).dependent(:destroy) } + it { is_expected.to have_many(:badges).through(:badge_achievements) } + it { is_expected.to have_many(:collections).dependent(:destroy) } + it { is_expected.to have_many(:comments) } + it { is_expected.to have_many(:email_messages).class_name("Ahoy::Message") } + it { is_expected.to have_many(:identities).dependent(:destroy) } + it { is_expected.to have_many(:mentions).dependent(:destroy) } + it { is_expected.to have_many(:notes) } + it { is_expected.to have_many(:notifications).dependent(:destroy) } + it { is_expected.to have_many(:reactions).dependent(:destroy) } + it { is_expected.to have_many(:tweets).dependent(:destroy) } + it { is_expected.to have_many(:github_repos).dependent(:destroy) } + it { is_expected.to have_many(:chat_channel_memberships).dependent(:destroy) } + it { is_expected.to have_many(:chat_channels).through(:chat_channel_memberships) } + it { is_expected.to have_many(:push_notification_subscriptions).dependent(:destroy) } + it { is_expected.to have_many(:notification_subscriptions).dependent(:destroy) } + it { is_expected.to have_one(:pro_membership).dependent(:destroy) } + + # rubocop:disable RSpec/NamedSubject + it do + expect(subject).to have_many(:access_grants). + class_name("Doorkeeper::AccessGrant"). + with_foreign_key("resource_owner_id"). + dependent(:delete_all) + end + + it do + expect(subject).to have_many(:access_tokens). + class_name("Doorkeeper::AccessToken"). + with_foreign_key("resource_owner_id"). + dependent(:delete_all) + end + # rubocop:enable RSpec/NamedSubject + + it { is_expected.to have_many(:organization_memberships).dependent(:destroy) } + + it { is_expected.to validate_uniqueness_of(:username).case_insensitive } + it { is_expected.to validate_uniqueness_of(:github_username).allow_nil } + it { is_expected.to validate_uniqueness_of(:twitter_username).allow_nil } + it { is_expected.to validate_presence_of(:username) } + it { is_expected.to validate_length_of(:username).is_at_most(30).is_at_least(2) } + it { is_expected.to validate_length_of(:name).is_at_most(100).is_at_least(1) } + it { is_expected.to validate_inclusion_of(:inbox_type).in_array(%w[open private]) } + end it "validates username against reserved words" do user = build(:user, username: "readinglist") @@ -69,306 +90,445 @@ RSpec.describe User, type: :model do end end - # the followings are failing - # it { is_expected.to have_many(:keys) } - # it { is_expected.to have_many(:job_applications) } - # it { is_expected.to have_many(:answers) } - # it { is_expected.to validate_uniqueness_of(:email).case_insensitive.allow_blank } + context "when callbacks are triggered before validation" do + let(:user) { build(:user) } - def user_from_authorization_service(service_name, signed_in_resource, cta_variant) - auth = OmniAuth.config.mock_auth[service_name] - service = AuthorizationService.new(auth, signed_in_resource, cta_variant) - service.get_user - end + describe "#twitter_username" do + it "sets twitter username to nil if empty" do + user.twitter_username = "" + user.validate! + expect(user.twitter_username).to eq(nil) + end - describe "makes sure usernames and email are not blank" do - it "sets twitter username to nil" do - user = create(:user, twitter_username: "") - user.reload - expect(user.twitter_username).to eq(nil) + it "does not change a valid name" do + user.twitter_username = "hello" + user.validate! + expect(user.twitter_username).to eq("hello") + end end - it "sets github username to nil" do - user = create(:user, github_username: "") - user.reload - expect(user.github_username).to eq(nil) + describe "#github_username" do + it "sets github username to nil if empty" do + user.github_username = "" + user.validate! + expect(user.github_username).to eq(nil) + end + + it "does not change a valid name" do + user.github_username = "hello" + user.validate! + expect(user.github_username).to eq("hello") + end end - it "sets correct usernames if they are not blank" do - user = create(:user, github_username: "hello", twitter_username: "world") - user.reload - expect(user.github_username).to eq("hello") - expect(user.twitter_username).to eq("world") + describe "#email" do + it "sets email to nil if empty" do + user.email = "" + user.validate! + expect(user.email).to eq(nil) + end + + it "does not change a valid name" do + user.email = "anna@example.com" + user.validate! + expect(user.email).to eq("anna@example.com") + end end - it "sets email to nil" do - user = create(:user, email: "") - user.reload - expect(user.email).to eq(nil) + describe "#username" do + it "receives a temporary username if none is given" do + user.username = "" + user.validate! + expect(user.username).not_to be_blank + end + + it "does not allow to change to a username that is taken" do + user.username = other_user.username + expect(user).not_to be_valid + end + + it "does not allow to change to a username that is taken by an organization" do + user.username = create(:organization).slug + expect(user).not_to be_valid + end end - it "sets correct email if it's not blank" do - user = create(:user, email: "anna@example.com") - user.reload - expect(user.email).to eq("anna@example.com") + describe "#website_url" do + it "does not accept invalid website url" do + user.website_url = "ben.com" + expect(user).not_to be_valid + end + + it "accepts valid http website url" do + user.website_url = "http://ben.com" + expect(user).to be_valid + end end - it "sets onboarding_variant_version" do - user = create(:user, email: "anna@example.com") - user.reload - expect(user.onboarding_variant_version).to be_in(%w[0 1 2 3 4 5 6 7 8 9]) + describe "#mastodon_url" do + it "accepts valid https mastodon url" do + user.mastodon_url = "https://mastodon.social/@test" + expect(user).to be_valid + end + + it "does not accept a denied mastodon instance" do + user.mastodon_url = "https://SpammyMcSpamface.com/" + expect(user).not_to be_valid + end + + it "does not accept invalid mastodon url" do + user.mastodon_url = "mastodon.social/@test" + expect(user).not_to be_valid + end + end + + describe "#facebook_url" do + it "accepts valid https facebook url", :aggregate_failures do + %w[thepracticaldev thepracticaldev/ the.practical.dev].each do |username| + user.facebook_url = "https://facebook.com/#{username}" + expect(user).to be_valid + end + end + + it "does not accept invalid facebook url" do + user.facebook_url = "ben.com" + expect(user).not_to be_valid + end + end + + describe "#behance_url" do + it "accepts valid https behance url", :aggregate_failures do + %w[jess jess/ je-ss jes_ss].each do |username| + user.behance_url = "https://behance.net/#{username}" + expect(user).to be_valid + end + end + + it "does not accept invalid behance url" do + user.behance_url = "ben.com" + expect(user).not_to be_valid + end + end + + describe "#twitch_url" do + it "does not accept invalid twitch url" do + user.twitch_url = "ben.com" + expect(user).not_to be_valid + end + + it "accepts valid https twitch url", :aggregate_failures do + %w[pandyzhao pandyzhao/ PandyZhao_ pandy_Zhao].each do |username| + user.twitch_url = "https://twitch.tv/#{username}" + expect(user).to be_valid + end + end + end + + describe "#stackoverflow_url" do + it "accepts valid https stackoverflow url", :aggregate_failures do + %w[pandyzhao pandyzhao/ pandy-zhao].each do |username| + user.stackoverflow_url = "https://stackoverflow.com/users/7381391/#{username}" + expect(user).to be_valid + end + end + + it "does not accept invalid stackoverflow url" do + user.stackoverflow_url = "ben.com" + expect(user).not_to be_valid + end + + it "accepts valid stackoverflow sub community url", :aggregate_failures do + %w[pt ru es ja].each do |subcommunity| + user.stackoverflow_url = "https://#{subcommunity}.stackoverflow.com/users/7381391/mazen" + expect(user).to be_valid + end + end + + it "does not accept invalid stackoverflow sub community url" do + user.stackoverflow_url = "https://fr.stackoverflow.com/users/7381391/mazen" + expect(user).not_to be_valid + end + end + + describe "#linkedin_url" do + it "accepts valid https linkedin url", :aggregate_failures do + %w[jessleenyc jessleenyc/ jess-lee-nyc].each do |username| + user.linkedin_url = "https://linkedin.com/in/#{username}" + expect(user).to be_valid + end + end + + it "accepts valid country specific https linkedin url" do + user.linkedin_url = "https://mx.linkedin.com/in/jessleenyc" + expect(user).to be_valid + end + + it "does not accept three letters country codes in http linkedin url" do + user.linkedin_url = "http://mex.linkedin.com/in/jessleenyc" + expect(user).not_to be_valid + end + + it "does not accept three letters country codes in https linkedin url" do + user.linkedin_url = "https://mex.linkedin.com/in/jessleenyc" + expect(user).not_to be_valid + end + + it "does not accept invalid linkedin url" do + user.linkedin_url = "ben.com" + expect(user).not_to be_valid + end + end + + describe "#dribbble_url", :aggregate_failures do + it "accepts valid https dribbble url" do + %w[jess jess/ je-ss je_ss].each do |username| + user.dribbble_url = "https://dribbble.com/#{username}" + expect(user).to be_valid + end + end + + it "does not accept invalid dribbble url" do + user.dribbble_url = "ben.com" + expect(user).not_to be_valid + end + end + + describe "#medium_url" do + it "accepts valid https medium url", :aggregate_failures do + %w[jess jess/ je-ss je_ss].each do |username| + user.medium_url = "https://medium.com/#{username}" + expect(user).to be_valid + end + end + + it "does not accept invalid medium url" do + user.medium_url = "ben.com" + expect(user).not_to be_valid + end + end + + describe "#instagram_url" do + it "does not accept invalid instagram url" do + user.instagram_url = "ben.com" + expect(user).not_to be_valid + end + + it "accepts valid instagram url", :aggregate_failures do + %w[jess je_ss je_ss.tt A.z.E.r.T.y].each do |username| + user.instagram_url = "https://instagram.com/#{username}" + expect(user).to be_valid + end + end + end + + describe "#gitlab_url" do + it "accepts valid https gitlab url", :aggregate_failures do + %w[jess jess/ je-ss je_ss].each do |username| + user.gitlab_url = "https://gitlab.com/#{username}" + expect(user).to be_valid + end + end + + it "does not accept invalid gitlab url" do + user.gitlab_url = "ben.com" + expect(user).not_to be_valid + end + end + + describe "#employer_url" do + it "does not accept invalid employer url" do + user.employer_url = "ben.com" + expect(user).not_to be_valid + end + + it "does accept valid http employer url" do + user.employer_url = "http://ben.com" + expect(user).to be_valid + end + + it "does accept valid https employer url" do + user.employer_url = "https://ben.com" + expect(user).to be_valid + end + end + + describe "#config_theme" do + it "accepts valid theme" do + user.config_theme = "night theme" + expect(user).to be_valid + end + + it "does not accept invalid theme" do + user.config_theme = "no night mode" + expect(user).not_to be_valid + end + end + + describe "#config_font" do + it "accepts valid font" do + user.config_font = "sans serif" + expect(user).to be_valid + end + + it "does not accept invalid font" do + user.config_font = "goobledigook" + expect(user).not_to be_valid + end + end + + describe "#config_navbar" do + it "accepts valid navbar" do + user.config_navbar = "static" + expect(user).to be_valid + end + + it "does not accept invalid navbar" do + user.config_navbar = "not valid navbar input" + expect(user).not_to be_valid + end + end + + context "when the past value is relevant" do + let(:user) { create(:user) } + + it "changes old_username and old_old_username properly if username changes" do + old_username = user.username + random_new_username = "username_#{rand(100_000_000)}" + user.update(username: random_new_username) + expect(user.username).to eq(random_new_username) + expect(user.old_username).to eq(old_username) + + new_username = user.username + user.update(username: "username_#{rand(100_000_000)}") + expect(user.old_username).to eq(new_username) + expect(user.old_old_username).to eq(old_username) + end + + it "enforces summary length validation if previous summary was valid" do + user.summary = "0" * 999 + user.save(validate: false) + user.summary = "0" * 999 + expect(user).to be_valid + end + + it "does not enforce summary validation if previous summary was invalid" do + user = build(:user, summary: "0" * 999) + expect(user).not_to be_valid + end end end - describe "validations" do - it "gets a username after create" do - expect(user.username).not_to eq(nil) - end + context "when callbacks are triggered before and after create" do + let_it_be(:user) { create(:user, email: nil) } - it "does not accept invalid website url" do - user.website_url = "ben.com" - expect(user).not_to be_valid - end + describe "#language_settings" do + it "sets correct language_settings by default" do + expect(user.language_settings).to eq("preferred_languages" => %w[en]) + end - it "accepts valid https mastodon url" do - user.mastodon_url = "https://mastodon.social/@test" - expect(user).to be_valid - end - - it "does not accept a denied mastodon instance" do - user.mastodon_url = "https://SpammyMcSpamface.com/" - expect(user).not_to be_valid - end - - it "does not accept invalid mastodon url" do - user.mastodon_url = "mastodon.social/@test" - expect(user).not_to be_valid - end - - it "accepts valid http website url" do - user.website_url = "http://ben.com" - expect(user).to be_valid - end - - it "accepts valid https website url" do - user.website_url = "https://ben.com" - expect(user).to be_valid - end - - it "accepts valid https facebook url" do - %w[thepracticaldev thepracticaldev/ the.practical.dev].each do |username| - user.facebook_url = "https://facebook.com/#{username}" - expect(user).to be_valid + it "sets correct language_settings by default after the jobs are processed" do + perform_enqueued_jobs do + expect(user.language_settings).to eq("preferred_languages" => %w[en]) + end end end - it "does not accept invalid facebook url" do - user.facebook_url = "ben.com" - expect(user).not_to be_valid - end + describe "#estimated_default_language" do + it "estimates default language to be nil" do + perform_enqueued_jobs do + expect(user.estimated_default_language).to be(nil) + end + end - it "accepts valid https behance url" do - %w[jess jess/ je-ss jes_ss].each do |username| - user.behance_url = "https://behance.net/#{username}" - expect(user).to be_valid + it "estimates default language to be japanese with .jp email" do + perform_enqueued_jobs do + user = create(:user, email: "ben@hello.jp") + expect(user.reload.estimated_default_language).to eq("ja") + end + end + + it "estimates default language based on ID dump" do + perform_enqueued_jobs do + new_user = user_from_authorization_service(:twitter, nil, "navbar_basic") + expect(new_user.estimated_default_language).to eq(nil) + end end end - it "does not accept invalid behance url" do - user.behance_url = "ben.com" - expect(user).not_to be_valid - end - - it "does not accept invalid twitch url" do - user.twitch_url = "ben.com" - expect(user).not_to be_valid - end - - it "accepts valid https twitch url" do - %w[pandyzhao pandyzhao/ PandyZhao_ pandy_Zhao].each do |username| - user.twitch_url = "https://twitch.tv/#{username}" - expect(user).to be_valid + describe "#preferred_languages_array" do + it "returns proper preferred_languages_array" do + perform_enqueued_jobs do + user = create(:user, email: "ben@hello.jp") + expect(user.reload.preferred_languages_array).to eq(%w[en ja]) + end end - end - it "accepts valid https stackoverflow url" do - %w[pandyzhao pandyzhao/ pandy-zhao].each do |username| - user.stackoverflow_url = "https://stackoverflow.com/users/7381391/#{username}" - expect(user).to be_valid + it "returns a correct array when language settings are in a new format" do + language_settings = { estimated_default_language: "en", preferred_languages: %w[en ru it] } + user = build(:user, language_settings: language_settings) + expect(user.preferred_languages_array).to eq(%w[en ru it]) end - end - it "does not accept invalid stackoverflow url" do - user.stackoverflow_url = "ben.com" - expect(user).not_to be_valid - end - - it "accepts valid stackoverflow sub community url" do - %w[pt ru es ja].each do |subcommunity| - user.stackoverflow_url = "https://#{subcommunity}.stackoverflow.com/users/7381391/mazen" - expect(user).to be_valid + it "returns a correct array when language settings are in the old format" do + language_settings = { + estimated_default_language: "en", + prefer_language_en: true, + prefer_language_ja: false, + prefer_language_es: true + } + user = build(:user, language_settings: language_settings) + expect(user.preferred_languages_array).to eq(%w[en es]) end end - - it "does not accept invalid stackoverflow sub community url" do - user.stackoverflow_url = "https://fr.stackoverflow.com/users/7381391/mazen" - expect(user).not_to be_valid - end - - it "accepts valid https linkedin url" do - %w[jessleenyc jessleenyc/ jess-lee-nyc].each do |username| - user.linkedin_url = "https://linkedin.com/in/#{username}" - expect(user).to be_valid - end - end - - it "accepts valid country specific https linkedin url" do - user.linkedin_url = "https://mx.linkedin.com/in/jessleenyc" - expect(user).to be_valid - end - - it "does not accept three letters country codes in http linkedin url" do - user.linkedin_url = "http://mex.linkedin.com/in/jessleenyc" - expect(user).not_to be_valid - end - - it "does not accept three letters country codes in https linkedin url" do - user.linkedin_url = "https://mex.linkedin.com/in/jessleenyc" - expect(user).not_to be_valid - end - - it "does not accept invalid linkedin url" do - user.linkedin_url = "ben.com" - expect(user).not_to be_valid - end - - it "accepts valid https dribbble url" do - %w[jess jess/ je-ss je_ss].each do |username| - user.dribbble_url = "https://dribbble.com/#{username}" - expect(user).to be_valid - end - end - - it "does not accept invalid dribbble url" do - user.dribbble_url = "ben.com" - expect(user).not_to be_valid - end - - it "accepts valid https medium url" do - %w[jess jess/ je-ss je_ss].each do |username| - user.medium_url = "https://medium.com/#{username}" - expect(user).to be_valid - end - end - - it "does not accept invalid medium url" do - user.medium_url = "ben.com" - expect(user).not_to be_valid - end - - it "does not accept invalid instagram url" do - user.instagram_url = "ben.com" - expect(user).not_to be_valid - end - - it "accepts valid instagram url" do - %w[jess je_ss je_ss.tt A.z.E.r.T.y].each do |username| - user.instagram_url = "https://instagram.com/#{username}" - expect(user).to be_valid - end - end - - it "accepts valid https gitlab url" do - %w[jess jess/ je-ss je_ss].each do |username| - user.gitlab_url = "https://gitlab.com/#{username}" - expect(user).to be_valid - end - end - - it "does not accept invalid gitlab url" do - user.gitlab_url = "ben.com" - expect(user).not_to be_valid - end - - it "changes old_username and old_old_username properly if username changes" do - old_username = user.username - random_new_username = "username_#{rand(100_000_000)}" - user.update(username: random_new_username) - expect(user.username).to eq(random_new_username) - expect(user.old_username).to eq(old_username) - new_username = user.username - user.update(username: "username_#{rand(100_000_000)}") - expect(user.old_username).to eq(new_username) - expect(user.old_old_username).to eq(old_username) - end - - it "does not allow too short or too long name" do - user.name = "" - expect(user).not_to be_valid - user.name = Faker::Lorem.paragraph_by_chars(number: 200) - expect(user).not_to be_valid - end - - it "does not accept invalid employer url" do - user.employer_url = "ben.com" - expect(user).not_to be_valid - end - - it "does accept valid http employer url" do - user.employer_url = "http://ben.com" - expect(user).to be_valid - end - - it "does accept valid https employer url" do - user.employer_url = "https://ben.com" - expect(user).to be_valid - end - - it "enforces summary length validation if old summary was valid" do - user.summary = "0" * 999 - user.save(validate: false) - user.summary = "0" * 999 - expect(user).to be_valid - end - - it "does not inforce summary validation if old summary was invalid" do - user.summary = "0" * 999 - expect(user).not_to be_valid - end - - it "accepts valid theme" do - user.config_theme = "night theme" - expect(user).to be_valid - end - - it "does not accept invalid theme" do - user.config_theme = "no night mode" - expect(user).not_to be_valid - end - - it "accepts valid font" do - user.config_font = "sans serif" - expect(user).to be_valid - end - - it "does not accept invalid font" do - user.config_theme = "goobledigook" - expect(user).not_to be_valid - end - - it "accepts valid navbar" do - user.config_navbar = "static" - expect(user).to be_valid - end - - it "does not accept invalid navbar" do - user.config_navbar = "not valid navbar input" - expect(user).not_to be_valid - end end - ## Registration + context "when callbacks are triggered after save" do + describe "subscribing to mailchip newsletter" do + it "enqueues SubscribeToMailchimpNewsletterJob" do + expect do + user.save + end.to have_enqueued_job(Users::SubscribeToMailchimpNewsletterJob).exactly(:once).with(user.id) + end + + it "does not enqueue without an email" do + expect do + user.update(email: "") + end.not_to have_enqueued_job(Users::SubscribeToMailchimpNewsletterJob).exactly(:once).with(user.id) + end + + it "does not enqueue with an invalid email" do + expect do + user.update(email: "foobar") + end.not_to have_enqueued_job(Users::SubscribeToMailchimpNewsletterJob).exactly(:once).with(user.id) + end + + it "does not enqueue with an unconfirmed email" do + expect do + user.update(unconfirmed_email: "bob@bob.com", confirmation_sent_at: Time.current) + end.not_to have_enqueued_job(Users::SubscribeToMailchimpNewsletterJob).exactly(:once).with(user.id) + end + end + end + + context "when indexing and deindexing" do + it "triggers background auto-indexing when user is saved" do + expect { user.save }.to have_enqueued_job.with(user, "index!").on_queue("algoliasearch") + end + + it "doesn't enqueue a job on destroy" do + user = build(:user) + + perform_enqueued_jobs do + user.save + end + + expect { user.destroy }.not_to have_enqueued_job.on_queue("algoliasearch") + end + end + describe "user registration" do + let(:user) { create(:user) } + it "finds user by email and assigns identity to that if exists" do OmniAuth.config.mock_auth[:twitter].info.email = user.email @@ -385,7 +545,6 @@ RSpec.describe User, type: :model do end it "assigns random username if username is taken by organization on registration" do - org = create(:organization) OmniAuth.config.mock_auth[:twitter].info.nickname = org.slug new_user = user_from_authorization_service(:twitter, nil, "navbar_basic") @@ -399,6 +558,7 @@ RSpec.describe User, type: :model do end it "does not assign signup_cta_variant to non-new users" do + returning_user = build(:user, signup_cta_variant: nil) new_user = user_from_authorization_service(:twitter, returning_user, "hey-hey-hey") expect(new_user.signup_cta_variant).to eq(nil) end @@ -426,285 +586,110 @@ RSpec.describe User, type: :model do expect(new_user.identities.size).to eq(2) end - context "when estimating the default language" do - it "sets correct language_settings by default" do - user2 = create(:user, email: nil) - expect(user2.language_settings).to eq("preferred_languages" => %w[en]) - end + it "persists JSON dump of identity data" do + new_user = user_from_authorization_service(:twitter, nil, "navbar_basic") + identity = new_user.identities.first + expect(identity.auth_data_dump.provider).to eq(identity.provider) + end - it "sets correct language_settings by default after the callbacks" do - perform_enqueued_jobs do - user2 = create(:user, email: nil) - expect(user2.language_settings).to eq("preferred_languages" => %w[en]) - end - end + it "persists extracts relevant identity data from new twitter user" do + new_user = user_from_authorization_service(:twitter, nil, "navbar_basic") + expect(new_user.twitter_followers_count).to eq(100) + expect(new_user.twitter_created_at).to be_kind_of(ActiveSupport::TimeWithZone) + end - it "estimates default language to be nil" do - perform_enqueued_jobs do - user.estimate_default_language - end - expect(user.reload.estimated_default_language).to eq(nil) - end - - it "estimates default language to be japan with jp email" do - perform_enqueued_jobs do - user.update_column(:email, "ben@hello.jp") - user.estimate_default_language - end - expect(user.reload.estimated_default_language).to eq("ja") - end - - it "estimates default language based on ID dump" do - perform_enqueued_jobs do - new_user = user_from_authorization_service(:twitter, nil, "navbar_basic") - new_user.estimate_default_language - expect(user.reload.estimated_default_language).to eq(nil) - end - end - - it "returns proper preferred_languages_array" do - perform_enqueued_jobs do - user.update_column(:email, "ben@hello.jp") - user.estimate_default_language - end - expect(user.reload.preferred_languages_array).to include("ja") - end + it "persists extracts relevant identity data from new github user" do + new_user = user_from_authorization_service(:github, nil, "navbar_basic") + expect(new_user.github_created_at).to be_kind_of(ActiveSupport::TimeWithZone) end end - describe "#preferred_languages_array" do - it "returns a correct array when language settings are in a new format" do - user.update_columns(language_settings: { estimated_default_language: "en", preferred_languages: %w[en ru it] }) - expect(user.preferred_languages_array).to eq(%w[en ru it]) + describe "#follow and #all_follows" do + it "follows users" do + expect do + user.follow(create(:user)) + end.to change(user.all_follows, :size).by(1) end - - it "returns a correct array when language settings are in the old format" do - user.update_columns(language_settings: { estimated_default_language: "en", prefer_language_en: true, prefer_language_ja: false, prefer_language_es: true }) - expect(user.preferred_languages_array).to eq(%w[en es]) - end - end - - it "follows users" do - user2 = create(:user) - user3 = create(:user) - user.follow(user2) - user.follow(user3) - expect(user.all_follows.size).to eq(2) end describe "#moderator_for_tags" do - let(:tag1) { create(:tag) } - let(:tag2) { create(:tag) } - let(:tag3) { create(:tag) } + let(:tags) { create_list(:tag, 2) } it "lists tags user moderates" do - user.add_role(:tag_moderator, tag1) - user.add_role(:tag_moderator, tag2) - expect(user.moderator_for_tags).to include(tag1.name) - expect(user.moderator_for_tags).to include(tag2.name) - expect(user.moderator_for_tags).not_to include(tag3.name) + user.add_role(:tag_moderator, tags.first) + + expect(user.moderator_for_tags).to include(tags.first.name) + expect(user.moderator_for_tags).not_to include(tags.last.name) end it "returns empty array if no tags moderated" do - expect(user.moderator_for_tags).to eq([]) + expect(user.moderator_for_tags).to be_empty end end describe "#followed_articles" do - let(:user2) { create(:user) } - let(:user3) { create(:user) } + let_it_be(:another_user) { create(:user) } + let_it_be(:articles) { create_list(:article, 2, user: another_user) } before do - create_list(:article, 3, user_id: user2.id) - user.follow(user2) + user.follow(another_user) end it "returns all articles following" do - expect(user.followed_articles.size).to eq(3) + expect(user.followed_articles.size).to eq(articles.size) end it "returns segment of articles if limit is passed" do - expect(user.followed_articles.limit(2).size).to eq(2) + expect(user.followed_articles.limit(1).size).to eq(articles.size - 1) end end - describe "#cached_followed_tags" do - let(:tag1) { create(:tag) } - let(:tag2) { create(:tag) } - let(:tag3) { create(:tag) } - let(:tag4) { create(:tag) } + describe "#calculate_score" do + it "calculates a score" do + create(:article, featured: true, user: user) - it "returns empty if no tags followed" do - expect(user.decorate.cached_followed_tags.size).to eq(0) + user.calculate_score + expect(user.score).to be_positive end - - it "returns array of tags if user follows them" do - user.follow(tag1) - user.follow(tag2) - user.follow(tag3) - expect(user.decorate.cached_followed_tags.size).to eq(3) - end - - it "returns tag object with name" do - user.follow(tag1) - expect(user.decorate.cached_followed_tags.first.name).to eq(tag1.name) - end - - it "returns follow points for tag" do - user.follow(tag1) - expect(user.decorate.cached_followed_tags.first.points).to eq(1.0) - end - - it "returns adjusted points for tag" do - user.follow(tag1) - Follow.last.update(points: 0.1) - expect(user.decorate.cached_followed_tags.first.points).to eq(0.1) - end - end - - describe "theming properties" do - it "creates proper body class with defaults" do - expect(user.decorate.config_body_class).to eq("default default-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} #{user.config_navbar}-navbar-config") - end - - it "determines dark theme if night theme" do - user.config_theme = "night_theme" - expect(user.decorate.dark_theme?).to eq(true) - end - - it "determines dark theme if ten x hacker" do - user.config_theme = "ten_x_hacker_theme" - expect(user.decorate.dark_theme?).to eq(true) - end - - it "determines not dark theme if not one of the dark themes" do - user.config_theme = "default" - expect(user.decorate.dark_theme?).to eq(false) - end - - it "creates proper body class with sans serif config" do - user.config_font = "sans_serif" - expect(user.decorate.config_body_class).to eq("default sans-serif-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} #{user.config_navbar}-navbar-config") - end - - it "creates proper body class with night theme" do - user.config_theme = "night_theme" - expect(user.decorate.config_body_class).to eq("night-theme default-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} #{user.config_navbar}-navbar-config") - end - - it "creates proper body class with pink theme" do - user.config_theme = "pink_theme" - expect(user.decorate.config_body_class).to eq("pink-theme default-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} #{user.config_navbar}-navbar-config") - end - - it "creates proper body class with minimal light theme" do - user.config_theme = "minimal_light_theme" - expect(user.decorate.config_body_class).to eq("minimal-light-theme default-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} #{user.config_navbar}-navbar-config") - end - - it "creates proper body class with pro user" do - user.add_role(:pro) - expect(user.decorate.config_body_class).to eq("default default-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} #{user.config_navbar}-navbar-config") - end - - it "creates proper body class with trusted user" do - user.add_role(:trusted) - expect(user.decorate.config_body_class).to eq("default default-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} #{user.config_navbar}-navbar-config") - end - - it "works with static navbar" do - user.config_navbar = "static" - expect(user.decorate.config_body_class).to eq("default default-article-body pro-status-#{user.pro?} trusted-status-#{user.trusted} static-navbar-config") - end - end - - it "does not allow to change to username that is taken" do - user.username = second_user.username - expect(user).not_to be_valid - end - - it "does not allow to change to username that is taken by an organization" do - user.username = create(:organization).slug - expect(user).not_to be_valid - end - - it "indexes into Algolia search" do - user.index! - end - - it "calculates score" do - article.featured = true - article.save - user.calculate_score - expect(user.score).to be > 0 - end - - it "persists JSON dump of identity data" do - new_user = user_from_authorization_service(:twitter, nil, "navbar_basic") - identity = new_user.identities.first - expect(identity.auth_data_dump.provider).to eq(identity.provider) - end - - it "persists extracts relevant identity data from logged in user" do - new_user = user_from_authorization_service(:twitter, nil, "navbar_basic") - expect(new_user.twitter_following_count).to be_an(Integer) - expect(new_user.twitter_followers_count).to eq(100) - expect(new_user.twitter_created_at).to be_kind_of(ActiveSupport::TimeWithZone) - new_user = user_from_authorization_service(:github, nil, "navbar_basic") - expect(new_user.github_created_at).to be_kind_of(ActiveSupport::TimeWithZone) end describe "cache counts" do it "has an accurate tag follow count" do + tag = create(:tag) user.follow(tag) - expect(user.reload.following_tags_count).to eq 1 + expect(user.reload.following_tags_count).to eq(1) end it "has an accurate user follow count" do - user.follow(second_user) - expect(user.reload.following_users_count).to eq 1 + user.follow(other_user) + expect(user.reload.following_users_count).to eq(1) end it "has an accurate organization follow count" do user.follow(org) - expect(user.reload.following_orgs_count).to eq 1 + expect(user.reload.following_orgs_count).to eq(1) end end - describe "organization admin privileges" do + describe "#org_admin?" do it "recognizes an org admin" do create(:organization_membership, user: user, organization: org, type_of_user: "admin") - expect(user.org_admin?(org)).to be true + expect(user.org_admin?(org)).to be(true) end it "forbids an incorrect org admin" do + other_org = create(:organization) create(:organization_membership, user: user, organization: org, type_of_user: "admin") - expect(user.org_admin?(second_org)).to be false - expect(second_user.org_admin?(org)).to be false + expect(user.org_admin?(other_org)).to be(false) + expect(other_user.org_admin?(org)).to be(false) end - it "responds to nil" do - expect(user.org_admin?(nil)).to be false - expect(second_user.org_admin?(nil)).to be false - end - end - - describe "#destroy" do - it "successfully destroys a user" do - user.destroy - expect(user.persisted?).to be false - end - - it "destroys associated organization memberships" do - organization_membership = create(:organization_membership, user_id: user.id, organization_id: org.id) - user.destroy - expect { organization_membership.reload }.to raise_error ActiveRecord::RecordNotFound + it "returns false if nil is given" do + expect(user.org_admin?(nil)).to be(false) end end describe "#pro?" do - let(:user) { create(:user) } - it "returns false if the user is not a pro" do expect(user.pro?).to be(false) end @@ -715,7 +700,7 @@ RSpec.describe User, type: :model do end it "returns true if the user has an active pro membership" do - create(:pro_membership, user: user) + user.pro_membership = build(:pro_membership, status: "active") expect(user.pro?).to be(true) end @@ -728,16 +713,6 @@ RSpec.describe User, type: :model do end end - describe "when agolia auto-indexing/removal is triggered" do - it "process background auto-indexing when user is saved" do - expect { user.save }.to have_enqueued_job.with(user, "index!").on_queue("algoliasearch") - end - - it "doesn't schedule a job on destroy" do - expect { user.destroy }.not_to have_enqueued_job.on_queue("algoliasearch") - end - end - describe "#has_enough_credits?" do it "returns false if the user has less unspent credits than neeed" do expect(user.has_enough_credits?(1)).to be(false) @@ -753,14 +728,4 @@ RSpec.describe User, type: :model do expect(user.has_enough_credits?(1)).to be(true) end end - - describe "#subscribe_to_mailchimp_newsletter" do - let(:user2) { create :user } - - it "schedules the job" do - expect do - user2.subscribe_to_mailchimp_newsletter - end.to have_enqueued_job(Users::SubscribeToMailchimpNewsletterJob).exactly(:once).with(user2.id) - end - end end diff --git a/spec/models/webhook/event_spec.rb b/spec/models/webhook/event_spec.rb index 9e4d05244..08215f14c 100644 --- a/spec/models/webhook/event_spec.rb +++ b/spec/models/webhook/event_spec.rb @@ -1,13 +1,15 @@ require "rails_helper" RSpec.describe Webhook::Event, type: :model do - let(:article) { create(:article) } - let!(:payload) { Webhook::PayloadAdapter.new(article).hash } + let_it_be(:article) { create(:article) } + let_it_be(:payload) { Webhook::PayloadAdapter.new(article).hash } - it "raises an exception with a unknown event type" do - expect do - described_class.new(event_type: "cool_event") - end.to raise_error(Webhook::InvalidEvent) + describe "validations" do + it "raises an exception with a unknown event type" do + expect do + described_class.new(event_type: "cool_event") + end.to raise_error(Webhook::InvalidEvent) + end end describe "#as_json" do