Add Port check when creating allocations #661
Conversation
app/Filament/Resources/NodeResource/RelationManagers/AllocationsRelationManager.php
Outdated
Show resolved
Hide resolved
| @@ -122,8 +132,19 @@ public function table(Table $table): Table | |||
|
|
|||
| $start = max((int) $start, 0); | |||
| $end = min((int) $end, 2 ** 16 - 1); | |||
| foreach (range($start, $end) as $i) { | |||
| $ports->push($i); | |||
| $range = $start <= $end ? range($start, $end) : range($end, $start); | |||
There was a problem hiding this comment.
If the first port number is greater than the second port number, then no numbers should be added.
| $ports->push($i); | ||
| $range = $start <= $end ? range($start, $end) : range($end, $start); | ||
| foreach ($range as $i) { | ||
| if ($i > 1024 && $i <= 65535) { |
There was a problem hiding this comment.
Invert the if statement and continue early!
| $range = $start <= $end ? range($start, $end) : range($end, $start); | ||
| foreach ($range as $i) { | ||
| if ($i > 1024 && $i <= 65535) { | ||
| if (Allocation::query()->where('ip', $get('allocation_ip'))->where('port', $portEntry)->exists()) { |
There was a problem hiding this comment.
This needs to happen after the foreach loop because of the N+1 problem!
| $ports->push((int) $portEntry); | ||
|
|
||
| continue; | ||
| if (Allocation::query()->where('ip', $get('allocation_ip'))->where('port', $portEntry)->exists()) { |
There was a problem hiding this comment.
This needs to happen after the foreach loop because of the N+1 problem!
| @@ -247,7 +256,15 @@ public function form(Form $form): Form | |||
| $range = $start <= $end ? range($start, $end) : range($end, $start); | |||
| foreach ($range as $i) { | |||
| if ($i > 1024 && $i <= 65535) { | |||
| $ports->push($i); | |||
| if (Allocation::query()->where('ip', $get('allocation_ip'))->where('port', $portEntry)->exists()) { | |||
There was a problem hiding this comment.
This needs to happen after the foreach loop because of the N+1 problem!
| $ports = collect(); | ||
| $update = false; | ||
| foreach ($state as $portEntry) { | ||
| if (!str_contains($portEntry, '-')) { | ||
| if (is_numeric($portEntry)) { | ||
| $ports->push((int) $portEntry); | ||
| if (Allocation::query()->where('ip', $get('allocation_ip'))->where('port', $portEntry)->exists()) { |
There was a problem hiding this comment.
This needs to happen after the foreach loop because of the N+1 problem!
| $range = $start <= $end ? range($start, $end) : range($end, $start); | ||
| foreach ($range as $i) { | ||
| if ($i > 1024 && $i <= 65535) { | ||
| if (Allocation::query()->where('ip', $get('allocation_ip'))->where('port', $portEntry)->exists()) { |
There was a problem hiding this comment.
This needs to happen after the foreach loop because of the N+1 problem!
| @@ -118,8 +128,19 @@ public function table(Table $table): Table | |||
|
|
|||
| $start = max((int) $start, 0); | |||
| $end = min((int) $end, 2 ** 16 - 1); | |||
| foreach (range($start, $end) as $i) { | |||
| $ports->push($i); | |||
| $range = $start <= $end ? range($start, $end) : range($end, $start); | |||
|
What's your take on this ?
|
I'd prefer to keep it the same. Does this push a notification to the end user? Where is the exception displayed? Screenshots? |
|
…nsRelationManager.php Co-authored-by: Lance Pioch <[email protected]>
Will check the allocations for the given ip and the given ports to see if they already exist in the database, if they do they're removed from the tag list, and a notification is sent telling the user that the port already exists.
This is applied to all three places where you can create an allocation.
Closes #655