Skip to content

Commit

Permalink
Avoid confusion with class methods and protected/private modifiers (#…
Browse files Browse the repository at this point in the history
…1512)

Working on something else, I saw the following:

```ruby
  protected

  # ...

  def self.field_type
    to_s.split("::").last.underscore
  end
```

This code can lead to misunderstanding, as the `protected` modifier doesn't
apply to class methods like `self.field_type`.

On this PR, I move two such class methods behind the modifier, near the top of
the class, to avoid this confusion.

I also move three other class methods to the top of the class, as I feel class
methods should go together in general, in order to clarify interfaces and avoid
future instances of this anti-pattern.
  • Loading branch information
pablobm committed Feb 13, 2020
1 parent 650a2d6 commit 35f72ad
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 23 deletions.
18 changes: 8 additions & 10 deletions lib/administrate/field/base.rb
Expand Up @@ -16,6 +16,14 @@ def self.searchable?
false
end

def self.field_type
to_s.split("::").last.underscore
end

def self.permitted_attribute(attr, _options = nil)
attr
end

def initialize(attribute, data, page, options = {})
@attribute = attribute
@data = data
Expand All @@ -24,10 +32,6 @@ def initialize(attribute, data, page, options = {})
@options = options
end

def self.permitted_attribute(attr, _options = nil)
attr
end

def html_class
self.class.html_class
end
Expand All @@ -41,12 +45,6 @@ def to_partial_path
end

attr_reader :attribute, :data, :options, :page, :resource

protected

def self.field_type
to_s.split("::").last.underscore
end
end
end
end
14 changes: 7 additions & 7 deletions lib/administrate/field/has_one.rb
Expand Up @@ -3,13 +3,6 @@
module Administrate
module Field
class HasOne < Associative
def nested_form
@nested_form ||= Administrate::Page::Form.new(
resolver.dashboard_class.new,
data || resolver.resource_class.new,
)
end

def self.permitted_attribute(attr, options = nil)
associated_class_name =
if options
Expand All @@ -24,6 +17,13 @@ def self.permitted_attribute(attr, options = nil)
{ "#{attr}_attributes": related_dashboard_attributes }
end

def nested_form
@nested_form ||= Administrate::Page::Form.new(
resolver.dashboard_class.new,
data || resolver.resource_class.new,
)
end

private

def resolver
Expand Down
8 changes: 4 additions & 4 deletions lib/administrate/field/polymorphic.rb
Expand Up @@ -3,6 +3,10 @@
module Administrate
module Field
class Polymorphic < BelongsTo
def self.permitted_attribute(attr, _options = nil)
{ attr => %i{type value} }
end

def associated_resource_grouped_options
classes.map do |klass|
[klass.to_s, candidate_resources_for(klass).map do |resource|
Expand All @@ -11,10 +15,6 @@ def associated_resource_grouped_options
end
end

def self.permitted_attribute(attr, _options = nil)
{ attr => %i{type value} }
end

def permitted_attribute
{ attribute => %i{type value} }
end
Expand Down
4 changes: 2 additions & 2 deletions lib/administrate/view_generator.rb
Expand Up @@ -5,15 +5,15 @@ module Administrate
class ViewGenerator < Rails::Generators::Base
include Administrate::GeneratorHelpers

private

def self.template_source_path
File.expand_path(
"../../../app/views/administrate/application",
__FILE__,
)
end

private

def copy_resource_template(template_name)
template_file = "#{template_name}.html.erb"

Expand Down

0 comments on commit 35f72ad

Please sign in to comment.