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.

Sign up for GitHub

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

Jump to bottom

Better error message if key is not specified in ATTRIBUTE_TYPES #1762

Merged
merged 2 commits into from
Feb 26, 2021
Merged

Better error message if key is not specified in ATTRIBUTE_TYPES #1762

merged 2 commits into from
Feb 26, 2021

Conversation

inem
Copy link
Contributor

@inem inem commented Sep 13, 2020 •
edited
Loading

I was getting:

NoMethodError: undefined method `associative?' for nil:NilClass

After the change:

KeyError: key not found: :created_at

@inem inem changed the title [key] → fetch(key) Better error message if key is not specified in ATTRIBUTE_TYPES Sep 13, 2020
@inem
Copy link
Contributor Author

inem commented Sep 16, 2020

@nickcharlton @pablobm @sedubois guys, can someone take a look please? It is a small change but I find it very useful - helps you debug thing when you messed up dashboard by not declaring some attributes.

@harley
Copy link

harley commented Sep 22, 2020

i got so used to this error that didn't pay attention, but i wanted to say this is an elegant change @inem - hope it gets merged soon as we're seeing more activities on master again :)

pablobm
pablobm reviewed Sep 24, 2020
View reviewed changes
lib/administrate/base_dashboard.rb Outdated
@@ -88,7 +88,7 @@ def attribute_not_found_message(attr)

def attribute_includes(attributes)
attributes.map do |key|
field = self.class::ATTRIBUTE_TYPES[key]
field = self.class::ATTRIBUTE_TYPES.fetch(key)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good call; thank you. Since we are at it, how about using attribute_type_for instead? That way we also get a more descriptive error than just a KeyError.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I didn't know what is attribute_type_for

Now googled and found that it is an internal administrate method.

Kind of done, I guess.

@nickcharlton nickcharlton added bug breakages in functionality that is implemented dashboards how administrate presents fields and displays data labels Oct 6, 2020
@caljess599
Copy link
Contributor

+1 for this change.

@pablobm
Copy link
Collaborator

pablobm commented Feb 4, 2021

Looks good! Could you please rebase so that the build passes? The errors should just be about security advisories in dependencies, but it's worth seeing it green before merging.

inem added 2 commits February 26, 2021 16:24
@inem @nickcharlton
[key] → fetch(key)
e715984 
to provide better error description if something goes wrong. Currently I get "NoMethodError: undefined method `associative?' for nil:NilClass"
@inem @nickcharlton
using attribute_type_for(key)
0e53a06
@nickcharlton nickcharlton merged commit 1b68b75 into thoughtbot:master Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pablobm pablobm pablobm approved these changes

Assignees
No one assigned
Labels
bug breakages in functionality that is implemented dashboards how administrate presents fields and displays data
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@inem @harley @caljess599 @pablobm @nickcharlton