Skip to content

Commit dccb788

Browse files
snipesnipe
authored
Merge pull request #15691 from marcusmoore/fixes/get-id-for-current-user
Updated `Company::getIdForCurrentUser()` to return null in certain scenarios
2 parents 5e1d792 + d10fe77 commit dccb788

11 files changed

+316
-4
lines changed

‎app/Http/Requests/StoreAssetRequest.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,19 @@ public function authorize(): bool
2626

2727
public function prepareForValidation(): void
2828
{
29+
// Guard against users passing in an array for company_id instead of an integer.
30+
// If the company_id is not an integer then we simply use what was
31+
// provided to be caught by model level validation later.
32+
// The use of is_numeric accounts for 1 and '1'.
33+
$idForCurrentUser = is_numeric($this->company_id)
34+
? Company::getIdForCurrentUser($this->company_id)
35+
: $this->company_id;
36+
2937
$this->parseLastAuditDate();
3038

3139
$this->merge([
3240
'asset_tag' => $this->asset_tag ?? Asset::autoincrement_asset(),
33-
'company_id' => Company::getIdForCurrentUser($this->company_id),
41+
'company_id' => $idForCurrentUser,
3442
'assigned_to' => $assigned_to ?? null,
3543
]);
3644
}

‎app/Models/Company.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ public static function getIdForCurrentUser($unescaped_input)
116116
if ($current_user->company_id != null) {
117117
return $current_user->company_id;
118118
} else {
119-
return static::getIdFromInput($unescaped_input);
119+
return null;
120120
}
121121
}
122122
}

‎tests/Feature/Accessories/Ui/CreateAccessoryWithFullMultipleCompanySupportTest.php‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
namespace Tests\Feature\Accessories\Ui;
4+
5+
use App\Models\Accessory;
6+
use App\Models\Category;
7+
use PHPUnit\Framework\Attributes\DataProvider;
8+
use Tests\Support\ProvidesDataForFullMultipleCompanySupportTesting;
9+
use Tests\TestCase;
10+
11+
class CreateAccessoryWithFullMultipleCompanySupportTest extends TestCase
12+
{
13+
use ProvidesDataForFullMultipleCompanySupportTesting;
14+
15+
#[DataProvider('dataForFullMultipleCompanySupportTesting')]
16+
public function testAdheresToFullMultipleCompaniesSupportScoping($data)
17+
{
18+
['actor' => $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data();
19+
20+
$this->settings->enableMultipleFullCompanySupport();
21+
22+
$this->actingAs($actor)
23+
->post(route('accessories.store'), [
24+
'redirect_option' => 'index',
25+
'name' => 'My Cool Accessory',
26+
'qty' => '1',
27+
'category_id' => Category::factory()->create()->id,
28+
'company_id' => $company->id,
29+
]);
30+
31+
$accessory = Accessory::withoutGlobalScopes()->where([
32+
'name' => 'My Cool Accessory',
33+
])->sole();
34+
35+
$assertions($accessory);
36+
}
37+
}

‎tests/Feature/Assets/Api/StoreAssetTest.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,9 @@ public function testAnAssetCanBeCheckedOutToAssetOnStore()
560560
$this->assertTrue($asset->assignedAssets()->find($response['payload']['id'])->is($apiAsset));
561561
}
562562

563+
/**
564+
* @link https://app.shortcut.com/grokability/story/24475
565+
*/
563566
public function testCompanyIdNeedsToBeInteger()
564567
{
565568
$this->actingAsForApi(User::factory()->createAssets()->create())

‎tests/Feature/Assets/Api/StoreAssetWithFullMultipleCompanySupportTest.php‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
namespace Tests\Feature\Assets\Api;
4+
5+
use App\Models\Asset;
6+
use App\Models\AssetModel;
7+
use App\Models\Statuslabel;
8+
use PHPUnit\Framework\Attributes\DataProvider;
9+
use Tests\Support\ProvidesDataForFullMultipleCompanySupportTesting;
10+
use Tests\TestCase;
11+
12+
class StoreAssetWithFullMultipleCompanySupportTest extends TestCase
13+
{
14+
use ProvidesDataForFullMultipleCompanySupportTesting;
15+
16+
/**
17+
* @link https://github.com/snipe/snipe-it/issues/15654
18+
*/
19+
#[DataProvider('dataForFullMultipleCompanySupportTesting')]
20+
public function testAdheresToFullMultipleCompaniesSupportScoping($data)
21+
{
22+
['actor' => $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data();
23+
24+
$this->settings->enableMultipleFullCompanySupport();
25+
26+
$response = $this->actingAsForApi($actor)
27+
->postJson(route('api.assets.store'), [
28+
'asset_tag' => 'random_string',
29+
'company_id' => $company->id,
30+
'model_id' => AssetModel::factory()->create()->id,
31+
'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id,
32+
]);
33+
34+
$asset = Asset::withoutGlobalScopes()->findOrFail($response['payload']['id']);
35+
36+
$assertions($asset);
37+
}
38+
39+
#[DataProvider('dataForFullMultipleCompanySupportTesting')]
40+
public function testHandlesCompanyIdBeingString($data)
41+
{
42+
['actor' => $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data();
43+
44+
$this->settings->enableMultipleFullCompanySupport();
45+
46+
$response = $this->actingAsForApi($actor)
47+
->postJson(route('api.assets.store'), [
48+
'asset_tag' => 'random_string',
49+
'company_id' => (string) $company->id,
50+
'model_id' => AssetModel::factory()->create()->id,
51+
'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id,
52+
]);
53+
54+
$asset = Asset::withoutGlobalScopes()->findOrFail($response['payload']['id']);
55+
56+
$assertions($asset);
57+
}
58+
}

‎tests/Feature/Assets/Ui/StoreAssetWithFullMultipleCompanySupportTest.php‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
namespace Tests\Feature\Assets\Ui;
4+
5+
use App\Models\Asset;
6+
use App\Models\AssetModel;
7+
use App\Models\Statuslabel;
8+
use PHPUnit\Framework\Attributes\DataProvider;
9+
use Tests\Support\ProvidesDataForFullMultipleCompanySupportTesting;
10+
use Tests\TestCase;
11+
12+
class StoreAssetWithFullMultipleCompanySupportTest extends TestCase
13+
{
14+
use ProvidesDataForFullMultipleCompanySupportTesting;
15+
16+
#[DataProvider('dataForFullMultipleCompanySupportTesting')]
17+
public function testAdheresToFullMultipleCompaniesSupportScoping($data)
18+
{
19+
['actor' => $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data();
20+
21+
$this->settings->enableMultipleFullCompanySupport();
22+
23+
$this->actingAs($actor)
24+
->post(route('hardware.store'), [
25+
'asset_tags' => ['1' => '1234'],
26+
'model_id' => AssetModel::factory()->create()->id,
27+
'status_id' => Statuslabel::factory()->create()->id,
28+
'company_id' => $company->id,
29+
]);
30+
31+
$asset = Asset::where('asset_tag', '1234')->sole();
32+
33+
$assertions($asset);
34+
}
35+
}

‎tests/Feature/Components/Ui/StoreComponentWithFullMultipleCompanySupportTest.php‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
namespace Tests\Feature\Components\Ui;
4+
5+
use App\Models\Category;
6+
use App\Models\Component;
7+
use PHPUnit\Framework\Attributes\DataProvider;
8+
use Tests\Support\ProvidesDataForFullMultipleCompanySupportTesting;
9+
use Tests\TestCase;
10+
11+
class StoreComponentWithFullMultipleCompanySupportTest extends TestCase
12+
{
13+
use ProvidesDataForFullMultipleCompanySupportTesting;
14+
15+
#[DataProvider('dataForFullMultipleCompanySupportTesting')]
16+
public function testAdheresToFullMultipleCompaniesSupportScoping($data)
17+
{
18+
['actor' => $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data();
19+
20+
$this->settings->enableMultipleFullCompanySupport();
21+
22+
$this->actingAs($actor)
23+
->post(route('components.store'), [
24+
'name' => 'My Cool Component',
25+
'qty' => '1',
26+
'category_id' => Category::factory()->create()->id,
27+
'company_id' => $company->id,
28+
]);
29+
30+
$component = Component::where('name', 'My Cool Component')->sole();
31+
32+
$assertions($component);
33+
}
34+
}

‎tests/Feature/Consumables/Ui/StoreConsumableWithFullMultipleCompanySupportTest.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
namespace Tests\Feature\Consumables\Ui;
4+
5+
use App\Models\Category;
6+
use App\Models\Consumable;
7+
use PHPUnit\Framework\Attributes\DataProvider;
8+
use Tests\Support\ProvidesDataForFullMultipleCompanySupportTesting;
9+
use Tests\TestCase;
10+
11+
class StoreConsumableWithFullMultipleCompanySupportTest extends TestCase
12+
{
13+
use ProvidesDataForFullMultipleCompanySupportTesting;
14+
15+
#[DataProvider('dataForFullMultipleCompanySupportTesting')]
16+
public function testAdheresToFullMultipleCompaniesSupportScoping($data)
17+
{
18+
['actor' => $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data();
19+
20+
$this->settings->enableMultipleFullCompanySupport();
21+
22+
$this->actingAs($actor)
23+
->post(route('consumables.store'), [
24+
'name' => 'My Cool Consumable',
25+
'category_id' => Category::factory()->forConsumables()->create()->id,
26+
'company_id' => $company->id,
27+
]);
28+
29+
$consumable = Consumable::where('name', 'My Cool Consumable')->sole();
30+
31+
$assertions($consumable);
32+
}
33+
}

‎tests/Feature/Licenses/Ui/StoreLicenseWithFullMultipleCompanySupportTest.php‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
namespace Tests\Feature\Licenses\Ui;
4+
5+
use App\Models\Category;
6+
use App\Models\License;
7+
use PHPUnit\Framework\Attributes\DataProvider;
8+
use Tests\Support\ProvidesDataForFullMultipleCompanySupportTesting;
9+
use Tests\TestCase;
10+
11+
class StoreLicenseWithFullMultipleCompanySupportTest extends TestCase
12+
{
13+
use ProvidesDataForFullMultipleCompanySupportTesting;
14+
15+
#[DataProvider('dataForFullMultipleCompanySupportTesting')]
16+
public function testAdheresToFullMultipleCompaniesSupportScoping($data)
17+
{
18+
['actor' => $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data();
19+
20+
$this->settings->enableMultipleFullCompanySupport();
21+
22+
$this->actingAs($actor)
23+
->post(route('licenses.store'), [
24+
'name' => 'My Cool License',
25+
'seats' => '1',
26+
'category_id' => Category::factory()->forLicenses()->create()->id,
27+
'company_id' => $company->id,
28+
]);
29+
30+
$license = License::where('name', 'My Cool License')->sole();
31+
32+
$assertions($license);
33+
}
34+
}

‎tests/Support/ProvidesDataForFullMultipleCompanySupportTesting.php‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<?php
2+
3+
namespace Tests\Support;
4+
5+
use App\Models\Company;
6+
use App\Models\User;
7+
use Generator;
8+
9+
trait ProvidesDataForFullMultipleCompanySupportTesting
10+
{
11+
public static function dataForFullMultipleCompanySupportTesting(): Generator
12+
{
13+
yield "User in a company should result in user's company_id being used" => [
14+
function () {
15+
$jedi = Company::factory()->create();
16+
$sith = Company::factory()->create();
17+
$luke = User::factory()->for($jedi)
18+
->createAccessories()
19+
->createAssets()
20+
->createComponents()
21+
->createConsumables()
22+
->createLicenses()
23+
->create();
24+
25+
return [
26+
'actor' => $luke,
27+
'company_attempting_to_associate' => $sith,
28+
'assertions' => function ($model) use ($jedi) {
29+
self::assertEquals($jedi->id, $model->company_id);
30+
},
31+
];
32+
}
33+
];
34+
35+
yield "User without a company should result in company_id being null" => [
36+
function () {
37+
$userInNoCompany = User::factory()
38+
->createAccessories()
39+
->createAssets()
40+
->createComponents()
41+
->createConsumables()
42+
->createLicenses()
43+
->create(['company_id' => null]);
44+
45+
return [
46+
'actor' => $userInNoCompany,
47+
'company_attempting_to_associate' => Company::factory()->create(),
48+
'assertions' => function ($model) {
49+
self::assertNull($model->company_id);
50+
},
51+
];
52+
}
53+
];
54+
55+
yield "Super-User assigning across companies should result in company_id being set to what was provided" => [
56+
function () {
57+
$superUser = User::factory()->superuser()->create(['company_id' => null]);
58+
$company = Company::factory()->create();
59+
60+
return [
61+
'actor' => $superUser,
62+
'company_attempting_to_associate' => $company,
63+
'assertions' => function ($model) use ($company) {
64+
self::assertEquals($model->company_id, $company->id);
65+
},
66+
];
67+
}
68+
];
69+
}
70+
}

0 commit comments

Comments
 (0)