add namespace option to generators #956

Rodrigora · 2017-08-01T16:05:36Z

fixes #754

With this change, we'll be able to run the generators with the namespace option:

rails generate administrate:install --namespace manager
rails generate administrate:dashboard --namespace manager

houndci-bot · 2017-08-01T16:05:50Z

spec/generators/routes_generator_spec.rb

+    it "populates default dashboards under the given namespace" do
+      routes = file("config/routes.rb")
+
+      run_generator ['--namespace', 'manager']


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-08-01T16:05:50Z

spec/generators/install_generator_spec.rb

    it "is generated" do
      stub_generator_dependencies

      run_generator

      %w[customer order product line_item].each do |resource|
        expect(Rails::Generators).
-          to invoke_generator("administrate:dashboard", [resource])
+          to invoke_generator("administrate:dashboard", [resource, "--namespace", "admin"])


Line is too long. [91/80]

houndci-bot · 2017-08-01T16:05:50Z

spec/generators/install_generator_spec.rb

+          namespace(:manager) { resources :customers }
+        end
+
+        run_generator ['--namespace', 'manager']


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-08-01T16:05:50Z

spec/generators/install_generator_spec.rb

+      stub_generator_dependencies
+      controller = file("app/controllers/manager/application_controller.rb")
+
+      run_generator ['--namespace', 'manager']


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-08-01T16:05:51Z

spec/generators/install_generator_spec.rb

@@ -18,6 +18,20 @@ module Admin
          class ApplicationController < Administrate::ApplicationController
      RB
    end
+
+    it 'uses the namespace option' do


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-08-01T16:05:51Z

lib/generators/administrate/routes/routes_generator.rb

@@ -6,6 +6,7 @@ module Administrate
  module Generators
    class RoutesGenerator < Rails::Generators::Base
      source_root File.expand_path("../templates", __FILE__)
+      class_option :namespace, type: :string, default: 'admin'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-08-01T16:05:51Z

lib/generators/administrate/install/install_generator.rb

-          "app/controllers/admin/application_controller.rb"
+        template(
+          "application_controller.rb.erb",
+          "app/controllers/#{namespace}/application_controller.rb"


Put a comma after the last parameter of a multiline method call.

houndci-bot · 2017-08-01T16:05:51Z

lib/generators/administrate/install/install_generator.rb

@@ -8,34 +8,41 @@ class InstallGenerator < Rails::Generators::Base
      include Administrate::GeneratorHelpers
      source_root File.expand_path("../templates", __FILE__)

+      class_option :namespace, type: :string, default: 'admin'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-08-01T16:05:51Z

lib/generators/administrate/dashboard/dashboard_generator.rb

@@ -24,6 +24,8 @@ class DashboardGenerator < Rails::Generators::NamedBase
      COLLECTION_ATTRIBUTE_LIMIT = 4
      READ_ONLY_ATTRIBUTES = %w[id created_at updated_at]

+      class_option :namespace, type: :string, default: 'admin'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-08-01T17:08:56Z

spec/generators/install_generator_spec.rb

@@ -4,7 +4,10 @@
 require "support/constant_helpers"

 describe Administrate::Generators::InstallGenerator, :generator do
-  describe "admin/application_controller" do
+


Extra empty line detected at block body beginning.

nickcharlton

This is looking great! A few comments.

nickcharlton · 2017-08-02T10:24:43Z

spec/generators/install_generator_spec.rb

+    end
+
+    it "does not invoke routes generator if namespace routes already exist \
+    when using custom namespace" do


Do you think we'd be able to use a context here, instead of having a super long it?

nickcharlton · 2017-08-02T10:25:58Z

spec/generators/install_generator_spec.rb

+
+      run_generator ["--namespace", "manager"]
+
+      expect(controller).to exist


Do we need to check it exists if we're also ensuring it looks right?

I like this verification as it will raise a proper error message when the file doesn't exist (expected "/home/rodrigo/Projects/administrate/tmp/app/controllers/manager/application_controller.rb" to exist)

If we use only the have_correct_syntax matcher and the generator fails to create the file, the error message would be No such file or directory @ rb_sysopen - /home/rodrigo/Projects/administrate/tmp/app/controllers/manager/application_controller.rb

What do you think?

That's a good reason! Let's leave it in there.

houndci-bot · 2017-08-03T13:22:25Z

spec/generators/install_generator_spec.rb

+    end
+
+
+    context 'using the namespace option' do


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-08-03T13:22:25Z

spec/generators/install_generator_spec.rb

+      )
+    end
+
+


Extra blank line detected.

Rodrigora · 2017-08-03T15:57:46Z

rebased

Rodrigora · 2017-09-27T23:17:47Z

Seems that it was fixed by #339

nickcharlton · 2017-09-28T11:11:40Z

Hi @Rodrigora! I noticed you closed this, but I don't quite think we've solved it.

Looking at the current master, we don't have an option for the generator (but everything else seems to be ready). This issue is relevant, too: #754

pablobm

houndci-bot reviewed Aug 1, 2017

View reviewed changes

Rodrigora force-pushed the add-namespace-option-to-generators branch from c8e9623 to feb785a Compare August 1, 2017 17:16

nickcharlton reviewed Aug 2, 2017

View reviewed changes

houndci-bot reviewed Aug 3, 2017

View reviewed changes

add namespace option to generators

6d4f822

Rodrigora force-pushed the add-namespace-option-to-generators branch from c682de6 to 6d4f822 Compare August 3, 2017 15:56

nickcharlton mentioned this pull request Sep 26, 2017

TypeError in Admin::UsersController#index Admin is not a module #225

Closed

Rodrigora closed this Sep 27, 2017

nickcharlton reopened this Sep 28, 2017

pablobm approved these changes Nov 3, 2017

View reviewed changes

nickcharlton merged commit 844d7b2 into thoughtbot:master Nov 10, 2017

nickcharlton mentioned this pull request Dec 4, 2017

add module name option for generator commands to support #855 #917

Closed

nickcharlton mentioned this pull request Dec 19, 2017

Custom /admin route? #1047

Closed

Rodrigora deleted the add-namespace-option-to-generators branch July 24, 2018 15:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add namespace option to generators #956

add namespace option to generators #956

Rodrigora commented Aug 1, 2017

houndci-bot Aug 1, 2017

houndci-bot Aug 1, 2017

houndci-bot Aug 1, 2017

houndci-bot Aug 1, 2017

houndci-bot Aug 1, 2017

houndci-bot Aug 1, 2017

houndci-bot Aug 1, 2017

houndci-bot Aug 1, 2017

houndci-bot Aug 1, 2017

houndci-bot Aug 1, 2017

nickcharlton left a comment

nickcharlton Aug 2, 2017

Rodrigora Aug 3, 2017

nickcharlton Aug 2, 2017

Rodrigora Aug 3, 2017

nickcharlton Aug 3, 2017

houndci-bot Aug 3, 2017

houndci-bot Aug 3, 2017

Rodrigora commented Aug 3, 2017

Rodrigora commented Sep 27, 2017

nickcharlton commented Sep 28, 2017 •

edited

Loading

pablobm left a comment


		run_generator ["--namespace", "manager"]

		expect(controller).to exist

add namespace option to generators #956

add namespace option to generators #956

Conversation

Rodrigora commented Aug 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickcharlton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rodrigora commented Aug 3, 2017

Rodrigora commented Sep 27, 2017

nickcharlton commented Sep 28, 2017 • edited Loading

pablobm left a comment

Choose a reason for hiding this comment

nickcharlton commented Sep 28, 2017 •

edited

Loading