Project name is now unique per client and organization

app/Http/Requests/V1/Project/ProjectStoreRequest.php

@@ -11,6 +11,7 @@ use App\Rules\ColorRule;
 use Illuminate\Contracts\Validation\ValidationRule;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Foundation\Http\FormRequest;
+use Illuminate\Support\Str;
 use Korridor\LaravelModelValidationRules\Rules\ExistsEloquent;
 use Korridor\LaravelModelValidationRules\Rules\UniqueEloquent;
 
@@ -27,6 +28,7 @@ class ProjectStoreRequest extends FormRequest
     public function rules(): array
     {
         return [
+            // Name of the project, the name needs to be unique per client and organization
             'name' => [
                 'required',
                 'string',
@@ -34,7 +36,13 @@ class ProjectStoreRequest extends FormRequest
                 'max:255',
                 UniqueEloquent::make(Project::class, 'name', function (Builder $builder): Builder {
                     /** @var Builder<Project> $builder */
-                    return $builder->whereBelongsTo($this->organization, 'organization');
+                    $clientId = $this->input('client_id');
+                    if (! is_string($clientId) || ! Str::isUuid($clientId)) {
+                        $clientId = null;
+                    }
+
+                    return $builder->whereBelongsTo($this->organization, 'organization')
+                        ->where('client_id', $clientId);
                 })->withCustomTranslation('validation.project_name_already_exists'),
             ],
             'color' => [
@@ -55,6 +63,7 @@ class ProjectStoreRequest extends FormRequest
             ],
             // ID of the client
             'client_id' => [
+                'present',
                 'nullable',
                 ExistsEloquent::make(Client::class, null, function (Builder $builder): Builder {
                     /** @var Builder<Client> $builder */

app/Http/Requests/V1/Project/ProjectUpdateRequest.php

@@ -11,6 +11,7 @@ use App\Rules\ColorRule;
 use Illuminate\Contracts\Validation\ValidationRule;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Foundation\Http\FormRequest;
+use Illuminate\Support\Str;
 use Korridor\LaravelModelValidationRules\Rules\ExistsEloquent;
 use Korridor\LaravelModelValidationRules\Rules\UniqueEloquent;
 
@@ -34,7 +35,13 @@ class ProjectUpdateRequest extends FormRequest
                 'max:255',
                 UniqueEloquent::make(Project::class, 'name', function (Builder $builder): Builder {
                     /** @var Builder<Project> $builder */
-                    return $builder->whereBelongsTo($this->organization, 'organization');
+                    $clientId = $this->input('client_id');
+                    if (! is_string($clientId) || ! Str::isUuid($clientId)) {
+                        $clientId = null;
+                    }
+
+                    return $builder->whereBelongsTo($this->organization, 'organization')
+                        ->where('client_id', $clientId);
                 })->ignore($this->project?->getKey())->withCustomTranslation('validation.project_name_already_exists'),
             ],
             'color' => [
@@ -54,6 +61,7 @@ class ProjectUpdateRequest extends FormRequest
                 'boolean',
             ],
             'client_id' => [
+                'present',
                 'nullable',
                 ExistsEloquent::make(Client::class, null, function (Builder $builder): Builder {
                     /** @var Builder<Client> $builder */

app/Service/Import/Importers/ClockifyProjectsImporter.php

@@ -37,9 +37,9 @@ class ClockifyProjectsImporter extends DefaultImporter
                 if ($record['Project'] !== '') {
                     $projectId = $this->projectImportHelper->getKey([
                         'name' => $record['Project'],
+                        'client_id' => $clientId,
                         'organization_id' => $this->organization->id,
                     ], [
-                        'client_id' => $clientId,
                         'color' => $this->colorService->getRandomColor(),
                         'is_billable' => $record['Billability'] === 'Yes',
                         'billable_rate' => $billableRateKey !== null && $record[$billableRateKey] !== '' ? (int) (((float) $record[$billableRateKey]) * 100) : null,

app/Service/Import/Importers/ClockifyTimeEntriesImporter.php

@@ -83,9 +83,9 @@ class ClockifyTimeEntriesImporter extends DefaultImporter
                 if ($record['Project'] !== '') {
                     $projectId = $this->projectImportHelper->getKey([
                         'name' => $record['Project'],
+                        'client_id' => $clientId,
                         'organization_id' => $this->organization->id,
                     ], [
-                        'client_id' => $clientId,
                         'color' => $this->colorService->getRandomColor(),
                         'is_billable' => false,
                     ]);

app/Service/Import/Importers/DefaultImporter.php

@@ -97,7 +97,7 @@ abstract class DefaultImporter implements ImporterContract
                 'in:placeholder',
             ],
         ]);
-        $this->projectImportHelper = new ImportDatabaseHelper(Project::class, ['name', 'organization_id'], true, function (Builder $builder) {
+        $this->projectImportHelper = new ImportDatabaseHelper(Project::class, ['name', 'client_id', 'organization_id'], true, function (Builder $builder) {
             /** @var Builder<Project> $builder */
             return $builder->where('organization_id', $this->organization->id);
         }, validate: [
@@ -114,6 +114,11 @@ abstract class DefaultImporter implements ImporterContract
                 'integer',
                 'max:2147483647',
             ],
+            'client_id' => [
+                'nullable',
+                'string',
+                'uuid',
+            ],
         ], beforeSave: function (Project $project): void {
             if ($project->billable_rate === 0) {
                 $project->billable_rate = null;

app/Service/Import/Importers/GenericProjectsImporter.php

@@ -55,12 +55,12 @@ class GenericProjectsImporter extends DefaultImporter
                     }
                     $this->projectImportHelper->getKey([
                         'name' => $record['name'],
+                        'client_id' => $clientId,
                         'organization_id' => $this->organization->id,
                     ], [
                         'color' => isset($record['color']) && $record['color'] !== '' ? $record['color'] : app(ColorService::class)->getRandomColor(),
                         'billable_rate' => isset($record['billable_rate']) && $record['billable_rate'] !== '' ? (int) $record['billable_rate'] : null,
                         'is_public' => isset($record['is_public']) && $record['is_public'] === 'true',
-                        'client_id' => $clientId,
                         'is_billable' => isset($record['billable_default']) && $record['billable_default'] === 'true',
                         'estimated_time' => isset($record['estimated_time']) && $record['estimated_time'] !== '' && is_numeric($record['estimated_time']) && ((int) $record['estimated_time'] !== 0) ? (int) $record['estimated_time'] : null,
                         'archived_at' => $archivedAt,

app/Service/Import/Importers/GenericTimeEntriesImporter.php

@@ -99,9 +99,9 @@ class GenericTimeEntriesImporter extends DefaultImporter
                 if ($record['project'] !== '') {
                     $projectId = $this->projectImportHelper->getKey([
                         'name' => $record['project'],
+                        'client_id' => $clientId,
                         'organization_id' => $this->organization->id,
                     ], [
-                        'client_id' => $clientId,
                         'is_billable' => false,
                         'color' => $this->colorService->getRandomColor(),
                     ]);

app/Service/Import/Importers/HarvestProjectsImporter.php

@@ -60,10 +60,10 @@ class HarvestProjectsImporter extends DefaultImporter
                     $billableHours = $billableHoursField !== '' && is_numeric($billableHoursField) ? (int) ((float) $billableHoursField) : null;
                     $this->projectImportHelper->getKey([
                         'name' => $record['Project'],
+                        'client_id' => $clientId,
                         'organization_id' => $this->organization->id,
                     ], [
                         'color' => $this->colorService->getRandomColor(),
-                        'client_id' => $clientId,
                         'estimated_time' => $estimatedTime,
                         'is_billable' => $billableHours > 0,
                     ]);

app/Service/Import/Importers/HarvestTimeEntriesImporter.php

@@ -78,9 +78,9 @@ class HarvestTimeEntriesImporter extends DefaultImporter
                 if ($record['Project'] !== '') {
                     $projectId = $this->projectImportHelper->getKey([
                         'name' => $record['Project'],
+                        'client_id' => $clientId,
                         'organization_id' => $this->organization->id,
                     ], [
-                        'client_id' => $clientId,
                         'color' => $this->colorService->getRandomColor(),
                         'is_billable' => true,
                     ]);

app/Service/Import/Importers/SolidtimeImporter.php

@@ -176,12 +176,12 @@ class SolidtimeImporter extends DefaultImporter
 
                 $this->projectImportHelper->getKey([
                     'name' => $project['name'],
+                    'client_id' => $clientId,
                     'organization_id' => $this->organization->getKey(),
                 ], [
                     'color' => $project['color'],
                     'billable_rate' => $project['billable_rate'] === '' ? null : (int) $project['billable_rate'],
                     'is_public' => $project['is_public'] === 'true',
-                    'client_id' => $clientId,
                     'is_billable' => $project['is_billable'] === 'true',
                     'archived_at' => $project['archived_at'] !== '' ? Carbon::createFromFormat('Y-m-d\TH:i:s\Z', $project['archived_at'], 'UTC') : null,
                 ], $project['id']);

app/Service/Import/Importers/TogglDataImporter.php

@@ -137,9 +137,9 @@ class TogglDataImporter extends DefaultImporter
 
                 $projectId = $this->projectImportHelper->getKey([
                     'name' => $project->name,
+                    'client_id' => $clientId,
                     'organization_id' => $this->organization->getKey(),
                 ], [
-                    'client_id' => $clientId,
                     'color' => $project->color,
                     'is_billable' => $project->billable,
                     'is_public' => ! $project->is_private,

app/Service/Import/Importers/TogglTimeEntriesImporter.php

@@ -83,9 +83,9 @@ class TogglTimeEntriesImporter extends DefaultImporter
                 if ($record['Project'] !== '') {
                     $projectId = $this->projectImportHelper->getKey([
                         'name' => $record['Project'],
+                        'client_id' => $clientId,
                         'organization_id' => $this->organization->id,
                     ], [
-                        'client_id' => $clientId,
                         'is_billable' => false,
                         'color' => $this->colorService->getRandomColor(),
                     ]);

lang/en/validation.php

@@ -202,7 +202,7 @@ return [
     'currency' => 'The :attribute field must be a valid currency code (ISO 4217).',
     'organization' => 'The :attribute does not exist.',
     'task_belongs_to_project' => 'The :attribute is not part of the given project.',
-    'project_name_already_exists' => 'A project with the same name already exists in the organization.',
+    'project_name_already_exists' => 'A project with the same name and client already exists in the organization.',
     'tag_name_already_exists' => 'A tag with the same name already exists in the organization.',
     'client_name_already_exists' => 'A client with the same name already exists in the organization.',
     'task_name_already_exists' => 'A task with the same name already exists in the project.',

tests/Unit/Endpoint/Api/V1/ProjectEndpointTest.php

@@ -277,6 +277,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->postJson(route('api.v1.projects.store', [$data->organization->getKey()]), [
             'name' => $projectFake->name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
         ]);
 
@@ -299,6 +300,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
             'name' => $projectFake->name,
             'color' => $projectFake->color,
             'is_billable' => $projectFake->is_billable,
+            'client_id' => null,
             'billable_rate' => $billableRate,
         ]);
 
@@ -309,6 +311,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
             'color' => $projectFake->color,
             'organization_id' => $projectFake->organization_id,
             'is_billable' => $projectFake->is_billable,
+            'client_id' => null,
             'billable_rate' => $billableRate,
         ]);
     }
@@ -328,6 +331,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
             'name' => $projectFake->name,
             'color' => $projectFake->color,
             'is_billable' => $projectFake->is_billable,
+            'client_id' => null,
             'billable_rate' => $billableRate,
         ]);
 
@@ -351,6 +355,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->postJson(route('api.v1.projects.store', [$data->organization->getKey()]), [
             'name' => $projectFake->name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
         ]);
 
@@ -360,6 +365,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
             'name' => $projectFake->name,
             'color' => $projectFake->color,
             'organization_id' => $projectFake->organization_id,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
         ]);
     }
@@ -378,6 +384,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
             'name' => $projectFake->name,
             'color' => $projectFake->color,
             'is_billable' => $projectFake->is_billable,
+            'client_id' => null,
             'estimated_time' => 10000,
         ]);
 
@@ -394,6 +401,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
             'color' => $projectFake->color,
             'organization_id' => $projectFake->organization_id,
             'is_billable' => $projectFake->is_billable,
+            'client_id' => null,
             'estimated_time' => null,
         ]);
     }
@@ -413,6 +421,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
             'name' => $projectFake->name,
             'color' => $projectFake->color,
             'is_billable' => $projectFake->is_billable,
+            'client_id' => null,
             'estimated_time' => 10000,
         ]);
 
@@ -429,11 +438,47 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
             'color' => $projectFake->color,
             'organization_id' => $projectFake->organization_id,
             'is_billable' => $projectFake->is_billable,
+            'client_id' => null,
             'estimated_time' => 10000,
         ]);
     }
 
-    public function test_store_endpoint_fails_if_name_is_already_used_in_organization(): void
+    public function test_store_endpoint_can_create_project_if_project_name_already_exists_in_organization_but_with_different_client(): void
+    {
+        // Arrange
+        $data = $this->createUserWithPermission([
+            'projects:create',
+        ]);
+        $name = 'Project Name';
+        $clientA = Client::factory()->forOrganization($data->organization)->create();
+        $clientB = Client::factory()->forOrganization($data->organization)->create();
+        $projectA = Project::factory()->forOrganization($data->organization)->forClient($clientA)->create([
+            'name' => $name,
+        ]);
+        $projectFake = Project::factory()->forOrganization($data->organization)->make();
+        Passport::actingAs($data->user);
+
+        // Act
+        $response = $this->postJson(route('api.v1.projects.store', [$data->organization->getKey()]), [
+            'name' => $name,
+            'color' => $projectFake->color,
+            'client_id' => $clientB->getKey(),
+            'is_billable' => $projectFake->is_billable,
+        ]);
+
+        // Assert
+        $response->assertStatus(201);
+        $this->assertDatabaseHas(Project::class, [
+            'name' => $name,
+            'client_id' => $clientB->getKey(),
+        ]);
+        $this->assertDatabaseHas(Project::class, [
+            'name' => $name,
+            'client_id' => $clientA->getKey(),
+        ]);
+    }
+
+    public function test_store_endpoint_fails_without_client_if_name_is_already_used_for_project_without_client_in_organization(): void
     {
         // Arrange
         $data = $this->createUserWithPermission([
@@ -450,13 +495,43 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->postJson(route('api.v1.projects.store', [$data->organization->getKey()]), [
             'name' => $name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
         ]);
 
         // Assert
         $response->assertStatus(422);
         $response->assertJsonValidationErrors([
-            'name' => 'A project with the same name already exists in the organization.',
+            'name' => 'A project with the same name and client already exists in the organization.',
+        ]);
+    }
+
+    public function test_store_endpoint_fails_with_client_if_name_is_already_used_for_the_same_client(): void
+    {
+        // Arrange
+        $data = $this->createUserWithPermission([
+            'projects:create',
+        ]);
+        $name = 'Project Name';
+        $client = Client::factory()->forOrganization($data->organization)->create();
+        $project = Project::factory()->forOrganization($data->organization)->forClient($client)->create([
+            'name' => $name,
+        ]);
+        $projectFake = Project::factory()->forOrganization($data->organization)->make();
+        Passport::actingAs($data->user);
+
+        // Act
+        $response = $this->postJson(route('api.v1.projects.store', [$data->organization->getKey()]), [
+            'name' => $name,
+            'color' => $projectFake->color,
+            'client_id' => $client->getKey(),
+            'is_billable' => $projectFake->is_billable,
+        ]);
+
+        // Assert
+        $response->assertStatus(422);
+        $response->assertJsonValidationErrors([
+            'name' => 'A project with the same name and client already exists in the organization.',
         ]);
     }
 
@@ -478,6 +553,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->postJson(route('api.v1.projects.store', [$data->organization->getKey()]), [
             'name' => $name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
         ]);
 
@@ -534,6 +610,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->postJson(route('api.v1.projects.store', [$data->organization->getKey()]), [
             'name' => $projectFake->name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => true,
             'billable_rate' => 10001,
         ]);
@@ -565,6 +642,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [
             'name' => $projectFake->name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
         ]);
 
@@ -585,6 +663,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [
             'name' => $projectFake->name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
         ]);
 
@@ -592,7 +671,43 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response->assertForbidden();
     }
 
-    public function test_update_endpoint_fails_if_name_is_already_used_in_organization(): void
+    public function test_update_endpoint_can_update_project_if_project_name_already_exists_in_organization_but_with_different_client(): void
+    {
+        // Arrange
+        $data = $this->createUserWithPermission([
+            'projects:update',
+        ]);
+        $name = 'Project Name';
+        $clientA = Client::factory()->forOrganization($data->organization)->create();
+        $clientB = Client::factory()->forOrganization($data->organization)->create();
+        $projectWithTheName = Project::factory()->forOrganization($data->organization)->forClient($clientA)->create([
+            'name' => $name,
+        ]);
+        $project = Project::factory()->forOrganization($data->organization)->create();
+        $projectFake = Project::factory()->forOrganization($data->organization)->create();
+        Passport::actingAs($data->user);
+
+        // Act
+        $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [
+            'name' => $name,
+            'color' => $projectFake->color,
+            'client_id' => $clientB->getKey(),
+            'is_billable' => $projectFake->is_billable,
+        ]);
+
+        // Assert
+        $response->assertStatus(200);
+        $this->assertDatabaseHas(Project::class, [
+            'name' => $name,
+            'client_id' => $clientA->getKey(),
+        ]);
+        $this->assertDatabaseHas(Project::class, [
+            'name' => $name,
+            'client_id' => $clientB->getKey(),
+        ]);
+    }
+
+    public function test_update_endpoint_fails_without_client_if_name_is_already_used_for_project_without_client_in_organization(): void
     {
         // Arrange
         $data = $this->createUserWithPermission([
@@ -610,13 +725,44 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [
             'name' => $name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
         ]);
 
         // Assert
         $response->assertStatus(422);
         $response->assertJsonValidationErrors([
-            'name' => 'A project with the same name already exists in the organization.',
+            'name' => 'A project with the same name and client already exists in the organization.',
+        ]);
+    }
+
+    public function test_update_endpoint_fails_with_client_if_name_is_already_used_for_the_same_client(): void
+    {
+        // Arrange
+        $data = $this->createUserWithPermission([
+            'projects:update',
+        ]);
+        $name = 'Project Name';
+        $client = Client::factory()->forOrganization($data->organization)->create();
+        $projectWithTheName = Project::factory()->forOrganization($data->organization)->forClient($client)->create([
+            'name' => $name,
+        ]);
+        $project = Project::factory()->forOrganization($data->organization)->create();
+        $projectFake = Project::factory()->forOrganization($data->organization)->create();
+        Passport::actingAs($data->user);
+
+        // Act
+        $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [
+            'name' => $name,
+            'color' => $projectFake->color,
+            'client_id' => $client->getKey(),
+            'is_billable' => $projectFake->is_billable,
+        ]);
+
+        // Assert
+        $response->assertStatus(422);
+        $response->assertJsonValidationErrors([
+            'name' => 'A project with the same name and client already exists in the organization.',
         ]);
     }
 
@@ -720,6 +866,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [
             'name' => $name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
         ]);
 
@@ -782,6 +929,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [
             'name' => $projectFake->name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
             'estimated_time' => 10000,
         ]);
@@ -815,6 +963,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [
             'name' => $projectFake->name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
             'estimated_time' => 10000,
         ]);
@@ -848,6 +997,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [
             'name' => $projectFake->name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
             'billable_rate' => $project->billable_rate,
         ]);
@@ -880,6 +1030,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [
             'name' => $projectFake->name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
             'billable_rate' => 10003,
         ]);
@@ -907,6 +1058,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [
             'name' => $projectFake->name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
             'is_archived' => true,
         ]);
@@ -935,6 +1087,7 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
         $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [
             'name' => $projectFake->name,
             'color' => $projectFake->color,
+            'client_id' => null,
             'is_billable' => $projectFake->is_billable,
             'is_archived' => false,
         ]);