Feature register pages #758

pcreux · 2011-11-17T15:14:06Z

This branch is ready for review. Could you approve the refactorings I made?

I might add more scenarios to the feature registering_pages.

Cheers!

It goes like: ActiveAdmin.page "My Page" do content do h2 "Hey, this is a custom page" para "Awesome!" end end

Note: The documentation says that when registering a resource you can set its name doing: ``` ActiveAdmin.register Post, :as => "Article" ``` The code however was made so that the :as resource would get a plural. For instance: ``` ActiveAdmin.register Post, :as => "Articles" ``` I fixed this. It might be worth mentioning in the release notes.

mattvague · 2011-11-17T23:30:26Z

lib/active_admin/resource_controller.rb

@@ -5,23 +5,15 @@ require 'active_admin/resource_controller/callbacks'
 require 'active_admin/resource_controller/collection'
 require 'active_admin/resource_controller/filters'
 require 'active_admin/resource_controller/form'
-require 'active_admin/resource_controller/menu'
-require 'active_admin/resource_controller/page_configurations'
 require 'active_admin/resource_controller/scoping'



Seems like it might be worth writing some documentation on what a ResourceController is for vs. a PageController

pcreux · 2011-11-18T09:06:18Z

@mattvague Thanks for reviewing this! :)

gregbell · 2011-11-22T13:51:12Z

lib/active_admin/dsl.rb

@@ -117,6 +117,10 @@ module ActiveAdmin
      controller.set_page_config :show, options, &block
    end

+    def content(options = {}, &block)
+      controller.set_page_config :index, options, &block
+    end


I'm not a big fan of parsing the config through DSL class. There are only two methods that are relavent to a Page, #content and #menu. I wonder if instead of using the DSL, we could just assume that the block is the content and the hash options can include the menu options.

We now have PageDSL and ContentDSL. They both inherit from DSL.

I want customisation common to both DSL (such as menu or controller) similar.

pcreux · 2011-11-23T15:26:58Z

Tested against a "real" rails app revealed a few regressions that I fixed. Also renamed #page method to #register_page and splitted DSL into ResourceDSL and PageDSL.

This feature is ready to be merged in.

An other thing we could do before releasing 0.4.0 is grouping classes by types in directories: controller, dsl, config.

pcreux · 2011-11-25T18:07:26Z

"This pull request cannot be automatically merged." Feck!

simonoff · 2011-11-25T19:34:05Z

This feature must be merged!

meskallito · 2011-11-25T21:37:19Z

Hi
I've test your branch a bit and found issue with internalization.
Names of resources become non-latins if there are non-latin translations in active record locales.
result: namespace.rb:187:in `eval': (eval):1

meskallito · 2011-11-25T21:59:01Z

line number is false actually. Error appears in "register_resource_controller" function

gregbell · 2011-11-26T00:05:28Z

I'll get this merged over the weekend. It's next on my list to merge :)

pcreux · 2011-11-26T09:35:25Z

@meskallito I didn't get what the issue is. What do you mean by non Latin translation?

pcreux · 2011-11-26T09:35:59Z

@gregbell I can fix the merge conflict if you want!

meskallito · 2011-11-26T13:40:53Z

Functions in module "Naming" (class Resource < Config), that were rewritten by you in commit [f5c662f], provide resource names for controller_name in class Config. The problem occurs if I have models names translations in locales (for example in Chinese), because in that case "register_page_controller" tries to create controllers with non-latin names. is it clear?
Anyway, thank you for this functionality. It really helped me.

pcreux · 2011-11-26T17:33:59Z

Ok, that makes a lot of sense!

So the user facing names (resource.name and resource.plural_name) should
use i18n while the "system" ones (camelcased and underscored) should use...
the resource one?

For now you can't register pages with non-latin names then.

I will try to fix this before 11am GMT+1 (
http://thatz.at/Nov-27-2011-11-00-GMT+1-DST) so that this can get merged
into master before we release 0.4.0.

pcreux · 2011-11-26T17:34:55Z

@meskallito said:

Functions in module "Naming" (class Resource < Config), that were rewritten by you in commit [f5c662f], provide resource names for controller_name in class Config. The problem occurs if I have models names translations in locales (for example in Chinese), because in that case "register_page_controller" tries to create controllers with non-latin names. is it clear?
Anyway, thank you for this functionality. It really helped me.

pcreux · 2011-11-26T17:35:01Z

Ok, that makes a lot of sense!

So the user facing names (resource.name and resource.plural_name) should use i18n while the "system" ones (camelcased and underscored) should use... the resource one?

For now you can't register pages with non-latin names then.

I will try to fix this before 11am GMT+1 (http://thatz.at/Nov-27-2011-11-00-GMT+1-DST) so that this can get merged into master before we release 0.4.0.

pcreux · 2011-11-26T17:51:43Z

@gregbell I merged this branch into the current master and resolved 4 straight forward merge conflict.

It's available here: https://github.com/pcreux/active_admin/tree/merge-feature-register-pages

The feature registering_page does not pass though: http://travis-ci.org/#!/pcreux/active_admin/builds/344890

undefined local variable or method `active_admin_namespace' for #<ActiveAdmin::Views::HeaderRenderer:0xb8c2ee4> (ActionView::Template::Error)
493./lib/active_admin/arbre/builder.rb:47:in `method_missing'
494./lib/active_admin/renderer.rb:26:in `method_missing'
495./lib/active_admin/views/header_renderer.rb:16:in `title'

I let you taking care of this today. I won't have time to work on that this week-end.

FYI:
I also ran into this issue when running the specs on my machine (Ubuntu + ree 1.8.7):

Stylesheets when Rails 3.0.x should not have any syntax errors
     Failure/Error: css.should_not include("Syntax error:")
       expected "/*\nSyntax error: File to import not found or unreadable: bourbon.\n "
  [...]

meskallito · 2011-11-26T17:54:29Z

Yes you are right. In addition, this functions affects the process of creating a common active_admin resources. Even if I dont init any page I'll get problems in register_resource_controller' cause it uses the same config.controller_name and in router.rb (it uses underscored_resource_name). I can prepapre pull request into your branch if you want

pcreux · 2011-11-26T17:55:45Z

@meskallito Sure, please do!

meskallito · 2011-11-26T18:24:05Z

@pcreux ok, I'll do in an hour or two

pcreux · 2011-11-27T10:15:10Z

I just had to clean up my test env to get the spec to pass.

And I won't fix the non-latin support this week-end.

gregbell · 2011-11-28T15:43:39Z

I rebased the branch off master and then merged it in. There are definitely some things I would like to add before releasing this with 0.4.0 such as documentation, sidebar items, action items and the issue that @meskallito is having.

Nice work @pcreux! This is an awesome feature.

pcreux added 10 commits November 7, 2011 17:15

Add comments to describe & contexts closing tags

b93e47f

Add simple page registration.

a242bc0

It goes like: ActiveAdmin.page "My Page" do content do h2 "Hey, this is a custom page" para "Awesome!" end end

Refactor ActiveAdmin Controller hierarchy.

6bd6114

Add Config and starting to move things up to it.

eb86bff

Move shared behavior to Config.

11fd858

Move shared behavior up to BaseController.

c2476a7

Add abstract methods and spec Config::Naming.

83cde01

Remove #comments? from Config. Only Resource respond_to comments

078e573

Moved Menu specs up to Config shared examples.

00a6d13

mattvague reviewed Nov 17, 2011
View reviewed changes

pcreux added 6 commits November 18, 2011 16:39

Add comments to Page & Resource's Config and Controllers

8bb90b2

Use plural_camelized_resource_name to define controller_name

d6823d5

Remove unecessary code.

7b273a3

Split namespace_spec: register page / resources

7dabab0

Better feature for Page registration

fda3405

Fix naming for resource without a titleized name.

dbc7713

gregbell reviewed Nov 22, 2011
View reviewed changes

pcreux added 6 commits November 23, 2011 13:38

Add attribute reader :resource to all Config.

4f5f2a4

Logic to generate routes for plural resources moved down to Resource

81376b3

Resource names are now titleized.

175acf3

Rename method ActiveAdmin.page to register_page

c8a30c7

Move routing specs from page to routing specs.

e34b389

Split DSL into ResourceDSL and PageDSL

9e0f46c

gregbell closed this Nov 28, 2011

awingla mentioned this pull request May 31, 2014

remove ActiveAdmin::Config and replace it to ActiveAdmin::Resource #3181

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature register pages #758

Feature register pages #758

pcreux commented Nov 17, 2011

mattvague Nov 17, 2011

pcreux commented Nov 18, 2011

gregbell Nov 22, 2011

pcreux Nov 23, 2011

pcreux commented Nov 23, 2011

pcreux commented Nov 25, 2011

simonoff commented Nov 25, 2011

meskallito commented Nov 25, 2011

meskallito commented Nov 25, 2011

gregbell commented Nov 26, 2011

pcreux commented Nov 26, 2011

pcreux commented Nov 26, 2011

meskallito commented Nov 26, 2011

pcreux commented Nov 26, 2011

pcreux commented Nov 26, 2011

pcreux commented Nov 26, 2011

pcreux commented Nov 26, 2011

meskallito commented Nov 26, 2011

pcreux commented Nov 26, 2011

meskallito commented Nov 26, 2011

pcreux commented Nov 27, 2011

gregbell commented Nov 28, 2011

Feature register pages #758

Feature register pages #758

Conversation

pcreux commented Nov 17, 2011

mattvague Nov 17, 2011

Choose a reason for hiding this comment

pcreux commented Nov 18, 2011

gregbell Nov 22, 2011

Choose a reason for hiding this comment

pcreux Nov 23, 2011

Choose a reason for hiding this comment

pcreux commented Nov 23, 2011

pcreux commented Nov 25, 2011

simonoff commented Nov 25, 2011

meskallito commented Nov 25, 2011

meskallito commented Nov 25, 2011

gregbell commented Nov 26, 2011

pcreux commented Nov 26, 2011

pcreux commented Nov 26, 2011

meskallito commented Nov 26, 2011

pcreux commented Nov 26, 2011

pcreux commented Nov 26, 2011

pcreux commented Nov 26, 2011

pcreux commented Nov 26, 2011

meskallito commented Nov 26, 2011

pcreux commented Nov 26, 2011

meskallito commented Nov 26, 2011

pcreux commented Nov 27, 2011

gregbell commented Nov 28, 2011