Allow pages to have subdirectories (#20549)

* Allow pages to have subdirectories

* Add locale text
This commit is contained in:
Ben Halpern 2024-01-24 16:09:29 -05:00 committed by GitHub
parent 7149558f34
commit b763ba8544
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 87 additions and 3 deletions

View file

@ -3,6 +3,7 @@ class PagesController < ApplicationController
before_action :set_cache_control_headers, only: %i[show badge bounty faq robots]
def show
params[:slug] = combined_fragmented_slug if params[:slug_0].present?
@page = Page.find_by!(slug: params[:slug])
not_found_conditions
set_surrogate_key_header "show-page-#{params[:slug]}"
@ -136,4 +137,8 @@ class PagesController < ApplicationController
not_found unless FeatureFlag.accessible?(@page.feature_flag_name, current_user)
not_found if params[:format] == "txt" && @page.template != "txt"
end
def combined_fragmented_slug
(0..5).filter_map { |i| params["slug_#{i}"] }.join("/")
end
end

View file

@ -41,6 +41,8 @@ class StoriesController < ApplicationController
elsif (@podcast = Podcast.available.find_by(slug: params[:username]))
@episode = @podcast.podcast_episodes.available.find_by!(slug: params[:slug])
handle_podcast_show
elsif (@page = Page.find_by(slug: "#{params[:username]}/#{params[:slug]}", is_top_level_path: true))
handle_page_display
else
not_found
end

View file

@ -1,13 +1,15 @@
##
# Validates if the give attribute is used across the reserved spaces.
class CrossModelSlugValidator < ActiveModel::EachValidator
FORMAT_REGEX = /\A[0-9a-z\-_]+\z/
ORGANIZATION_FORMAT_REGEX = /\A(?![0-9]+\z)[0-9a-z\-_]+\z/
## allow / in page slugs
PAGE_FORMAT_REGEX = /\A[0-9a-z\-_\/]+\z/
PAGE_DIRECTORY_LIMIT = 6
def validate_each(record, attribute, value)
return if value.blank?
correct_format?(record, attribute, value)
allowed_subdirectory_count?(record, attribute, value)
not_on_reserved_list?(record, attribute, value)
unique_across_models?(record, attribute, value)
end
@ -21,7 +23,14 @@ class CrossModelSlugValidator < ActiveModel::EachValidator
end
def correct_format?(record, attribute, value)
format_regex = record.is_a?(Organization) ? ORGANIZATION_FORMAT_REGEX : FORMAT_REGEX
format_regex = case record.class.name
when "Organization"
ORGANIZATION_FORMAT_REGEX
when "Page"
PAGE_FORMAT_REGEX
else
FORMAT_REGEX
end
return false if value.match?(format_regex)
record.errors.add(attribute, I18n.t("validators.cross_model_slug_validator.is_invalid"))
@ -35,6 +44,13 @@ class CrossModelSlugValidator < ActiveModel::EachValidator
record.errors.add(attribute, I18n.t("validators.cross_model_slug_validator.is_taken"))
end
def allowed_subdirectory_count?(record, attribute, value)
return false unless record.instance_of?(::Page)
return false if value.split("/").count <= PAGE_DIRECTORY_LIMIT
record.errors.add(attribute, I18n.t("validators.cross_model_slug_validator.too_many_subdirectories"))
end
def already_exists?(value)
CrossModelSlug.exists?(value.downcase)
end

View file

@ -15,6 +15,7 @@ en:
is_taken: has already been taken
is_invalid: is invalid
is_reserved: is reserved
too_many_subdirectories: has too many subdirectories
valid_domain_csv_validator:
invalid_list_format: must be a comma-separated list of valid domains
iso3166_hash_validator:

View file

@ -15,6 +15,7 @@ fr:
is_taken: est déjà pris
is_invalid: est invalide
is_reserved: est réservé
too_many_subdirectories: a trop de sous-répertoires
valid_domain_csv_validator:
invalid_list_format: doit être une liste de domaines valides, séparés par des virgules.
iso3166_hash_validator:

View file

@ -398,6 +398,11 @@ Rails.application.routes.draw do
constraints: { format: /html/ }
get "/:slug", to: "pages#show",
constraints: { format: /txt/ }
get "/:slug_0/:slug_1", to: "pages#show", as: :page_0_1
get "/:slug_0/:slug_1/:slug_2", to: "pages#show", as: :page_0_1_2
get "/:slug_0/:slug_1/:slug_2/:slug_3", to: "pages#show", as: :page_0_1_2_3
get "/:slug_0/:slug_1/:slug_2/:slug_3/:slug_4", to: "pages#show", as: :page_0_1_2_3_4
get "/:slug_0/:slug_1/:slug_2/:slug_3/:slug_4/:slug_5", to: "pages#show", as: :page_0_1_2_3_4_5
root "stories#index"
end
end

View file

@ -72,6 +72,17 @@ RSpec.describe Page do
page = build(:page, slug: "code-of-conduct")
expect(page).to be_valid
end
it "allows / in slug" do
page = build(:page, slug: "heyhey/hey")
expect(page).to be_valid
end
it "disallows 6+ directories via /" do
page = build(:page, slug: "heyhey/hey/hey/hey/hey/hey/hey")
expect(page).not_to be_valid
expect(page.errors[:slug].to_s.include?("subdirectories")).to be true
end
end
context "when callbacks are triggered before save" do

View file

@ -59,6 +59,49 @@ RSpec.describe "Pages" do
end
end
describe "GET /slug/slug/slug/etc." do
it "renders proper page when slug has one subdirectory" do
page = create(:page, slug: "first-slug/second-slug", is_top_level_path: true)
get "/#{page.slug}"
expect(response.body).to include(CGI.escapeHTML(page.title))
end
it "renders proper page when slug has two subdirectories" do
page = create(:page, slug: "first-slug/second-slug/third-slug", is_top_level_path: true)
get "/#{page.slug}"
expect(response.body).to include(CGI.escapeHTML(page.title))
end
it "renders proper page when slug has three subdirectories" do
page = create(:page, slug: "first-slug/second-slug/third-slug/fourth-slug", is_top_level_path: true)
get "/#{page.slug}"
expect(response.body).to include(CGI.escapeHTML(page.title))
end
it "renders proper page when slug has four subdirectories" do
page = create(:page, slug: "first-slug/second-slug/third-slug/fourth-slug/fifth-slug", is_top_level_path: true)
get "/#{page.slug}"
expect(response.body).to include(CGI.escapeHTML(page.title))
end
it "renders proper page when slug has five subdirectories" do
page = create(:page, slug: "first-slug/second-slug/third-slug/fourth-slug/fifth-slug/sixth-slug",
is_top_level_path: true)
get "/#{page.slug}"
expect(response.body).to include(CGI.escapeHTML(page.title))
end
it "returns not found when five directories, but no page" do
# 6+ directories will be a non-valid page, so just further testing routing error
expect { get "/heyhey/hey/hey/hey/hey" }.to raise_error(ActiveRecord::RecordNotFound)
end
it "returns routing error when 7+ directories" do
# 6+ directories will be a non-valid page, so just further testing routing error
expect { get "/heyhey/hey/hey/hey/hey/hey/hey" }.to raise_error(ActionController::RoutingError)
end
end
describe "GET /about" do
it "has proper headline" do
get "/about"