mirror of
https://github.com/snipe/snipe-it.git
synced 2026-08-18 11:15:42 +00:00
Merge branch 'develop' into calendar-ui
This commit is contained in:
snipe
GitHub
@ -159,11 +159,15 @@ class AccessoriesController extends Controller
|
||||
// create-form input names so enrichInitialOrderFromRequest
|
||||
// in store() picks them up on save and writes them onto the
|
||||
// observer-created initial Order + OrderItem for the new row.
|
||||
foreach ($accessory->lastOrderPrefill() as $field => $value) {
|
||||
if ($value !== null) {
|
||||
$cloned->{$field} = $value;
|
||||
}
|
||||
}
|
||||
// Explicit assignments (not a foreach) so each typed value from
|
||||
// lastOrderPrefill() lands on the matching typed model property
|
||||
// without going through a mixed intermediate that would fail
|
||||
// larastan's assign.propertyType check.
|
||||
$prefill = $accessory->lastOrderPrefill();
|
||||
$cloned->supplier_id = $prefill['supplier_id'];
|
||||
$cloned->purchase_date = $prefill['purchase_date'];
|
||||
$cloned->purchase_cost = $prefill['purchase_cost'];
|
||||
$cloned->order_number = $prefill['order_number'];
|
||||
|
||||
return view('accessories/edit')
|
||||
->with('cloned_model', $accessory_to_clone)
|
||||
|
||||
@ -252,14 +252,13 @@ class ComponentsController extends Controller
|
||||
$cloned_component->id = null;
|
||||
$cloned_component->deleted_at = null;
|
||||
|
||||
// See AccessoriesController::getClone — same rationale for
|
||||
// carrying the source item's most recent acquisition context
|
||||
// onto the cloned create form.
|
||||
foreach ($component->lastOrderPrefill() as $field => $value) {
|
||||
if ($value !== null) {
|
||||
$cloned_component->{$field} = $value;
|
||||
}
|
||||
}
|
||||
// See AccessoriesController::getClone for the rationale, including
|
||||
// the note on why these are explicit assignments not a foreach.
|
||||
$prefill = $component->lastOrderPrefill();
|
||||
$cloned_component->supplier_id = $prefill['supplier_id'];
|
||||
$cloned_component->purchase_date = $prefill['purchase_date'];
|
||||
$cloned_component->purchase_cost = $prefill['purchase_cost'];
|
||||
$cloned_component->order_number = $prefill['order_number'];
|
||||
|
||||
// Show the page
|
||||
return view('components/edit')
|
||||
|
||||
@ -255,14 +255,13 @@ class ConsumablesController extends Controller
|
||||
$consumable->id = null;
|
||||
$consumable->created_by = null;
|
||||
|
||||
// See AccessoriesController::getClone — same rationale for
|
||||
// carrying the source item's most recent acquisition context
|
||||
// onto the cloned create form.
|
||||
foreach ($consumable_to_close->lastOrderPrefill() as $field => $value) {
|
||||
if ($value !== null) {
|
||||
$consumable->{$field} = $value;
|
||||
}
|
||||
}
|
||||
// See AccessoriesController::getClone for the rationale, including
|
||||
// the note on why these are explicit assignments not a foreach.
|
||||
$prefill = $consumable_to_close->lastOrderPrefill();
|
||||
$consumable->supplier_id = $prefill['supplier_id'];
|
||||
$consumable->purchase_date = $prefill['purchase_date'];
|
||||
$consumable->purchase_cost = $prefill['purchase_cost'];
|
||||
$consumable->order_number = $prefill['order_number'];
|
||||
|
||||
return view('consumables/edit')
|
||||
->with('cloned_model', $consumable_to_close)
|
||||
|
||||
@ -255,8 +255,35 @@ class SnipeMutableCollection extends MutableCollection
|
||||
// stash the object into the request so the displayName uniqueness closure
|
||||
// (which re-runs after mapping) can recognize its own row instead of
|
||||
// treating it as an existing name collision.
|
||||
//
|
||||
// Missing-value guard: the per-member `required` rule that used to live on
|
||||
// the SCIM config was dropped because it caused ValidationRuleParser to
|
||||
// allocate O(N) rule stacks on the flattened payload, which OOMed on
|
||||
// large group syncs (see the docblock above the members mapping in
|
||||
// SnipeSCIMConfig::getGroupConfig). The check now happens here in a
|
||||
// single walk so clients get a clean 400 pointing at the bad indices
|
||||
// instead of the parent library's misleading 500 with an empty
|
||||
// "One or more members are unknown: " message from findMany() eating
|
||||
// the nulls.
|
||||
public function add($value, Model &$object)
|
||||
{
|
||||
$missing = [];
|
||||
foreach ((array) $value as $index => $entry) {
|
||||
if (!is_array($entry)
|
||||
|| !array_key_exists('value', $entry)
|
||||
|| $entry['value'] === null
|
||||
|| $entry['value'] === ''
|
||||
) {
|
||||
$missing[] = $index;
|
||||
}
|
||||
}
|
||||
if ($missing !== []) {
|
||||
throw new SCIMException(
|
||||
'Every members entry must include a "value" field. Missing at indices: ' . implode(',', $missing),
|
||||
400
|
||||
);
|
||||
}
|
||||
|
||||
if (! $object->exists) {
|
||||
$object->save();
|
||||
request()->attributes->set('scim_in_flight_resource', $object);
|
||||
@ -413,19 +440,19 @@ class SCIMMultiCompanyArray extends Attribute
|
||||
|
||||
public function add($value, Model &$object)
|
||||
{
|
||||
\Log::debug("MC ADD VALUE IS: " . print_r($value, true));
|
||||
\Log::debug('MC ADD VALUE IS: ' . print_r($value, true));
|
||||
$this->applyCompanies($value, $object);
|
||||
}
|
||||
|
||||
public function replace($value, Model &$object, $path = null, $removeIfNotSet = false)
|
||||
{
|
||||
\Log::debug("MC REPLACE VALUE IS: " . print_r($value, true));
|
||||
\Log::debug('MC REPLACE VALUE IS: ' . print_r($value, true));
|
||||
$this->applyCompanies($value, $object);
|
||||
}
|
||||
|
||||
public function patch($operation, $value, Model &$object, ?Path $path = null, $removeIfNotSet = false)
|
||||
{
|
||||
\Log::debug("MC PATCH VALUE IS: " . print_r($value, true));
|
||||
\Log::debug('MC PATCH VALUE IS: ' . print_r($value, true));
|
||||
$this->applyCompanies($value, $object);
|
||||
}
|
||||
}
|
||||
@ -710,7 +737,7 @@ class SnipeSCIMConfig
|
||||
} else {
|
||||
// Okta hits this one for creating a user - it does a full PUT for their ID
|
||||
\Log::debug("GetValuePAthFilter is null for path: $path");
|
||||
\Log::debug("GetValuePathFilter is now null and trying to set value of: " . print_r($value, true));
|
||||
\Log::debug('GetValuePathFilter is now null and trying to set value of: ' . print_r($value, true));
|
||||
// the Addresses object is a 'list' (array with numeric indices) by definition...
|
||||
if (is_array($value) && array_is_list($value)) {
|
||||
foreach ($value as $address) {
|
||||
@ -718,18 +745,18 @@ class SnipeSCIMConfig
|
||||
if (@$address['type'] == 'work') {
|
||||
foreach ($address as $key => $v) {
|
||||
if (array_key_exists($key, self::$addressmap)) {
|
||||
\Log::debug("Addresses: Setting " . self::$addressmap[$key] . " to '$v'");
|
||||
\Log::debug('Addresses: Setting ' . self::$addressmap[$key] . " to '$v'");
|
||||
$object->{self::$addressmap[$key]} = $v;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
//should we throw if you give us a 'home' address? I don't know.
|
||||
// should we throw if you give us a 'home' address? I don't know.
|
||||
// what if you gave us _both_ ?
|
||||
}
|
||||
}
|
||||
} else {
|
||||
\Log::debug("Unknown Address Object: " . print_r($value, true));
|
||||
throw new SCIMException("Unknown Address object of type: " . gettype($value), 422);
|
||||
\Log::debug('Unknown Address Object: ' . print_r($value, true));
|
||||
throw new SCIMException('Unknown Address object of type: ' . gettype($value), 422);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -892,8 +919,18 @@ class SnipeSCIMConfig
|
||||
}
|
||||
$fail('The name has already been taken.');
|
||||
}),
|
||||
// The per-member `required` rule on `value` used to live
|
||||
// on the eloquent() below. Removed intentionally: Laravel's
|
||||
// ValidationRuleParser::mergeRulesForAttribute allocates one
|
||||
// rule stack per attribute path in the flattened payload, so
|
||||
// an incoming members array of N entries produced O(N) rule
|
||||
// stacks and blew the PHP memory_limit on large group syncs
|
||||
// The per-member value check now lives inside
|
||||
// SnipeMutableCollection::add() as one array walk, and the
|
||||
// parent ensure() below adds a `max:` guardrail so a truly
|
||||
// runaway payload still gets rejected with a clean 400.
|
||||
(new SnipeMutableCollection('members'))->withSubAttributes(
|
||||
eloquent('value', 'id')->ensure('required'),
|
||||
eloquent('value', 'id'),
|
||||
(new class('$ref') extends Eloquent
|
||||
{
|
||||
protected function doRead(&$object, $attributes = [])
|
||||
@ -908,7 +945,7 @@ class SnipeSCIMConfig
|
||||
}
|
||||
}),
|
||||
eloquent('display', 'name')
|
||||
)->ensure('nullable', 'array')
|
||||
)->ensure('nullable', 'array', 'max:200000')
|
||||
)
|
||||
),
|
||||
];
|
||||
|
||||
@ -143,9 +143,7 @@ trait HasOrders
|
||||
* Prefill values for the create / clone form's initial-acquisition
|
||||
* fields. Distinct from lastOrderDefaults() because this shape
|
||||
* includes `order_number` (per-shipment, not a "default" concept)
|
||||
* and matches the request keys the create form posts back, so a
|
||||
* controller can loop the return array to assign values directly
|
||||
* onto a cloned model's attributes.
|
||||
* and matches the request keys the create form posts back.
|
||||
*
|
||||
* Used by getClone() on Accessories / Consumables / Components to
|
||||
* carry the source item's most recent acquisition context onto the
|
||||
@ -154,26 +152,35 @@ trait HasOrders
|
||||
* / price for a fast restock. Items with no order history return
|
||||
* an all-null array.
|
||||
*
|
||||
* Returns native types (Carbon for purchase_date, float for
|
||||
* purchase_cost) so getClone() can assign directly onto typed
|
||||
* model properties without a coercion step and without tripping
|
||||
* larastan's assign.propertyType on the cast-inferred property
|
||||
* signatures. The @var below is needed because MorphMany::first()
|
||||
* infers to Model|null; the annotation resolves $line to OrderItem
|
||||
* so `->order` and `->price` accesses type through cleanly rather
|
||||
* than falling into the Model::$order baseline ignore bucket.
|
||||
*
|
||||
* @return array{
|
||||
* supplier_id: ?int,
|
||||
* purchase_date: ?string,
|
||||
* purchase_cost: ?string,
|
||||
* purchase_date: ?\Carbon\Carbon,
|
||||
* purchase_cost: ?float,
|
||||
* order_number: ?string,
|
||||
* }
|
||||
*/
|
||||
public function lastOrderPrefill(): array
|
||||
{
|
||||
/** @var \App\Models\OrderItem|null $line */
|
||||
$line = $this->orderItems()
|
||||
->with('order:id,order_number,supplier_id,purchase_date')
|
||||
->latest('id')
|
||||
->first();
|
||||
$order = $line?->order;
|
||||
|
||||
return [
|
||||
'supplier_id' => $order?->supplier_id,
|
||||
'purchase_date' => $order?->purchase_date?->toDateString(),
|
||||
'purchase_cost' => $line?->price !== null ? (string) $line->price : null,
|
||||
'order_number' => $order?->order_number,
|
||||
'supplier_id' => $line?->order?->supplier_id,
|
||||
'purchase_date' => $line?->order?->purchase_date,
|
||||
'purchase_cost' => $line?->price !== null ? (float) $line->price : null,
|
||||
'order_number' => $line?->order?->order_number,
|
||||
];
|
||||
}
|
||||
|
||||
|
||||
@ -5724,16 +5724,28 @@ parameters:
|
||||
count: 6
|
||||
path: app/Models/Accessory.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$order_number\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
path: app/Models/Accessory.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$price\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
path: app/Models/Accessory.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$purchase_date\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
path: app/Models/Accessory.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$supplier_id\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
count: 2
|
||||
path: app/Models/Accessory.php
|
||||
|
||||
-
|
||||
@ -6018,16 +6030,28 @@ parameters:
|
||||
count: 6
|
||||
path: app/Models/Asset.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$order_number\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
path: app/Models/Asset.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$price\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
path: app/Models/Asset.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$purchase_date\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
path: app/Models/Asset.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$supplier_id\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
count: 2
|
||||
path: app/Models/Asset.php
|
||||
|
||||
-
|
||||
@ -6930,16 +6954,28 @@ parameters:
|
||||
count: 6
|
||||
path: app/Models/Component.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$order_number\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
path: app/Models/Component.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$price\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
path: app/Models/Component.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$purchase_date\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
path: app/Models/Component.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$supplier_id\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
count: 2
|
||||
path: app/Models/Component.php
|
||||
|
||||
-
|
||||
@ -7140,16 +7176,28 @@ parameters:
|
||||
count: 6
|
||||
path: app/Models/Consumable.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$order_number\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
path: app/Models/Consumable.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$price\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
path: app/Models/Consumable.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$purchase_date\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
path: app/Models/Consumable.php
|
||||
|
||||
-
|
||||
message: '#^Access to an undefined property Illuminate\\Database\\Eloquent\\Model\:\:\$supplier_id\.$#'
|
||||
identifier: property.notFound
|
||||
count: 1
|
||||
count: 2
|
||||
path: app/Models/Consumable.php
|
||||
|
||||
-
|
||||
|
||||
@ -100,16 +100,6 @@ Route::middleware(['web', 'auth', 'authorize:superuser'])->prefix('oauth')->grou
|
||||
});
|
||||
|
||||
Route::group(['middleware' => 'auth'], function () {
|
||||
/*
|
||||
* Calendar (unified view across every HasCalendarEvents source).
|
||||
* Companion API endpoint lives at /api/v1/calendar/events.
|
||||
*/
|
||||
Route::get('calendar', [App\Http\Controllers\CalendarEventsController::class, 'index'])
|
||||
->name('calendar.index')
|
||||
->breadcrumbs(fn (Tabuna\Breadcrumbs\Trail $trail) => $trail->parent('home')
|
||||
->push(trans('general.calendar'), route('calendar.index'))
|
||||
);
|
||||
|
||||
/*
|
||||
* Companies
|
||||
*/
|
||||
|
||||
@ -65,4 +65,67 @@ class CreateGroupWithMembersTest extends TestCase
|
||||
$response->assertStatus(201);
|
||||
$this->assertDatabaseHas('permission_groups', ['name' => 'SCIM Group No Members']);
|
||||
}
|
||||
|
||||
public function test_post_group_with_many_members_does_not_explode_validator()
|
||||
{
|
||||
// Regression for the SCIM group-sync OOM at
|
||||
// ValidationRuleParser::mergeRulesForAttribute:227. Previously the
|
||||
// per-member `required` rule on `value` caused Laravel to allocate
|
||||
// one rule stack per member entry in the flattened payload, so a
|
||||
// 101k-member group sync blew a 256MB PHP process before any code
|
||||
// ran. Rule was dropped from SnipeSCIMConfig::getGroupConfig and
|
||||
// the check moved into SnipeMutableCollection::add so the validator
|
||||
// does O(1) work on the members array regardless of size.
|
||||
//
|
||||
// 25 members here is a proxy for the customer's much larger sync.
|
||||
// If the O(N) rule explosion regresses, larger integration
|
||||
// environments would OOM again; a per-item explosion at N=25 is
|
||||
// fine on 256MB but any code path that reintroduces it is caught
|
||||
// by the mechanism-level assertion in
|
||||
// GroupMembersValidationShapeTest.
|
||||
Passport::actingAs(User::factory()->superuser()->create());
|
||||
|
||||
$members = User::factory()->count(25)->create();
|
||||
|
||||
$response = $this->postJson('/scim/v2/Groups', [
|
||||
'schemas' => ['urn:ietf:params:scim:schemas:core:2.0:Group'],
|
||||
'displayName' => 'SCIM Group Many Members',
|
||||
'members' => $members->map(fn ($u) => ['value' => $u->id])->all(),
|
||||
]);
|
||||
|
||||
$response->assertStatus(201);
|
||||
$group = Group::where('name', 'SCIM Group Many Members')->firstOrFail();
|
||||
$this->assertSame(25, DB::table('users_groups')->where('group_id', $group->id)->count());
|
||||
}
|
||||
|
||||
public function test_post_group_with_member_missing_value_returns_400_with_indices()
|
||||
{
|
||||
// Regression for the parent library's 500 with an empty
|
||||
// "One or more members are unknown: " message when a members entry
|
||||
// arrives without its `value` field. SnipeMutableCollection::add
|
||||
// now catches this at attach time with a 400 that names the
|
||||
// offending indices, matching how bad request bodies are surfaced
|
||||
// elsewhere in the SCIM stack (SnipeRootComplex::add/replace also
|
||||
// route malformed keys through 400s).
|
||||
Passport::actingAs(User::factory()->superuser()->create());
|
||||
|
||||
$goodMember = User::factory()->create();
|
||||
|
||||
$response = $this->postJson('/scim/v2/Groups', [
|
||||
'schemas' => ['urn:ietf:params:scim:schemas:core:2.0:Group'],
|
||||
'displayName' => 'SCIM Group Bad Member',
|
||||
'members' => [
|
||||
['value' => $goodMember->id],
|
||||
['display' => 'no value key here'],
|
||||
['value' => null],
|
||||
],
|
||||
]);
|
||||
|
||||
$response->assertStatus(400);
|
||||
$body = $response->json();
|
||||
$this->assertStringContainsString('Every members entry must include', json_encode($body));
|
||||
// Both offending entries by their indices in the payload.
|
||||
$this->assertStringContainsString('1', json_encode($body));
|
||||
$this->assertStringContainsString('2', json_encode($body));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user