(PDK-628) Addition of module_name question to interview

This enables the re-use of the modules interview with pdk convert without disturbing generate new module workflow.

lib/pdk/cli/module/generate.rb

@@ -27,7 +27,7 @@ module PDK::CLI
       answer = redirect.run
 
       if answer
-        opts[:name] = module_name
+        opts[:module_name] = module_name
         opts[:target_dir] = module_name
 
         PDK.logger.info(_('Creating new module: %{modname}') % { modname: module_name })

lib/pdk/cli/new/module.rb

@@ -21,7 +21,7 @@ module PDK::CLI
         exit 1
       end
 
-      opts[:name] = module_name
+      opts[:module_name] = module_name
       opts[:target_dir] = target_dir.nil? ? module_name : target_dir
 
       PDK.logger.info(_('Creating new module: %{modname}') % { modname: module_name })

lib/pdk/generators/module.rb

@@ -33,11 +33,11 @@ def self.puppetlabs_template_url
       end
 
       def self.validate_options(opts)
-        unless PDK::CLI::Util::OptionValidator.valid_module_name?(opts[:name])
+        unless PDK::CLI::Util::OptionValidator.valid_module_name?(opts[:module_name])
           error_msg = _(
             "'%{module_name}' is not a valid module name.\n" \
             'Module names must begin with a lowercase letter and can only include lowercase letters, digits, and underscores.',
-          ) % { module_name: opts[:name] }
+          ) % { module_name: opts[:module_name] }
           raise PDK::CLI::ExitWithError, error_msg
         end
 
@@ -98,7 +98,7 @@ def self.invoke(opts = {})
 
         begin
           if FileUtils.mv(temp_target_dir, target_dir)
-            PDK.logger.info(_('Module \'%{name}\' generated at path \'%{path}\'.') % { name: opts[:name], path: target_dir })
+            PDK.logger.info(_('Module \'%{name}\' generated at path \'%{path}\'.') % { name: opts[:module_name], path: target_dir })
             PDK.logger.info(_('In your module directory, add classes with the \'pdk new class\' command.'))
           end
         rescue Errno::EACCES => e
@@ -128,7 +128,7 @@ def self.prepare_metadata(opts)
         username = PDK.answers['forge-username'] || username_from_login
 
         defaults = {
-          'name'         => "#{username}-#{opts[:name]}",
+          'name'         => "#{username}-#{opts[:module_name]}",
           'version'      => '0.1.0',
           'dependencies' => [],
           'requirements' => [
@@ -169,6 +169,15 @@ def self.prepare_module_directory(target_dir)
 
       def self.module_interview(metadata, opts = {})
         questions = [
+          {
+            name:             'module_name',
+            question:         _('If you have a name for your module, add it here.'),
+            help:             _('This is the name that will be associated with your module, it should be relevant to the modules content.'),
+            required:         true,
+            validate_pattern: %r{\A[a-z0-9]+\Z}i,
+            validate_message: _('Module names can only contain lowercase letters and numbers'),
+            default:          metadata.data['name'],
+          },
           {
             name:             'forge_username',
             question:         _('If you have a Puppet Forge username, add it here.'),
@@ -290,6 +299,7 @@ def self.module_interview(metadata, opts = {})
 
         interview = PDK::CLI::Util::Interview.new(prompt)
 
+        questions.reject! { |q| q[:name] == 'module_name' } if opts.key?(:module_name)
         questions.reject! { |q| q[:name] == 'license' } if opts.key?(:license)
 
         interview.add_questions(questions)
@@ -310,7 +320,7 @@ def self.module_interview(metadata, opts = {})
         end
 
         forge_username = answers['forge_username']
-        answers['name'] = "#{answers['forge_username']}-#{opts[:name]}"
+        answers['name'] = "#{answers['forge_username']}-" + (opts[:module_name] || answers['module_name'])
         answers['license'] = opts[:license] if opts.key?(:license)
         answers['operatingsystem_support'].flatten!
         metadata.update!(answers)

spec/unit/pdk/cli/new/module_spec.rb

@@ -29,7 +29,7 @@
     let(:module_name) { 'test123' }
 
     it 'validates the module name' do
-      expect(PDK::Generate::Module).to receive(:invoke).with(hash_including(name: module_name))
+      expect(PDK::Generate::Module).to receive(:invoke).with(hash_including(module_name: module_name))
       expect(logger).to receive(:info).with("Creating new module: #{module_name}")
       PDK::CLI.run(['new', 'module', module_name])
     end
@@ -38,7 +38,7 @@
       let(:target_dir) { 'target' }
 
       it 'passes the target directory to PDK::Generate::Module.invoke' do
-        expect(PDK::Generate::Module).to receive(:invoke).with(hash_including(name: module_name, target_dir: target_dir))
+        expect(PDK::Generate::Module).to receive(:invoke).with(hash_including(module_name: module_name, target_dir: target_dir))
         expect(logger).to receive(:info).with("Creating new module: #{module_name}")
         PDK::CLI.run(['new', 'module', module_name, target_dir])
       end

spec/unit/pdk/generate/module_spec.rb

@@ -53,7 +53,7 @@
     let(:invoke_opts) do
       {
         :target_dir       => target_dir,
-        :name             => 'foo',
+        :module_name      => 'foo',
         :'skip-interview' => true,
       }
     end
@@ -68,7 +68,7 @@
         expect(logger).not_to receive(:info).with(a_string_matching(%r{In your new module directory, add classes with the 'pdk new class' command}i))
 
         expect {
-          described_class.invoke(name: 'foo', target_dir: target_dir)
+          described_class.invoke(module_name: 'foo', target_dir: target_dir)
         }.to raise_error(PDK::CLI::ExitWithError, %r{destination directory '.+' already exists}i)
       end
     end
@@ -259,7 +259,7 @@
 
     let(:module_name) { 'bar' }
     let(:default_metadata) { {} }
-    let(:options) { { name: module_name } }
+    let(:options) { { module_name: module_name } }
 
     before(:each) do
       prompt = TTY::TestPrompt.new
@@ -397,10 +397,45 @@
       end
     end
 
+    context 'when there is no module_name provided' do
+      include_context 'allow summary to be printed to stdout'
+
+      let(:options) { { license: 'MIT' } }
+      let(:responses) do
+        [
+          'mymodule',
+          'myforgename',
+          '2.2.0',
+          'William Hopper',
+          '',
+          'A simple module to do some stuff.',
+          'github.com/whopper/bar',
+          'forge.puppet.com/whopper/bar',
+          'tickets.foo.com/whopper/bar',
+          'yes',
+        ]
+      end
+
+      it 'populates the Metadata object based on user input for both module name and forge name' do
+        allow($stdout).to receive(:puts).with(a_string_matching(%r{9 questions}))
+
+        expect(interview_metadata).to include(
+          'name'         => 'myforgename-mymodule',
+          'version'      => '2.2.0',
+          'author'       => 'William Hopper',
+          'license'      => 'MIT',
+          'summary'      => 'A simple module to do some stuff.',
+          'source'       => 'github.com/whopper/bar',
+          'project_page' => 'forge.puppet.com/whopper/bar',
+          'issues_url'   => 'tickets.foo.com/whopper/bar',
+        )
+      end
+    end
+
     context 'when the user provides the license as a command line option' do
       include_context 'allow summary to be printed to stdout'
 
-      let(:options) { { name: module_name, license: 'MIT' } }
+      let(:options) { { module_name: module_name, license: 'MIT' } }
       let(:responses) do
         [
           'foo',
@@ -587,10 +622,10 @@
       allow(described_class).to receive(:username_from_login).and_return('testlogin')
     end
 
-    let(:options) { { name: 'baz' } }
+    let(:options) { { module_name: 'baz' } }
 
     context 'when provided :skip-interview => true' do
-      let(:options) { { :name => 'baz', :'skip-interview' => true } }
+      let(:options) { { :module_name => 'baz', :'skip-interview' => true } }
 
       it 'does not perform the module interview' do
         expect(described_class).not_to receive(:module_interview)
@@ -647,7 +682,7 @@
       end
 
       context 'and the user specifies a license as a command line option' do
-        let(:options) { { name: 'baz', license: 'Apache-2.0' } }
+        let(:options) { { module_name: 'baz', license: 'Apache-2.0' } }
 
         it 'prefers the license specified on the command line over the saved license answer' do
           expect(metadata.data).to include('license' => 'Apache-2.0')