Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/Adapters/Laravel/Console/Commands/PanDeleteCommand.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

declare(strict_types=1);

namespace Pan\Adapters\Laravel\Console\Commands;

use Illuminate\Console\Command;
use Pan\Contracts\AnalyticsRepository;

class PanDeleteCommand extends Command
{
/**
* The name and signature of the console command.
*
* @var string
*/
protected $signature = 'pan:delete {--id= : The ID of the analytic to delete}';

/**
* The console command description.
*
* @var string
*/
protected $description = 'Delete analytics by ID or all at once';
Comment thread
MrPunyapal marked this conversation as resolved.
Outdated

/**
* Execute the console command.
*/
public function handle(AnalyticsRepository $repository): void
Comment thread
nunomaduro marked this conversation as resolved.
{
$id = $this->option('id');
Comment thread
MrPunyapal marked this conversation as resolved.
Outdated

if (! $id) {
$this->error('Please specify--id');
Comment thread
ruchit288 marked this conversation as resolved.
Outdated
}

$repository->delete((int) $id);
$this->info('Analytic has been deleted.');
}
}
2 changes: 2 additions & 0 deletions src/Adapters/Laravel/Providers/PanServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Illuminate\Support\ServiceProvider;
use Pan\Adapters\Laravel\Console\Commands\InstallPanCommand;
use Pan\Adapters\Laravel\Console\Commands\PanCommand;
use Pan\Adapters\Laravel\Console\Commands\PanDeleteCommand;
use Pan\Adapters\Laravel\Console\Commands\PanFlushCommand;
use Pan\Adapters\Laravel\Http\Controllers\EventController;
use Pan\Adapters\Laravel\Http\Middleware\InjectJavascriptLibrary;
Expand Down Expand Up @@ -83,6 +84,7 @@ private function registerCommands(): void
InstallPanCommand::class,
PanCommand::class,
PanFlushCommand::class,
PanDeleteCommand::class,
]);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,13 @@ public function flush(): void
{
DB::table('pan_analytics')->truncate();
}

/**
Comment thread
ruchit288 marked this conversation as resolved.
* Delete a specific analytic by ID.
*/
public function delete(int $id): void
{
DB::table('pan_analytics')->where('id', $id)->delete();

}
}
5 changes: 5 additions & 0 deletions src/Contracts/AnalyticsRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,9 @@ public function increment(string $name, EventType $event): void;
* Flush all analytics.
*/
public function flush(): void;

/**
* Delete a specific analytic by ID.
*/
public function delete(int $id): void;
}
22 changes: 22 additions & 0 deletions tests/Feature/Laravel/Console/PanDeleteTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

beforeEach(function (): void {
$this->repository = mock(AnalyticsRepository::class)->makePartial();
Copy link
Copy Markdown
Contributor

@taghwo taghwo Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the test here, the way it's broken down.
What is the performance gain in using a mock versus hitting the DB here?
I would suggest hitting the DB and confirming the row is actually deleted or test the repository method in isolation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ruchit288 can you assert the database is missing the ID after deleting in these tests instead of the mock? Another way would be, leave the command class test as it is, and have a separate test for the delete of method of the repository (assert database missing the ID deleted).

You can check the merged branches, look at the feature test cases on how you can insert some data into the table. That would even prevent hard coding the id like ['id' => 1]. You can use insertGetId, which would make the tests more stable and less prone to failure.

app()->instance(AnalyticsRepository::class, $this->repository);
});

it('deletes a specific analytic by ID', function (): void {
$this->repository->shouldReceive('delete')->once()->with(1)->andReturn();

$command = artisan('pan:delete', ['--id' => 1]);

expect($command->exitCode())->toBe(0);
expect($command->output())->toContain('Analytic has been deleted.');
});

it('fails when no option is provided', function (): void {
$command = artisan('pan:delete');

expect($command->exitCode())->toBe(1);
expect($command->output())->toContain('Please specify --id option.');
});