diff --git a/app/models/profile.rb b/app/models/profile.rb index 7801fe838..c42a1cfb9 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -3,6 +3,7 @@ class Profile < ApplicationRecord validates :user_id, uniqueness: true validates :location, :website_url, length: { maximum: 100 } + validates :website_url, url: { allow_blank: true, no_local: true, schemes: %w[https http] } validates_with ProfileValidator # Static fields are columns on the profiles table; they have no relationship diff --git a/app/views/users/_profile.html.erb b/app/views/users/_profile.html.erb index dc1a690ac..03f538860 100644 --- a/app/views/users/_profile.html.erb +++ b/app/views/users/_profile.html.erb @@ -50,7 +50,7 @@ - <%= f.text_field "profile[website_url]", + <%= f.url_field "profile[website_url]", value: profile.website_url, placeholder: "https://yoursite.com", class: "crayons-textfield js-color-field" %> diff --git a/lib/data_update_scripts/20210722135549_profile_website_url_format.rb b/lib/data_update_scripts/20210722135549_profile_website_url_format.rb new file mode 100644 index 000000000..a6d9e5c9f --- /dev/null +++ b/lib/data_update_scripts/20210722135549_profile_website_url_format.rb @@ -0,0 +1,29 @@ +module DataUpdateScripts + class ProfileWebsiteUrlFormat + def run + profiles_to_fix.each do |profile| + fix_or_clear_website_url(profile) + profile.save || log_failure(profile) + end + end + + def fix_or_clear_website_url(profile) + profile.website_url = + begin + new_url = "https://#{profile.website_url}" + uri = URI.parse(new_url) + uri.scheme.present? && uri.host.present? && new_url || "" + end + end + + def log_failure(profile) + Rails.logger.warn("Attempted to update website_url for profile #{profile.id} but failed with #{profile.errors}") + end + + def profiles_to_fix + Profile + .where.not("website_url like 'http__/%'") + .where.not(website_url: [nil, ""]) + end + end +end diff --git a/spec/lib/data_update_scripts/profile_website_url_format_spec.rb b/spec/lib/data_update_scripts/profile_website_url_format_spec.rb new file mode 100644 index 000000000..1238de366 --- /dev/null +++ b/spec/lib/data_update_scripts/profile_website_url_format_spec.rb @@ -0,0 +1,48 @@ +require "rails_helper" +require Rails.root.join( + "lib/data_update_scripts/20210722135549_profile_website_url_format.rb", +) + +describe DataUpdateScripts::ProfileWebsiteUrlFormat do + let(:nil_profile) { create(:profile, website_url: nil) } + let(:empty_profile) { create(:profile, website_url: "") } + let(:valid_profile) { create(:profile, website_url: "https://www.example.com") } + + let(:invalid_profile) do + build(:profile, website_url: "www.example.com").tap do |profile| + profile.save(validate: false) + end + end + + let(:unfixable_profile) do + build(:profile, website_url: "/local.html").tap do |profile| + profile.save(validate: false) + end + end + + it "does not modify profiles where website url is null" do + expect { described_class.new.run }.not_to change(nil_profile, :website_url) + end + + it "does not modify profiles where website url is empty" do + expect { described_class.new.run }.not_to change(empty_profile, :website_url) + end + + it "does not modify profiles where website url is valid" do + expect { described_class.new.run }.not_to change(valid_profile, :website_url) + end + + it "prepends https:// to invalid urls to make a valid url from hostnames" do + expect { described_class.new.run } + .to change { invalid_profile.reload.website_url } + .from("www.example.com") + .to("https://www.example.com") + end + + it "clears websites that don't form valid urls by prepending" do + expect { described_class.new.run } + .to change { unfixable_profile.reload.website_url } + .from("/local.html") + .to("") + end +end diff --git a/spec/models/profile_spec.rb b/spec/models/profile_spec.rb index d05a1a215..503509b98 100644 --- a/spec/models/profile_spec.rb +++ b/spec/models/profile_spec.rb @@ -57,6 +57,23 @@ RSpec.describe Profile, type: :model do expect(profile.errors_as_sentence).to eq "Location is too long (maximum is 100 characters)" end end + + describe "validating website_url" do + it "is valid if blank" do + profile.website_url = nil + expect(profile).to be_valid + end + it "is valid with a complete url" do + profile.website_url = "https://dev.to" + expect(profile).to be_valid + end + + it "is invalid with an incomplete url" do + profile.website_url = "dev.to" + expect(profile).not_to be_valid + expect(profile.errors_as_sentence).to eq "Website url is not a valid URL" + end + end end context "when accessing profile fields" do