Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added #2353: Add ability to tie locations to companies - 2023 edition #12577

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
43 changes: 43 additions & 0 deletions app/Console/Commands/TestLocationsFMCS.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php

namespace App\Console\Commands;

use App\Helpers\Helper;
use Illuminate\Console\Command;

class TestLocationsFMCS extends Command
{
/**
* The name and signature of the console command.
*
* @var string
*/
protected $signature = 'snipeit:test-locations-fmcs {--location_id=}';

/**
* The console command description.
*
* @var string
*/
protected $description = 'Test for inconsistencies if FullMultipleCompanySupport with scoped locations will be used';

/**
* Execute the console command.
*/
public function handle()
{
$this->info('Test for inconsistencies if FullMultipleCompanySupport with scoped locations will be used');
$this->info('Depending on the database size this will take a while, output will be displayed after the complete test is over');

// if parameter location_id is set, only test this location
$location_id = null;
if ($this->option('location_id')) {
$location_id = $this->option('location_id');
}
$ret = Helper::test_locations_fmcs(true, $location_id);

foreach($ret as $output) {
$this->info($output);
}
}
}
65 changes: 65 additions & 0 deletions app/Helpers/Helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use App\Models\Setting;
use App\Models\Statuslabel;
use App\Models\License;
use App\Models\Location;
use Illuminate\Support\Facades\Crypt;
use Illuminate\Contracts\Encryption\DecryptException;
use Carbon\Carbon;
Expand Down Expand Up @@ -1529,4 +1530,68 @@ static public function getRedirectOption($request, $id, $table, $item_id = null)
}
return redirect()->back()->with('error', trans('admin/hardware/message.checkout.error'));
}

/**
* Check for inconsistencies before activating scoped locations with FullMultipleCompanySupport
* If there are locations with different companies than related objects unforseen problems could arise
*
* @author T. Regnery <[email protected]>
* @since 7.0
*
* @param $artisan when false, bail out on first inconsistent entry
* @param $location_id when set, only test this specific location
* @param $new_company_id in case of updating a location, this is the newly requested company_id
* @return string []
*/
static public function test_locations_fmcs($artisan, $location_id = null, $new_company_id = null) {
$ret = [];

if ($location_id) {
$location = Location::find($location_id);
if ($location) {
$locations = collect([])->push(Location::find($location_id));
}
} else {
$locations = Location::all();
}

foreach($locations as $location) {
// in case of an update of a single location use the newly requested company_id
if ($new_company_id) {
$location_company = $new_company_id;
} else {
$location_company = $location->company_id;
}

// depending on the relationship we must use different operations to retrieve the objects
$keywords_relation = ['many' => ['users', 'assets', 'rtd_assets', 'consumables', 'components', 'accessories', 'assignedAssets', 'assignedAccessories'],
'one' => ['parent', 'manager']];

// In case of a single location the children must be checked either, becuase we don't walk every location
if ($location_id) {
$keywords_relation['many'][] = 'children';
}

foreach ($keywords_relation as $relation => $keywords) {
foreach($keywords as $keyword) {
if ($relation == 'many') {
$items = $location->$keyword->all();
} else {
$items = collect([])->push($location->$keyword);
}

foreach ($items as $item) {
if ($item && $item->company_id != $location_company) {
$ret[] = 'type: ' . get_class($item) . ', id: ' . $item->id . ', company_id: ' . $item->company_id . ', location company_id: ' . $location_company;
// when not called from artisan command we bail out on the first error
if (!$artisan) {
return $ret;
}
}
}
}
}
}
return $ret;
}
}
46 changes: 45 additions & 1 deletion app/Http/Controllers/Api/LocationsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
use App\Models\Accessory;
use App\Models\AccessoryCheckout;
use App\Models\Asset;
use App\Models\Company;
use App\Models\Location;
use App\Models\Setting;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request;
use Illuminate\Pagination\LengthAwarePaginator;
Expand Down Expand Up @@ -46,6 +48,7 @@ public function index(Request $request) : JsonResponse | array
'id',
'image',
'ldap_ou',
'company_id',
'manager_id',
'name',
'rtd_assets_count',
Expand Down Expand Up @@ -73,7 +76,9 @@ public function index(Request $request) : JsonResponse | array
'locations.image',
'locations.ldap_ou',
'locations.currency',
'locations.company_id',
])
->withCount('assignedAssets as assigned_assets_count')
->withCount('assignedAssets as assigned_assets_count')
->withCount('assets as assets_count')
->withCount('assignedAccessories as assigned_accessories_count')
Expand All @@ -82,6 +87,11 @@ public function index(Request $request) : JsonResponse | array
->withCount('children as children_count')
->withCount('users as users_count');

// Only scope locations if the setting is enabled
if (Setting::getSettings()->scope_locations_fmcs) {
$locations = Company::scopeCompanyables($locations);
}
Comment on lines +90 to +93
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be wrapped in the following like we have in other places?

// Only scope the location if the setting is enabled
if (Setting::getSettings()->scope_locations_fmcs) {
    $location->company_id = Company::getIdForCurrentUser($request->input('company_id'));
} else {
    $location->company_id = $request->input('company_id');
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is in line with other scoped controllers and not about the company_id or am I misunderstanding something?


if ($request->filled('search')) {
$locations = $locations->TextSearch($request->input('search'));
}
Expand Down Expand Up @@ -114,6 +124,10 @@ public function index(Request $request) : JsonResponse | array
$locations->where('locations.manager_id', '=', $request->input('manager_id'));
}

if ($request->filled('company_id')) {
$locations->where('locations.company_id', '=', $request->input('company_id'));
}

// Make sure the offset and limit are actually integers and do not exceed system limits
$offset = ($request->input('offset') > $locations->count()) ? $locations->count() : app('api_offset_value');
$limit = app('api_limit_value');
Expand All @@ -130,6 +144,9 @@ public function index(Request $request) : JsonResponse | array
case 'manager':
$locations->OrderManager($order);
break;
case 'company':
$locations->OrderCompany($order);
break;
default:
$locations->orderBy($sort, $order);
break;
Expand Down Expand Up @@ -157,6 +174,15 @@ public function store(ImageUploadRequest $request) : JsonResponse
$location->fill($request->all());
$location = $request->handleImages($location);

// Only scope location if the setting is enabled
if (Setting::getSettings()->scope_locations_fmcs) {
$location->company_id = Company::getIdForCurrentUser($request->get('company_id'));
// check if parent is set and has a different company
if ($location->parent_id && Location::find($location->parent_id)->company_id != $location->company_id) {
response()->json(Helper::formatStandardApiResponse('error', null, 'different company than parent'));
}
}

if ($location->save()) {
return response()->json(Helper::formatStandardApiResponse('success', (new LocationsTransformer)->transformLocation($location), trans('admin/locations/message.create.success')));
}
Expand All @@ -174,7 +200,7 @@ public function store(ImageUploadRequest $request) : JsonResponse
public function show($id) : JsonResponse | array
{
$this->authorize('view', Location::class);
$location = Location::with('parent', 'manager', 'children')
$location = Location::with('parent', 'manager', 'children', 'company')
->select([
'locations.id',
'locations.name',
Expand Down Expand Up @@ -217,6 +243,19 @@ public function update(ImageUploadRequest $request, $id) : JsonResponse
$location->fill($request->all());
$location = $request->handleImages($location);

if ($request->filled('company_id')) {
// Only scope location if the setting is enabled
if (Setting::getSettings()->scope_locations_fmcs) {
$location->company_id = Company::getIdForCurrentUser($request->get('company_id'));
// check if there are related objects with different company
if (Helper::test_locations_fmcs(false, $id, $location->company_id)) {
return response()->json(Helper::formatStandardApiResponse('error', null, 'error scoped locations'));
}
} else {
$location->company_id = $request->get('company_id');
}
}

if ($location->isValid()) {

$location->save();
Expand Down Expand Up @@ -337,6 +376,11 @@ public function selectlist(Request $request) : array
'locations.image',
]);

// Only scope locations if the setting is enabled
if (Setting::getSettings()->scope_locations_fmcs) {
$locations = Company::scopeCompanyables($locations);
}

$page = 1;
if ($request->filled('page')) {
$page = $request->input('page');
Expand Down
50 changes: 42 additions & 8 deletions app/Http/Controllers/LocationsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@

namespace App\Http\Controllers;

use App\Helpers\Helper;
use App\Http\Requests\ImageUploadRequest;
use App\Models\Actionlog;
use App\Models\Asset;
use App\Models\Company;
use App\Models\Location;
use App\Models\Setting;
use App\Models\User;
use Illuminate\Support\Facades\Storage;
use Illuminate\Http\Request;
Expand Down Expand Up @@ -78,6 +81,18 @@ public function store(ImageUploadRequest $request) : RedirectResponse
$location->created_by = auth()->id();
$location->phone = request('phone');
$location->fax = request('fax');
$location->company_id = Company::getIdForCurrentUser($request->input('company_id'));

// Only scope the location if the setting is enabled
if (Setting::getSettings()->scope_locations_fmcs) {
$location->company_id = Company::getIdForCurrentUser($request->input('company_id'));
// check if parent is set and has a different company
if ($location->parent_id && Location::find($location->parent_id)->company_id != $location->company_id) {
return redirect()->back()->withInput()->withInput()->with('error', 'different company than parent');
}
} else {
$location->company_id = $request->input('company_id');
}

$location = $request->handleImages($location);

Expand Down Expand Up @@ -139,6 +154,17 @@ public function update(ImageUploadRequest $request, $locationId = null) : Redire
$location->ldap_ou = $request->input('ldap_ou');
$location->manager_id = $request->input('manager_id');

// Only scope the location if the setting is enabled
if (Setting::getSettings()->scope_locations_fmcs) {
$location->company_id = Company::getIdForCurrentUser($request->input('company_id'));
// check if there are related objects with different company
if (Helper::test_locations_fmcs(false, $locationId, $location->company_id)) {
return redirect()->back()->withInput()->withInput()->with('error', 'error scoped locations');
}
} else {
$location->company_id = $request->input('company_id');
}

$location = $request->handleImages($location);

if ($location->save()) {
Expand Down Expand Up @@ -211,20 +237,22 @@ public function show($id = null) : View | RedirectResponse

public function print_assigned($id) : View | RedirectResponse
{

if ($location = Location::where('id', $id)->first()) {
$parent = Location::where('id', $location->parent_id)->first();
$manager = User::where('id', $location->manager_id)->first();
$company = Company::where('id', $location->company_id)->first();
$users = User::where('location_id', $id)->with('company', 'department', 'location')->get();
$assets = Asset::where('assigned_to', $id)->where('assigned_type', Location::class)->with('model', 'model.category')->get();
return view('locations/print')->with('assets', $assets)->with('users', $users)->with('location', $location)->with('parent', $parent)->with('manager', $manager);

return view('locations/print')
->with('assets', $assets)
->with('users',$users)
->with('location', $location)
->with('parent', $parent)
->with('manager', $manager)
->with('company', $company);
}

return redirect()->route('locations.index')->with('error', trans('admin/locations/message.does_not_exist'));



}


Expand Down Expand Up @@ -296,10 +324,16 @@ public function print_all_assigned($id) : View | RedirectResponse
if ($location = Location::where('id', $id)->first()) {
$parent = Location::where('id', $location->parent_id)->first();
$manager = User::where('id', $location->manager_id)->first();
$company = Company::where('id', $location->company_id)->first();
$users = User::where('location_id', $id)->with('company', 'department', 'location')->get();
$assets = Asset::where('location_id', $id)->with('model', 'model.category')->get();
return view('locations/print')->with('assets', $assets)->with('users', $users)->with('location', $location)->with('parent', $parent)->with('manager', $manager);

return view('locations/print')
->with('assets', $assets)
->with('users',$users)
->with('location', $location)
->with('parent', $parent)
->with('manager', $manager)
->with('company', $company);
}
return redirect()->route('locations.index')->with('error', trans('admin/locations/message.does_not_exist'));
}
Expand Down
16 changes: 16 additions & 0 deletions app/Http/Controllers/SettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,23 @@ public function postSettings(Request $request) : RedirectResponse
$setting->modellist_displays = implode(',', $request->input('show_in_model_list'));
}

$old_locations_fmcs = $setting->scope_locations_fmcs;
$setting->full_multiple_companies_support = $request->input('full_multiple_companies_support', '0');
$setting->scope_locations_fmcs = $request->input('scope_locations_fmcs', '0');

// Backward compatibility for locations makes no sense without FullMultipleCompanySupport
if (!$setting->full_multiple_companies_support) {
$setting->scope_locations_fmcs = '0';
}
Comment on lines +321 to +324
Copy link
Collaborator

Choose a reason for hiding this comment

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

This makes sense but might confuse some users. I wonder if there is a way to concisely express this on the settings page? 🤔 // @snipe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this can be handled in the view with some javascript. If this is a problem I can think about it some more, but my experience with this is quite limited.


// check for inconsistencies when activating scoped locations
if ($old_locations_fmcs == '0' && $setting->scope_locations_fmcs == '1') {
$ret = Helper::test_locations_fmcs(false);
if (count($ret) != 0) {
return redirect()->back()->withInput()->with('error', 'Inconsistencies with scoped locations found, please use php artisan snipeit:test-locations-fmcs for details');
}
}

$setting->unique_serial = $request->input('unique_serial', '0');
$setting->shortcuts_enabled = $request->input('shortcuts_enabled', '0');
$setting->show_images_in_email = $request->input('show_images_in_email', '0');
Expand Down
4 changes: 4 additions & 0 deletions app/Http/Transformers/LocationsTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ public function transformLocation(Location $location = null)
'name'=> e($location->parent->name),
] : null,
'manager' => ($location->manager) ? (new UsersTransformer)->transformUser($location->manager) : null,
'company' => ($location->company) ? [
'id' => (int) $location->company->id,
'name'=> e($location->company->name)
] : null,

'children' => $children_arr,
];
Expand Down
1 change: 1 addition & 0 deletions app/Models/Accessory.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class Accessory extends SnipeModel
'qty' => 'required|integer|min:1',
'category_id' => 'required|integer|exists:categories,id',
'company_id' => 'integer|nullable',
'location_id' => 'exists:locations,id|nullable|fmcs_location',
'min_amt' => 'integer|min:0|nullable',
'purchase_cost' => 'numeric|nullable|gte:0|max:9999999999999',
'purchase_date' => 'date_format:Y-m-d|nullable',
Expand Down
Loading
Loading