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

Generating views creates wrong file paths #202

Closed
leewaa opened this issue Nov 9, 2015 · 8 comments
Closed

Generating views creates wrong file paths #202

leewaa opened this issue Nov 9, 2015 · 8 comments
Assignees

Comments

@leewaa
Copy link

leewaa commented Nov 9, 2015

I ran rails g administrate:views and I got:

create  app/views/admin/applications/index.html.erb
...

The paths are wrong. It should be:
app/views/administrate/application/index.html.erb etc.

The docs state the correct paths:
http://administrate-docs.herokuapp.com/customizing_page_views

@leewaa leewaa changed the title Generating views create wrong file paths Generating views creates wrong file paths Nov 10, 2015
@5minpause
Copy link
Contributor

The path is set from

/lib/administrate/view_generator.rb

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

      copy_file(
        template_file,
        "app/views/admin/#{resource_path}/#{template_file}",
      )
    end

But I wouldn't call it wrong, because all other generated files (controllers etc.) live under /admin as well. So I would rather say the documentation should be updated. @Graysonwright unfortunately I couldn't find a repo for the docs. Where could this change be made? Is the doc repo a private repo?

@leewaa
Copy link
Author

leewaa commented Nov 11, 2015

No no it should be /administrate, this is how to override the views from the gem, in any other directory you are simply adding new views and not overriding anything.

_also_

It is creating the views under .../applications/... which is totally wrong, should just be .../application/....

@leewaa
Copy link
Author

leewaa commented Nov 11, 2015

Plus keep in mind these are the base views...

@5minpause
Copy link
Contributor

@leewaa Yes, I believe you are right. I created a Pull Request for @Graysonwright. The part with applications instead of application is curious. If run inside an test Rails app I can confirm your comment. When just running the Administrate tests, I get application. I fixed this as well in the PR. Hope this is the correct way to handle this?

@c-lliope
Copy link
Contributor

@leewaa and @5minpause - thanks for the contributions!

It turns out both app/views/admin and app/views/administrate both work fine. Rails looks up views based on the inheritance tree of your controllers. In this case, our inheritance tree looks like:

Administrate::ApplicationController
Admin::ApplicationController
Admin::CustomersController # base level

so Rails will look for views in:

app/views/admin/customers,
THEN app/views/admin/application,
THEN app/views/administrate/application.

In the future, we'd like to support multiple admin dashboards in the same application, by nesting them under different namespaces (e.g. app/views/admin and app/views/super_admin). In that case, views under app/views/administrate would apply to all of the admin dashboards. Views under app/views/admin would only apply to the dashboard at the /admin route, and wouldn't affect the dashboard at /super_admin.

I'm not sure that affects us now, but it might be nice to keep the views under /admin because changes that people make will be more contained down the line.
I think I'd prefer to keep the generated views in app/views/admin.

@5minpause
Copy link
Contributor

That makes sense. Thanks for clearing this up. What about the application/applications issue though, @Graysonwright?

@c-lliope
Copy link
Contributor

yeah, I think that should definitely be app/views/admin/application/....

@c-lliope c-lliope added the ready label Nov 12, 2015
@c-lliope c-lliope added this to the v0.1.2 milestone Nov 12, 2015
@c-lliope
Copy link
Contributor

#221 will fix this.

c-lliope added a commit that referenced this issue Jan 8, 2016
Closes #202

Problem:

The documentation states that views will be generated
in the `app/views/administrate/` directory,
but they are actually generated in `app/views/admin/`.

Solution:

In fact, both paths work correctly for overriding views.

Rails looks up views based on the inheritance tree of your controllers.
In this case, our inheritance tree looks like:

```
Administrate::ApplicationController
Admin::ApplicationController
Admin::CustomersController # base level
```

... so Rails will look for views in:

`app/views/admin/customers`,
THEN `app/views/admin/application`,
THEN `app/views/administrate/application`.

In the future,
we'd like to support multiple admin dashboards in the same application,
by nesting them under different namespaces
(e.g. `app/views/admin` and `app/views/super_admin`).
In that case, views under `app/views/administrate`
would apply to *all* of the admin dashboards.
Views under `app/views/admin` would only apply
to the dashboard at the `/admin` route,
and wouldn't affect the dashboard at `/super_admin`.

Because of this, we'd prefer to keep the generated views in `/admin`.
We'll keep the current implementation, and update the documentation.
@c-lliope c-lliope self-assigned this Jan 8, 2016
c-lliope added a commit that referenced this issue Jan 19, 2016
Closes #202

Problem:

The documentation states that views will be generated
in the `app/views/administrate/` directory,
but they are actually generated in `app/views/admin/`.

Solution:

In fact, both paths work correctly for overriding views.

Rails looks up views based on the inheritance tree of your controllers.
In this case, our inheritance tree looks like:

```
Administrate::ApplicationController
Admin::ApplicationController
Admin::CustomersController # base level
```

... so Rails will look for views in:

`app/views/admin/customers`,
THEN `app/views/admin/application`,
THEN `app/views/administrate/application`.

In the future,
we'd like to support multiple admin dashboards in the same application,
by nesting them under different namespaces
(e.g. `app/views/admin` and `app/views/super_admin`).
In that case, views under `app/views/administrate`
would apply to *all* of the admin dashboards.
Views under `app/views/admin` would only apply
to the dashboard at the `/admin` route,
and wouldn't affect the dashboard at `/super_admin`.

Because of this, we'd prefer to keep the generated views in `/admin`.
We'll keep the current implementation, and update the documentation.
c-lliope added a commit that referenced this issue Jan 20, 2016
Closes #202

Problem:

The documentation states that views will be generated
in the `app/views/administrate/` directory,
but they are actually generated in `app/views/admin/`.

Solution:

In fact, both paths work correctly for overriding views.

Rails looks up views based on the inheritance tree of your controllers.
In this case, our inheritance tree looks like:

```
Administrate::ApplicationController
Admin::ApplicationController
Admin::CustomersController # base level
```

... so Rails will look for views in:

`app/views/admin/customers`,
THEN `app/views/admin/application`,
THEN `app/views/administrate/application`.

In the future,
we'd like to support multiple admin dashboards in the same application,
by nesting them under different namespaces
(e.g. `app/views/admin` and `app/views/super_admin`).
In that case, views under `app/views/administrate`
would apply to *all* of the admin dashboards.
Views under `app/views/admin` would only apply
to the dashboard at the `/admin` route,
and wouldn't affect the dashboard at `/super_admin`.

Because of this, we'd prefer to keep the generated views in `/admin`.
We'll keep the current implementation, and update the documentation.
@c-lliope c-lliope removed the ready label Jan 20, 2016
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 a pull request may close this issue.

3 participants