Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement slug field partial #764

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ def decode_id(id)
end

def find(*ids)
super(*ids.map { |id| decode_id(id) })
if respond_to?(:slug_attribute)
send("find_by_#{slug_attribute}".to_sym, ids.first)
else
super(*ids.map { |id| decode_id(id) })
end
end

def relation
Expand All @@ -46,6 +50,7 @@ def obfuscated_id
end

def to_param
obfuscated_id
# Even if the `slug` column exists, it might be empty (i.e. - Team).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(defined?(slug) && slug.present?) ? slug : obfuscated_id
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
module ObfuscatesId
module Sluggable
extend ActiveSupport::Concern

included do
before_validation :downcase_slug
after_validation :set_to_previous_value, on: :update

if respond_to?(:slug_attribute)
validates slug_attribute,
uniqueness: true,
length: {minimum: 2, maximum: 30},
format: {with: /\A[a-zA-Z0-9|-]+\Z/}
end

validates_with ::RestrictedPathsValidator

private

def downcase_slug
send(attribute_name_setter, send(attribute_name).downcase)
end

# Since Rails reloads the same invalid object into the form while displaying errors,
# we ensure the slug value being referenced to on reloading is the original value which
# has already been persisted to the database, not the invalid value from the form object.
def set_to_previous_value
if errors.any?
errors.each do |e|
if e.match?(attribute_name)
previous_slug_value = send("#{attribute_name}_was")
send(attribute_name_setter, previous_slug_value)
end
end
end
end

def attribute_name
self.class.send(:slug_attribute)
end

def attribute_name_setter
"#{attribute_name}=".to_sym
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class RestrictedPathsValidator < ActiveModel::Validator
def validate(record)
if record.class.restricted_paths.include?(record.slug)
record.errors.add record.class.slug_attribute, :restricted_path
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we may also need to validate that the slug doesn't match any of the controller actions for this model. For instance if someone tried to use the slug new for a team, then that would cause a collision for the path /teams/new. We'd always want that to point to new_team_path and not to team_path(slug: 'new').

This article suggests that something like TeamsController.action_methods would give us a good list to validate against.

For our Account::TeamsController the action_methods method returns this big ol' list:

Account::TeamsController.action_methods
=>
#<Set:
 {"team_params",
  "switch_to",
  "show",
  "index",
  "create",
  "edit",
  "destroy",
  "update",
  "new",
  "ensure_onboarding_is_complete",
  "assign_select_options",
  "create_model_if_new",
  "create_models_if_new",
  "ensure_backing_models_on",
  "assign_checkboxes",
  "assign_date",
  "assign_date_and_time",
  "assign_boolean",
  "load_team",
  "managing_account?",
  "ensure_onboarding_is_complete_and_set_next_step",
  "set_last_seen_at",
  "adding_team?",
  "accepting_invitation?",
  "adding_user_email?",
  "adding_user_details?",
  "switching_teams?",
  "set_current_user",
  "invited?",
  "show_sign_up_options?",
  "after_sign_up_path_for",
  "enforce_invitation_only",
  "only_allow_path",
  "delegate_json_to_api",
  "current_team",
  "current_membership",
  "current_locale",
  "set_locale",
  "layout_by_resource",
  "set_sentry_context"}
>

That seems like too much. This StackOverflow answer suggests a method like this:

def actions_for_controller(controller_path)
  route_defaults = Rails.application.routes.routes.map(&:defaults)
  route_defaults = route_defaults.select { |x| x[:controller] == controller_path }
  route_defaults.map { |x| x[:action] }.uniq
end

Which produces a much more reasonable list:

actions_for_controller('account/teams')
=> [
  "index",
  "create",
  "new",
  "edit",
  "show",
  "update",
  "destroy",
  "switch_to"
]

I'm not sure that we'd need to prevent member actions like edit and show from being used as a slug, though allowing them might result in weird URLs like /teams/show/edit (the edit action for the team with a slug of show).

Determining which controller to inspect for a given model may be difficult.

end
end
end
4 changes: 4 additions & 0 deletions bullet_train-obfuscates_id/config/locales/en/errors.en.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
en:
errors:
messages:
restricted_path: "is restricted, please try another path"
1 change: 1 addition & 0 deletions bullet_train-super_scaffolding/lib/scaffolding.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ def self.valid_attribute_type?(type)
"options",
"password_field",
"phone_field",
"slug",
"super_select",
"text_area",
"text_field",
Expand Down
3 changes: 3 additions & 0 deletions bullet_train-super_scaffolding/lib/scaffolding/attribute.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def collection_name
def partial_name
return options[:attribute] if options[:attribute]

# TODO: We should put these in alphabetical order.
case type
when "trix_editor", "ckeditor"
"html"
Expand Down Expand Up @@ -169,6 +170,8 @@ def partial_name
"number"
when "address_field"
"address"
when "slug"
"text"
else
raise "Invalid field type: #{type}."
end
Expand Down
1 change: 1 addition & 0 deletions bullet_train-super_scaffolding/lib/scaffolding/script.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
options: "string",
password_field: "string",
phone_field: "string",
slug: "string",
super_select: "string",
text_area: "text",
text_field: "string",
Expand Down
34 changes: 33 additions & 1 deletion bullet_train-super_scaffolding/lib/scaffolding/transformer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ def add_attributes_to_various_views(attributes, scaffolding_options = {})
# 'primary_key' => '',
# 'references' => '',
"string" => "text_field",
"text" => "text_area"
"text" => "text_area",
# 'time' => '',
# 'timestamp' => '',
}
Expand Down Expand Up @@ -1294,6 +1294,38 @@ def remove_#{attribute.name}
if attribute.is_boolean?
scaffold_add_line_to_file("./app/models/scaffolding/completely_concrete/tangible_thing.rb", "validates :#{attribute.name}, inclusion: [true, false]", VALIDATIONS_HOOK, prepend: true)
end
when "slug"
slug_methods = <<~RUBY
def slug
#{attribute.name}
end

def self.slug_attribute
:#{attribute.name}
end

# Define which paths you don't want to show up when defining slugs.
def self.restricted_paths
[
"admin",
"admins"
]
end
RUBY

scaffold_add_line_to_file(
"./app/models/scaffolding/completely_concrete/tangible_thing.rb",
"include Sluggable",
CONCERNS_HOOK,
prepend: true
)

scaffold_add_line_to_file(
"./app/models/scaffolding/completely_concrete/tangible_thing.rb",
slug_methods,
METHODS_HOOK,
prepend: true
)
end

end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<!-- This is the same content as text_field -->
<%
form ||= current_fields_form
options ||= {}
other_options ||= {}
%>

<%= render 'shared/fields/field', form: form, method: method, helper: :text_field, options: options, other_options: other_options %>
Loading