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

Consistent custom dashboards #2184

Closed
wants to merge 4 commits into from

Conversation

pablobm
Copy link
Collaborator

@pablobm pablobm commented Apr 28, 2022

Working on #1941, I noticed that model-less dashboards (also known as "custom dashboards") do not behave very consistently. Eventually I found myself having to fix this before I could work on the former PR.

There are some twists and turns in the code that handles them:

  1. There doesn't seem to be a way to have the navigation show a singular name for the dashboard. So for instance in the example app we have "Stats", but I couldn't create one with the name "Home".
  2. When used with Punditize, it requires the policy to use a plural name (HomesPolicy) instead of singular like for any other resource.
  3. The example StatsDashboard declares resource "Stats", which is a weird mix of singular and plural (in the end it doesn't seem to matter which one you use here).
  4. Internally this complicates things when working with authorization.

This PR introduces consistency, not fixing point 1 but at least addressing the other points and allowing me to proceed with my work at #1941. Original I was going to bundle it as part of the larger PR, but I thought it deserved scrutiny enough to have its own one.

As for the issue with singular names, this looks like a larger problem that should be looked into with more time, and it's probably related to how we currently can't generate dashboards for singular resources.

end

def resource(resource_name)
define_singleton_method(:named_resource) { resource_name }
@named_resource = resource_name
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change is not strictly necessary, but I felt that the current code was needlessly complicated and could be simplified like this.

<br>
<p><b>Total Orders:</b> <%= @stats[:order_count] %></h1>
<p><b>Total Customers:</b> <%= @stats[:customer_count] %></p>
<p><b>Total Orders:</b> <%= @stats[:order_count] %></p>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed markup.


<div style="padding: 20px">
<div>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The example is copied straight from spec/example_app/app/views/admin/stats/index.html.erb. I have removed the padding because it's not relevant here.

@pablobm pablobm requested a review from nickcharlton April 28, 2022 16:31
@pablobm pablobm marked this pull request as draft May 5, 2022 16:43
@pablobm
Copy link
Collaborator Author

pablobm commented May 5, 2022

Bringing this back to draft as I have to rethink it.

@pablobm
Copy link
Collaborator Author

pablobm commented May 19, 2022

Looks like I don't need any of this in the end. Closing.

@pablobm pablobm closed this May 19, 2022
@pablobm pablobm deleted the consistent-custom-dashboards branch July 7, 2022 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant