Ensure Article's username method returns a downcased string (#11813)

* Adds a lower() query to the DB in Stories#show

* Restructure spec

* bypass article callbacks in order to test edge case

* Fixes spec

* Change the approach to unsure downcase path in Article model

* Move .downcase call to set_caches before_save callback

* Adds data update script for fixing offending Article paths

* Moves model tests to correct block

* Apply suggestions from code review

Co-authored-by: Vaidehi Joshi <vaidehi.sj@gmail.com>

Co-authored-by: Vaidehi Joshi <vaidehi.sj@gmail.com>
This commit is contained in:
Fernando Valverde 2020-12-09 12:56:40 -06:00 committed by GitHub
parent b00ebc2778
commit c2982ef0fc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 53 additions and 1 deletions

View file

@ -409,7 +409,7 @@ class Article < ApplicationRecord
self.cached_user_name = user_name
self.cached_user_username = user_username
self.path = calculated_path
self.path = calculated_path.downcase
end
def evaluate_markdown

View file

@ -0,0 +1,13 @@
module DataUpdateScripts
class TouchArticlesWithCapitalizedPaths
def run
offending_articles = Article.where("path != lower(path)")
# Some log visibility for troubleshooting/monitoring
Rails.logger.info("Updating #{offending_articles.count} articles with capitalized paths...")
# touch won't trigger the callbacks so a manual update will do the trick
offending_articles.update(updated_at: Time.current)
end
end
end

View file

@ -0,0 +1,25 @@
require "rails_helper"
require Rails.root.join(
"lib/data_update_scripts/20201209134953_touch_articles_with_capitalized_paths.rb",
)
describe DataUpdateScripts::TouchArticlesWithCapitalizedPaths do
it "updates articles with a capitalized path" do
# Populate at least 5 articles
create_list(:article, 5)
# Create 1 article with a capitalized path
bad_article = create(:article, published: true)
new_path = bad_article.path.titleize.split(" ").join("-")
bad_article.update_columns(path: new_path) # bypass callback fix
# Check to make sure the query only catches the article with a bad path
expect(Article.where("path != lower(path)").count).to eq(1)
# Run the script
described_class.new.run
# Make sure we no longer have any Articles with bad paths
expect(Article.where("path != lower(path)").count).to eq(0)
end
end

View file

@ -745,6 +745,20 @@ RSpec.describe Article, type: :model do
end
context "when callbacks are triggered after save" do
describe "article path sanitizing" do
it "returns a downcased username when user has uppercase characters" do
upcased_user = create(:user, username: "UpcasedUserName")
upcased_article = create(:article, user: upcased_user)
expect(upcased_article.path).not_to match(/[AZ]+/)
end
it "returns a downcased username when an org slug has uppercase characters" do
upcased_org = create(:organization, slug: "UpcasedSlug")
upcased_article = create(:article, organization: upcased_org)
expect(upcased_article.path).not_to match(/[AZ]+/)
end
end
describe "main image background color" do
let(:article) { build(:article, user: user) }