Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions app/models/article.rb
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,6 @@ def body_preview
processed_html_final
end


def readable_publish_date
relevant_date = displayable_published_at
return unless relevant_date
Expand Down Expand Up @@ -1338,7 +1337,18 @@ def validate_video
I18n.t("models.article.video_processing"))
end

return unless video.present? && user.created_at > 2.weeks.ago
return unless user
return if user.created_at && user.created_at <= 2.weeks.ago

return unless video.present?

# Allow linked videos (YouTube, Mux, Twitch) even for new users
return if video_source_url.present? && (
video_source_url.include?("youtube.com") ||
video_source_url.include?("youtu.be") ||
video_source_url.include?("player.mux.com") ||
video_source_url.include?("twitch.tv")
)

errors.add(:video, I18n.t("models.article.video_unpermitted"))
end
Expand Down
60 changes: 29 additions & 31 deletions app/services/scheduled_automations/first_post_badge_awarder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ def initialize(automation)
@automation = automation
@organization_id = automation.action_config["organization_id"]&.to_i
@badge_slug = automation.action_config["badge_slug"]
@since_time = automation.last_run_at || Time.at(0) # Use epoch if first run

# Look back 15 minutes to overlapping intervals to prevent misses
last_run = automation.last_run_at || Time.at(0)
@since_time = last_run == Time.at(0) ? last_run : last_run - 15.minutes
end

def call
Expand All @@ -34,7 +37,7 @@ def call
return Result.new(
success?: false,
users_awarded: 0,
error_message: "Badge with slug '#{@badge_slug}' not found"
error_message: "Badge with slug '#{@badge_slug}' not found",
)
end

Expand All @@ -43,7 +46,7 @@ def call
return Result.new(
success?: false,
users_awarded: 0,
error_message: "Organization with id '#{@organization_id}' not found"
error_message: "Organization with id '#{@organization_id}' not found",
)
end

Expand All @@ -52,13 +55,13 @@ def call
Result.new(
success?: true,
users_awarded: users_awarded,
error_message: nil
error_message: nil,
)
rescue StandardError => e
Result.new(
success?: false,
users_awarded: 0,
error_message: "#{e.class}: #{e.message}"
error_message: "#{e.class}: #{e.message}",
)
end

Expand All @@ -69,27 +72,23 @@ def validate_config!
raise ArgumentError, "organization_id is required in action_config"
end

if @badge_slug.blank?
raise ArgumentError, "badge_slug is required in action_config"
end
return if @badge_slug.present?

raise ArgumentError, "badge_slug is required in action_config"
end

def award_badges_to_first_post_authors(organization, badge_id)
# Find all published articles under this organization since last run
recent_articles = Article.published
.where(organization_id: organization.id)
.where("published_at > ?", @since_time)
.includes(:user)
.order(published_at: :asc)
.where(organization_id: organization.id)
.where("published_at > ?", @since_time)
.includes(:user)
.order(published_at: :asc)

return 0 if recent_articles.empty?

users_awarded = 0

# Look up badge once to check allow_multiple_awards
badge = Badge.find_by(id: badge_id)
allow_multiple = badge&.allow_multiple_awards || false

# Group articles by user to find their first post under this org
recent_articles.group_by(&:user_id).each do |user_id, articles|
user = articles.first.user
Expand All @@ -100,29 +99,29 @@ def award_badges_to_first_post_authors(organization, badge_id)

# Check if this is the user's first published article under this organization
# by checking if they have any earlier published articles under this org (ever)
# Note: We check < earliest_recent_article.published_at, so if they have an earlier one that was
# picked up in a previous run (or missed), this will return true and we skip.
has_earlier_post = Article.published
.where(organization_id: organization.id, user_id: user_id)
.where("published_at < ?", earliest_recent_article.published_at)
.exists?
.where(organization_id: organization.id, user_id: user_id)
.where("published_at < ?", earliest_recent_article.published_at)
.exists?

# Only award badge if this is their first post under this org AND it's in the recent set
next if has_earlier_post

# Check if user already has this badge (to avoid duplicates)
# Only check if badge doesn't allow multiple awards
unless allow_multiple
existing_achievement = BadgeAchievement.find_by(
user_id: user_id,
badge_id: badge_id
)
next if existing_achievement
end
# We always check this regardless of badge settings to ensure idempotency of this automation
existing_achievement = BadgeAchievement.find_by(
user_id: user_id,
badge_id: badge_id,
)
next if existing_achievement

# Award the badge
achievement = BadgeAchievement.create(
user_id: user_id,
badge_id: badge_id,
rewarding_context_message_markdown: generate_message(organization, earliest_recent_article)
rewarding_context_message_markdown: generate_message(organization, earliest_recent_article),
)

if achievement.persisted?
Expand All @@ -137,10 +136,9 @@ def award_badges_to_first_post_authors(organization, badge_id)
def generate_message(organization, article)
org_url = URL.organization(organization)
article_url = URL.article(article)

"Congratulations on posting your first article under [#{organization.name}](#{org_url})! " \
"Your post was [#{article.title}](#{article_url})."
"Your post was [#{article.title}](#{article_url})."
end
end
end

68 changes: 56 additions & 12 deletions spec/models/article_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,16 @@ def build_and_validate_article(*args)
end

it "prevents org member from adding article to org collection when not publishing under org" do
article = build(:article, user: org_member, organization: nil, body_markdown: "---\npublished: true\ntags: test\n---\nContent")
article = build(:article, user: org_member, organization: nil,
body_markdown: "---\npublished: true\ntags: test\n---\nContent")
article.collection_id = org_collection.id
expect(article).not_to be_valid
expect(article.errors[:collection_id]).to include(I18n.t("models.article.series_unpermitted"))
end

it "prevents non-org member from adding article to org collection" do
article = build(:article, user: user, organization: organization, body_markdown: "---\npublished: true\ntags: test\n---\nContent")
article = build(:article, user: user, organization: organization,
body_markdown: "---\npublished: true\ntags: test\n---\nContent")
article.collection_id = org_collection.id
expect(article).not_to be_valid
expect(article.errors[:collection_id]).to include(I18n.t("models.article.series_unpermitted"))
Expand All @@ -116,7 +118,8 @@ def build_and_validate_article(*args)

it "prevents org member from adding article to org collection with different org_id" do
other_org = create(:organization)
article = build(:article, user: org_member, organization: other_org, body_markdown: "---\npublished: true\ntags: test\n---\nContent")
article = build(:article, user: org_member, organization: other_org,
body_markdown: "---\npublished: true\ntags: test\n---\nContent")
article.collection_id = org_collection.id
expect(article).not_to be_valid
expect(article.errors[:collection_id]).to include(I18n.t("models.article.series_unpermitted"))
Expand All @@ -125,14 +128,54 @@ def build_and_validate_article(*args)
it "prevents org member from adding article to org collection when org_id doesn't match" do
other_org = create(:organization)
create(:organization_membership, user: org_member, organization: other_org, type_of_user: "member")
article = build(:article, user: org_member, organization: other_org, body_markdown: "---\npublished: true\ntags: test\n---\nContent")
article = build(:article, user: org_member, organization: other_org,
body_markdown: "---\npublished: true\ntags: test\n---\nContent")
article.collection_id = org_collection.id
expect(article).not_to be_valid
expect(article.errors[:collection_id]).to include(I18n.t("models.article.series_unpermitted"))
end
end
end

describe "#validate_video" do
let(:new_user) { create(:user, created_at: 1.week.ago) }
let(:old_user) { create(:user, created_at: 3.weeks.ago) }

context "when user is new (less than 2 weeks old)" do
it "does not allow direct uploads (video present but no allowed source url)" do
# Simulating a direct upload where video is set but source URL isn't a whitelisted one
# We use a valid URL for 'video' to pass the format validation, focusing on the permission check
article = build(:article, user: new_user, video: "https://example.com/video.mp4", video_source_url: "https://unknown-source.com/video.mp4")

expect(article).not_to be_valid
expect(article.errors[:video]).to include(I18n.t("models.article.video_unpermitted"))
end

it "allows YouTube videos" do
article = build(:article, user: new_user, video: "https://youtube.com/video", video_source_url: "https://www.youtube.com/watch?v=dQw4w9WgXcQ")
expect(article).to be_valid
end

it "allows Mux videos" do
# Use player.mux.com to match the whitelist logic
article = build(:article, user: new_user, video: "https://player.mux.com/video", video_source_url: "https://player.mux.com/123.m3u8")
expect(article).to be_valid
end

it "allows Twitch videos" do
article = build(:article, user: new_user, video: "https://twitch.tv/video", video_source_url: "https://www.twitch.tv/videos/123")
expect(article).to be_valid
end
end

context "when user is old (more than 2 weeks old)" do
it "allows direct uploads" do
article = build(:article, user: old_user, video: "https://example.com/video.mp4", video_source_url: "https://unknown-source.com/video.mp4")
expect(article).to be_valid
end
end
end

describe "#all_series" do
let(:org_member) { create(:user) }
let(:organization) { create(:organization) }
Expand All @@ -154,7 +197,8 @@ def build_and_validate_article(*args)
org_collection = create(:collection, user: org_member, organization: organization, slug: "org-series")
article = build(:article, user: org_member)
series = article.all_series
expect(series).to include(hash_including(slug: "org-series", organization_id: organization.id, is_personal: false))
expect(series).to include(hash_including(slug: "org-series", organization_id: organization.id,
is_personal: false))
end

it "does not return collections from orgs the user is not a member of" do
Expand Down Expand Up @@ -1471,7 +1515,7 @@ def build_and_validate_article(*args)
it "parses Twitch URL and sets video embed URL" do
article.video_source_url = "https://www.twitch.tv/videos/1234567890"
article.valid?
expect(article.video).to match(/https:\/\/player\.twitch\.tv\/\?video=1234567890/)
expect(article.video).to match(%r{https://player\.twitch\.tv/\?video=1234567890})
expect(article.video).to include("autoplay=false")
end
end
Expand Down Expand Up @@ -2583,20 +2627,20 @@ def foo():

it "uses the correct cache key for reaction counts" do
cache_key = "reaction_counts_for_reactable-Article-#{article.id}"

# Clear any existing cache
Rails.cache.delete(cache_key)

# Ensure we have reactions (from before block)
expect(article.reactions.count).to be > 0

# Call the method to populate cache
result = article.public_reaction_categories

# Verify we got results
expect(result).to be_present
expect(result).to be_an(Array)

# The cache should be populated after calling the method
# Note: The cache might not be populated if there are no reactions or if the cache is disabled
if Rails.cache.exist?(cache_key)
Expand Down Expand Up @@ -2937,7 +2981,7 @@ def foo():
stub_request(:head, %r{https?://first\.com}).to_return(status: 200)
stub_request(:head, %r{https?://second\.org}).to_return(status: 200)
stub_request(:head, %r{https?://third\.net}).to_return(status: 200)

# Stub GET requests for metadata fetching (used by OpenGraph)
stub_request(:get, %r{https?://example\.com}).to_return(status: 200, body: "<html></html>")
stub_request(:get, %r{https?://another-example\.org}).to_return(status: 200, body: "<html></html>")
Expand Down
Loading
Loading