Import command tweaks and fixes

- Use PHPUnit 7.*
- Use Orchestra Testbench 3.7 for Laravel 5.7 compatibility
- Throw a runtime exception when no users are found when running the command
- Check if the console is interactive before asking confirmation to display users to be imported / synchornized
- Check if the console is not interactive before asking to import the located users
- Updated console tests

Author: stevebauman

composer.json

@@ -10,8 +10,8 @@
     },
     "require-dev": {
         "mockery/mockery": "~1.0",
-        "phpunit/phpunit": "~6.0",
-        "orchestra/testbench": "~3.2"
+        "phpunit/phpunit": "~7.0",
+        "orchestra/testbench": "~3.7"
     },
     "archive": {
         "exclude": ["/tests"]

src/Commands/Console/Import.php

@@ -39,6 +39,7 @@ class Import extends Command
      *
      * @return void
      *
+     * @throws \RuntimeException
      * @throws \Adldap\Models\ModelNotFoundException
      */
     public function handle()
@@ -47,17 +48,25 @@ public function handle()
 
         $count = count($users);
 
-        if ($count === 1) {
+        if ($count === 0) {
+            throw new \RuntimeException("There were no users found to import.");
+        } else if ($count === 1) {
             $this->info("Found user '{$users[0]->getCommonName()}'.");
         } else {
             $this->info("Found {$count} user(s).");
         }
 
-        if ($this->confirm('Would you like to display the user(s) to be imported / synchronized?', $default = false)) {
+        if (
+            $this->input->isInteractive() &&
+            $this->confirm('Would you like to display the user(s) to be imported / synchronized?', $default = false)
+        ) {
             $this->display($users);
         }
 
-        if ($this->confirm('Would you like these users to be imported / synchronized?', $default = true)) {
+        if (
+            ! $this->input->isInteractive() ||
+            $this->confirm('Would you like these users to be imported / synchronized?', $default = true)
+        ) {
             $imported = $this->import($users);
 
             $this->info("Successfully imported / synchronized {$imported} user(s).");

tests/Console/ImportTest.php

@@ -23,9 +23,11 @@ public function test_importing_one_user()
             ->shouldReceive('getLdapDiscoveryAttribute')->once()->andReturn('mail')
             ->shouldReceive('getEloquentUsernameAttribute')->once()->andReturn('email');
 
-        $r = $this->artisan('adldap:import', ['user' => 'jdoe', '--no-interaction']);
+        $this->artisan('adldap:import', ['user' => 'jdoe', '--no-interaction' => true])
+            ->expectsOutput("Found user 'John Doe'.")
+            ->expectsOutput("Successfully imported / synchronized 1 user(s).")
+            ->assertExitCode(0);
 
-        $this->assertEquals(0, $r);
         $this->assertDatabaseHas('users', ['email' => '[email protected]']);
     }
 
@@ -55,13 +57,37 @@ public function test_importing_multiple_users()
             ->shouldReceive('getLdapDiscoveryAttribute')->twice()->andReturn('mail')
             ->shouldReceive('getEloquentUsernameAttribute')->twice()->andReturn('email');
 
-        $r = $this->artisan('adldap:import', ['--no-interaction']);
+        $this->artisan('adldap:import', ['--no-interaction' => true])
+            ->expectsOutput("Found 2 user(s).")
+            ->expectsOutput("Successfully imported / synchronized 2 user(s).")
+            ->assertExitCode(0);
 
-        $this->assertEquals(0, $r);
         $this->assertDatabaseHas('users', ['email' => '[email protected]']);
         $this->assertDatabaseHas('users', ['email' => '[email protected]']);
     }
 
+    public function test_questions_asked_with_interaction()
+    {
+        $b = m::mock(Builder::class);
+
+        $u = $this->makeLdapUser();
+
+        $b->shouldReceive('findOrFail')->once()->with('jdoe')->andReturn($u);
+
+        Resolver::shouldReceive('query')->once()->andReturn($b)
+            ->shouldReceive('getLdapDiscoveryAttribute')->once()->andReturn('mail')
+            ->shouldReceive('getEloquentUsernameAttribute')->once()->andReturn('email');
+
+        $this->artisan('adldap:import', ['user' => 'jdoe'])
+            ->expectsOutput("Found user 'John Doe'.")
+            ->expectsQuestion('Would you like to display the user(s) to be imported / synchronized?', 'no')
+            ->expectsQuestion('Would you like these users to be imported / synchronized?', 'yes')
+            ->expectsOutput("Successfully imported / synchronized 1 user(s).")
+            ->assertExitCode(0);
+
+        $this->assertDatabaseHas('users', ['email' => '[email protected]']);
+    }
+
     public function test_model_will_be_restored_when_ldap_account_is_active()
     {
         $model = TestUser::create([
@@ -93,9 +119,11 @@ public function test_model_will_be_restored_when_ldap_account_is_active()
             ->shouldReceive('getLdapDiscoveryAttribute')->once()->andReturn('mail')
             ->shouldReceive('getEloquentUsernameAttribute')->once()->andReturn('email');
 
-        $r = $this->artisan('adldap:import', ['--restore' => true, '--no-interaction' => true]);
+        $this->artisan('adldap:import', ['--restore' => true, '--no-interaction' => true])
+            ->expectsOutput("Found user 'John Doe'.")
+            ->expectsOutput("Successfully imported / synchronized 1 user(s).")
+            ->assertExitCode(0);
 
-        $this->assertEquals(0, $r);
         $this->assertFalse($model->fresh()->trashed());
     }
 
@@ -128,27 +156,30 @@ public function test_model_will_be_soft_deleted_when_ldap_account_is_disabled()
             ->shouldReceive('getLdapDiscoveryAttribute')->once()->andReturn('mail')
             ->shouldReceive('getEloquentUsernameAttribute')->once()->andReturn('email');
 
-        $r = $this->artisan('adldap:import', ['--delete' => true, '--no-interaction' => true]);
-
-        $this->assertEquals(0, $r);
+        $this->artisan('adldap:import', ['--delete' => true, '--no-interaction' => true])
+            ->expectsOutput("Found user 'John Doe'.")
+            ->expectsOutput("Successfully imported / synchronized 1 user(s).")
+            ->assertExitCode(0);
+        
         $this->assertTrue($model->fresh()->trashed());
     }
 
+    /**
+     * @expectedException \RuntimeException
+     */
     public function test_filter_option_applies_to_ldap_query()
     {
         $f = '(samaccountname=jdoe)';
 
         $b = m::mock(Builder::class);
 
         $b
-            ->shouldReceive('rawFilter')->once()->with($f)->andReturn($b)
-            ->shouldReceive('paginate')->once()->andReturn($b)
+            ->shouldReceive('rawFilter')->once()->with($f)->andReturnSelf()
+            ->shouldReceive('paginate')->once()->andReturnSelf()
             ->shouldReceive('getResults')->once()->andReturn([]);
 
         Resolver::shouldReceive('query')->once()->andReturn($b);
 
-        $r = $this->artisan('adldap:import', ['--filter' => $f, '--no-interaction' => true]);
-
-        $this->assertEquals(0, $r);
+        $this->artisan('adldap:import', ['--filter' => $f, '--no-interaction' => true]);
     }
 }