Skip to content

Commit f335cb5

Browse files
authored
Conventional location and name for custom field (#2842)
There is an inconsistency between the `administrate:field` generator and the custom field `HasManyVariant` present in the demo app. They use different conventions for naming and file paths. After thinking about which of the two styles is right or wrong, I think the generator is doing the right thing. This PR moves `HasManyVariant` and renames it `HasManyVariantField`, in order to follow the convention set out by the generator (which is also the one shown in the docs). Notes: - The field class is generated under `app/fields`. The location means it's autoloaded and we don't need a `require`. - The `Field` suffix may seem annoying, but it's not different from controllers, helpers, etc, and removes the need for a namespace (eg: `Field::HasManyVariant`). - This establishes a difference between the official field types (eg: `Administrate::Field::String`) and custom ones (eg: `MyStringField`), but I think this is fine. Tangentially I also noticed that, if for any reason a user puts the template files in the wrong place, this is difficult to debug as Administrate will just fail silently, rendering empty templates. This is due to the changes introduced at #2467. This should not be an issue if using the generator, but might be if creating the field manually.
1 parent 14e042a commit f335cb5

File tree

4 files changed

+5
-13
lines changed

4 files changed

+5
-13
lines changed

spec/example_app/app/dashboards/customer_dashboard.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
require "administrate/field/has_many_variant"
21
require "administrate/base_dashboard"
32

43
class CustomerDashboard < Administrate::BaseDashboard
@@ -9,7 +8,7 @@ class CustomerDashboard < Administrate::BaseDashboard
98
lifetime_value: Field::Number.with_options(prefix: "$", decimals: 2, sortable: false),
109
name: Field::String,
1110
orders: Field::HasMany.with_options(limit: 2, sort_by: :id),
12-
log_entries: Field::HasManyVariant.with_options(limit: 2, sort_by: :id),
11+
log_entries: HasManyVariantField.with_options(limit: 2, sort_by: :id),
1312
updated_at: Field::DateTime,
1413
kind: Field::Select,
1514
territory: Field::BelongsTo.with_options(
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
class HasManyVariantField < Administrate::Field::HasMany
2+
# Only here to test that this works out of the box
3+
end

spec/example_app/lib/administrate/field/has_many_variant.rb

Lines changed: 0 additions & 9 deletions
This file was deleted.

spec/helpers/administrate/application_helper_spec.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
require "rails_helper"
22
require "administrate/field/has_many"
3-
require "administrate/field/has_many_variant"
43

54
RSpec.describe Administrate::ApplicationHelper do
65
describe "#find_partial_prefix" do
@@ -13,7 +12,7 @@
1312

1413
context "when the field does not have a partial and the superclass does" do
1514
it "returns the superclass prefix" do
16-
field = Administrate::Field::HasManyVariant.new(:name, "hello", :show)
15+
field = HasManyVariantField.new(:name, "hello", :show)
1716
expect(find_partial_prefix(field)).to eq("fields/has_many")
1817
end
1918
end

0 commit comments

Comments
 (0)