-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Display helpful message if attribute_name missing #251
Changes from 4 commits
62570b6
52567fd
da3b07c
c71fa38
772774f
58c6017
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,18 @@ def attribute_types | |
self.class::ATTRIBUTE_TYPES | ||
end | ||
|
||
def attribute_type_for(attribute_name) | ||
attribute_types.fetch(attribute_name) do | ||
fail "Attribute #{attribute_name} could not be found in #{self.class}::ATTRIBUTE_TYPES" | ||
end | ||
end | ||
|
||
def attribute_types_for(*attribute_names) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here for the splat operator. |
||
attribute_names.each_with_object({}) do |name, attributes| | ||
attributes[name] = attribute_type_for(name) | ||
end | ||
end | ||
|
||
def form_attributes | ||
self.class::FORM_ATTRIBUTES | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,7 @@ def attributes_for(resource) | |
end | ||
|
||
def attribute_types | ||
dashboard.attribute_types.slice(*attribute_names) | ||
dashboard.attribute_types_for(*attribute_names) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's get rid of the splat operator here ( |
||
end | ||
|
||
def ordered_html_class(attr) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,49 @@ | |
end | ||
end | ||
|
||
def missing_attribute_message(attribute, dashboard_class) | ||
"Attribute #{attribute} could not be found in #{dashboard_class}::ATTRIBUTE_TYPES" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line is too long. [86/80] There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line is too long. [86/80] There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm ok ignoring hound for this line. |
||
end | ||
|
||
describe "#attribute_type_for" do | ||
context "for an existing attribute" do | ||
it "returns the attribute field" do | ||
dashboard = CustomerDashboard.new | ||
field = dashboard.attribute_type_for(:name) | ||
expect(field).to eq Administrate::Field::String | ||
end | ||
end | ||
|
||
context "for a non-existent attribute" do | ||
it "raises an exception" do | ||
dashboard = CustomerDashboard.new | ||
expect { dashboard.attribute_type_for(:foo) }. | ||
to raise_error missing_attribute_message("foo", "CustomerDashboard") | ||
end | ||
end | ||
end | ||
|
||
describe "#attribute_types_for" do | ||
context "for existing attributes" do | ||
it "returns the attribute fields" do | ||
dashboard = CustomerDashboard.new | ||
fields = dashboard.attribute_types_for(:name, :email) | ||
expect(fields).to match( | ||
name: Administrate::Field::String, | ||
email: Administrate::Field::Email, | ||
) | ||
end | ||
end | ||
|
||
context "for one non-existent attribute" do | ||
it "raises an exception" do | ||
dashboard = CustomerDashboard.new | ||
expect { dashboard.attribute_types_for(:name, :foo) }. | ||
to raise_error missing_attribute_message("foo", "CustomerDashboard") | ||
end | ||
end | ||
end | ||
|
||
describe "#display_resource" do | ||
it "returns the customer's name" do | ||
customer = double(name: "Example Customer") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line is too long. [95/80]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can extract a method for the error message, similar to what you did in the tests. If the line's still too long after that, I'm ok ignoring it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will still be too long.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, sorry - I think it's a good idea to extract the message into its own method regardless. I meant that if hound comments again, we can ignore that.