Fix/optimise address creation/editing via System AKAs

app/Http/Controllers/SystemController.php

@@ -16,11 +16,10 @@ use Illuminate\Support\Facades\Notification;
 use Illuminate\Support\ViewErrorBag;
 
 use App\Classes\FTN\Message;
-use App\Http\Requests\{AddressMerge,AreafixRequest,SystemEchoareaRequest,SystemRegisterRequest,SystemSessionRequest};
+use App\Http\Requests\{AddressAdd,AddressMerge,AreafixRequest,SystemEchoareaRequest,SystemRegisterRequest,SystemSessionRequest};
 use App\Jobs\AddressPoll;
 use App\Models\{Address,Echoarea,Echomail,Filearea,Netmail,Setup,System,Zone};
 use App\Notifications\Netmails\AddressLink;
-use App\Rules\{FidoInteger,TwoByteInteger};
 
 class SystemController extends Controller
 {
@@ -74,223 +73,51 @@ class SystemController extends Controller
 	/**
 	 * Add an address to a system
 	 *
-	 * @param Request $request
+	 * @param AddressAdd $request
 	 * @param System $o
 	 * @return \Illuminate\Http\RedirectResponse
 	 * @throws \Illuminate\Auth\Access\AuthorizationException
 	 */
-	public function address_add(Request $request,System $o)
+	public function address_add(AddressAdd $request,System $o)
 	{
-		// @todo This should be admin of the zone
-		$this->authorize('admin',$o);
-		session()->flash('accordion','address');
-
-		$request->validate([
-			'action' => 'required|in:region,host,node,update',
-			'zone_id' => 'required|exists:zones,id',
-		]);
-
-		switch ($request->action) {
-			case 'region':
-				$request->validate([
-					'region_id_new' => [
-						'required',
-						new TwoByteInteger,
-						function ($attribute,$value,$fail) use ($request) {
-							// Check that the region doesnt already exist
-							$o = Address::where(function($query) use ($value) {
-								return $query->where(function($query) use ($value) {
-									return $query
-										->where('region_id',$value)
-										->where('role',Address::NODE_RC);
-									})
-									// Check that a host doesnt already exist
-									->orWhere(function($query) use ($value) {
-										return $query
-											->where('host_id',$value)
-											->where('role',Address::NODE_NC);
-									});
-								})
-								->where('zone_id',$request->zone_id)
-								->where('node_id',0)
-								->where('point_id',0);
-
-							if ($o->count()) {
-								$fail('Region or host already exists');
-							}
-						},
-					],
-				]);
-
-				$oo = new Address;
-				$oo->zone_id = $request->zone_id;
-				$oo->region_id = $request->region_id_new;
-				$oo->host_id = $request->region_id_new;
-				$oo->node_id = 0;
-				$oo->point_id = 0;
+		$oo = Address::findOrNew($request->validated('submit'));
+		$oo->zone_id = $request->validated('zone_id');
+		$oo->security = $request->validated('security');
 		$oo->active = TRUE;
 
-				$o->addresses()->save($oo);
+		switch ($request->validated('action')) {
+			case 'region':
+				$oo->region_id = $request->validated('region_id_new');
+				$oo->host_id = $request->validated('region_id_new');
+				$oo->node_id = 0;
+				$oo->point_id = 0;
+
 				break;
 
 			case 'host':
-				$request->validate([
-					'region_id' => ['required',new FidoInteger],
-					'host_id_new' => [
-						'required',
-						new TwoByteInteger,
-						function ($attribute,$value,$fail) use ($request) {
-							// Check that the region doesnt already exist
-							$o = Address::where(function($query) use ($value) {
-								return $query->where(function($query) use ($value) {
-									return $query
-										->where('region_id',$value)
-										->where('role',Address::NODE_RC);
-								})
-									// Check that a host doesnt already exist
-									->orWhere(function($query) use ($value) {
-										return $query
-											->where('host_id',$value)
-											->where('role',Address::NODE_NC);
-									});
-							})
-								->where('zone_id',$request->zone_id)
-								->where('node_id',$request->node_id_new)
-								->where('point_id',0);
-
-							if ($o->count()) {
-								$fail('Region or host already exists');
-							}
-						},
-					],
-					'node_id_new' => [
-						'required',
-						new TwoByteInteger,
-						function ($attribute,$value,$fail) use ($request) {
-							// Check that the region doesnt already exist
-							$o = Address::where(function($query) use ($request,$value) {
-								return $query
-									->where('zone_id',$request->zone_id)
-									->where('host_id',$request->host_id_new)
-									->where('node_id',$value)
-									->where('point_id',0)
-									->where('role',Address::NODE_RC);
-							});
-
-							if ($o->count()) {
-								$fail('Host already exists');
-							}
-						},
-					]
-				]);
-
-				// Find the Hub address
-				// Find the zones <HOST>/0 address, and assign it to this host.
-				$oo = Address::where('zone_id',$request->zone_id)
-					->where('region_id',$request->region_id)
-					->where('host_id',$request->host_id_new)
-					->where('node_id',0)
-					->where('point_id',0)
-					->single();
-
-				// Its not defined, so we'll create it.
-				if (! $oo) {
-					$oo = new Address;
-					$oo->forceFill([
-						'zone_id'=>$request->zone_id,
-						'region_id'=>$request->region_id,
-						'host_id'=>$request->host_id_new,
-						'node_id'=>0,
-						'point_id'=>0,
-						'role'=>Address::NODE_NC,
-					]);
-				}
-
-				$oo->system_id = $request->system_id;
-				$oo->active = TRUE;
-				$o->addresses()->save($oo);
-
-				$oo = new Address;
-				$oo->zone_id = $request->zone_id;
-				$oo->region_id = $request->region_id;
-				$oo->host_id = $request->host_id_new;
-				$oo->node_id = $request->node_id_new;
+				$oo->region_id = $request->validated('region_id');
+				$oo->host_id = $request->validated('host_id_new');
+				$oo->node_id = 0;
 				$oo->point_id = 0;
-				$oo->active = TRUE;
 
-				$o->addresses()->save($oo);
 				break;
 
 			case 'update':
 			case 'node':
-				$request->validate([
-					'region_id' => ['required',new FidoInteger],
-					'host_id' => ['required',new FidoInteger],
-					'node_id' => [
-						'required',
-						new TwoByteInteger,
-						function ($attribute,$value,$fail) use ($request) {
-							if ($request->point_id === 0) {
-								// Check that the host doesnt already exist
-								$o = Address::where(function($query) use ($request,$value) {
-									return $query
-										->where('zone_id',$request->zone_id)
-										->where('host_id',$request->host_id)
-										->where('node_id',$value)
-										->where('point_id',0)
-										->where('id','<>',$request->submit);
-								});
+				$oo->region_id = $request->validated('region_id');
+				$oo->host_id = $request->validated('host_id');
+				$oo->node_id = $request->validated('node_id');
+				$oo->point_id = $request->validated('point_id');
+				$oo->hub_id = $request->validated('hub_id') ? $request->validated('hub_id') : NULL;
 
-								if ($o->count()) {
-									$fail(sprintf('Host already exists: %s',$o->get()->pluck('ftn')->join(',')));
-								}
-							}
-						},
-					],
-					'point_id' => [
-						'required',
-						function($attribute,$value,$fail) use ($request) {
-							if (! is_numeric($value) || $value > Address::ADDRESS_FIELD_MAX)
-								$fail(sprintf('Point numbers must be between 0 and %d',Address::ADDRESS_FIELD_MAX));
-
-							// Check that the host doesnt already exist
-							$o = Address::where(function($query) use ($request,$value) {
-								return $query
-									->where('zone_id',$request->zone_id)
-									->where('host_id',$request->host_id)
-									->where('node_id',$request->node_id)
-									->where('point_id',$value)
-									->where('id','<>',$request->submit);
-							});
-
-							if ($o->count()) {
-								$fail(sprintf('Point already exists: %s',$o->get()->pluck('ftn')->join(',')));
-							}
-						}
-					],
-					'hub' => 'required|boolean',
-					'hub_id' => 'nullable|exists:addresses,id',
-					'security' => 'required|integer|min:0|max:7',
-				]);
-
-				$oo = Address::findOrNew($request->submit);
-				$oo->zone_id = $request->zone_id;
-				$oo->region_id = $request->region_id;
-				$oo->host_id = $request->host_id;
-				$oo->node_id = $request->node_id;
-				$oo->point_id = $request->point_id;
-				// @todo Validation should check that the hub is in the right region and net
-				$oo->hub_id = $request->hub_id > 0 ? $request->hub_id : NULL;
-				$oo->security = $request->security;
-				$oo->active = TRUE;
-
-				$o->addresses()->save($oo);
 				break;
 
 			default:
 				return redirect()->back()->withErrors(['action'=>'Unknown action: '.$request->action]);
 		}
 
+		$o->addresses()->save($oo);
+
 		return redirect()->to(sprintf('system/addedit/%d',$o->id));
 	}
 

app/Http/Requests/AddressAdd.php

@@ -0,0 +1,201 @@
+<?php
+
+namespace App\Http\Requests;
+
+use Illuminate\Foundation\Http\FormRequest;
+use Illuminate\Http\Request;
+use Illuminate\Support\Facades\Gate;
+
+use App\Models\Address;
+use App\Rules\{FidoInteger,TwoByteInteger};
+
+/**
+ * Validation to add an address to a system
+ * o = System::class
+ *
+ * @note This validation only expects to be used with POST
+ * Variables:
+ * + "action" => "node"		// type of address being created
+ * + "zone_id" => "2"
+ * + "region_id" => "21"
+ * + "host_id" => "3"
+ * + "hub_id" => null
+ * + "node_id" => "9999"
+ * + "point_id" => "0"
+ * + "region_id_new" => null	// creating a new region id
+ * + "host_id_new" => null		// creating a new host id
+ * + "node_id_new" => null		// creating a new node id
+ * + "hub" => "0"
+ * + "security" => "9"
+ *
+ * Rules:
+ * - ZC is z:0/0.0 - region,node,point must be zero
+ *   ZC is identified when region_id,host_id,node_id and point_id === 0
+ * - RC is z:r/0.0 - region is non-zero, host_id = region_id, node,point must be zero.
+ *   RC is identified when region_id === host_id and, node_id and point_id === 0
+ * - NC is z:h/0.0 (r=r, r!=h, h!=z) [parent where z:r/0 and h=r, n=0]
+ *   NC is identified when region_id !== host_id and, node_id and point_id === 0,
+ * - HC is z:h/n.0 (r=r) [parent pointed by hub_id AND validate by z:h/0 is the hub_id]
+ *   HC is a normal node with, but has children pointing to it with hub_id
+ * - NN is z:h/n.0 when point_id === 0
+ *   A normal node where node_id !== 0, it may or may not have a hub_id
+ * - PT is z:h/n.p where point_id !== 0
+ *   PT is identified when point_id !== 0;
+ */
+class AddressAdd extends FormRequest
+{
+	/**
+	 * Determine if the user is authorized to make this request.
+	 */
+	public function authorize(): bool
+	{
+		session()->flash('accordion','address');
+
+		return request()->isMethod('post')
+			&& Gate::allows('admin',$this->route('o'));
+	}
+
+	/**
+	 * Get the validation rules that apply to the request.
+	 *
+	 * @return array<string, \Illuminate\Contracts\Validation\ValidationRule|array<mixed>|string>
+	 */
+	public function rules(Request $request): array
+	{
+		$rules = [];
+
+		switch ($request->action) {
+			case 'region':
+				$rules = [
+					'region_id_new' => [
+						'required',
+						new TwoByteInteger,
+						// Check that the region doesnt already exist
+						function ($attribute,$value,$fail) use ($request) {
+							$o = Address::where(fn($query)=>
+								$query
+									->where('region_id',$value)
+									// Check that a host doesnt already exist
+									->orWhere('host_id',$value)
+								)
+								->where('zone_id',$request->zone_id)
+								->where('node_id',0)
+								->where('point_id',0)
+								->first();
+
+							if ($o && $o->count())
+								$fail(sprintf('%s already exists [<a href="%s">here</a>]',$o->ftn,url('system/addedit',$o->system_id)));
+						},
+					],
+				];
+
+				break;
+
+			case 'host':
+				$rules = [
+					'region_id' => [
+						'required',
+						new FidoInteger,		// @todo the RC should exist, ie: z:r/0.0 (h=0)
+					],
+					'host_id_new' => [
+						'required',
+						new TwoByteInteger,
+						// Make sure that the host isnt already defined
+						function ($attribute,$value,$fail) use ($request) {
+							// Check that the region doesnt already exist
+							$o = Address::where(fn($query)=>
+								$query
+									->where('region_id',$value)
+									// Check that a host doesnt already exist
+									->orWhere('host_id',$value)
+								)
+								->where('zone_id',$request->zone_id)
+								->where('node_id',0)
+								->where('point_id',0)
+								->first();
+
+							if ($o && $o->count())
+								$fail(sprintf('%s already exists [<a href="%s">here</a>]',$o->ftn,url('system/addedit',$o->system_id)));
+						},
+					],
+				];
+
+				break;
+
+			case 'update':
+			case 'node':
+				$rules = [
+					'region_id' => [
+						'required',
+						new FidoInteger // @todo the RC should exist, ie: z:r/0.0 (h=0)
+					],
+					'host_id' => [
+						'required',new FidoInteger // @todo the NC should exist, ie: z:r/0.0 (h=0)
+					],
+					'node_id' => [
+						'required',
+						new TwoByteInteger,
+						function ($attribute,$value,$fail) use ($request) {
+							if ($request->point_id === 0) {
+								// Check that the host doesnt already exist
+								$o = Address::where(function($query) use ($request,$value) {
+									return $query
+										->where('zone_id',$request->zone_id)
+										->where('host_id',$request->host_id)
+										->where('node_id',$value)
+										->where('point_id',0)
+										->where('id','<>',$request->submit);
+									})
+									->get();
+
+								if ($o && $o->count())
+									$fail(sprintf('%s already exists [<a href="%s">here</a>]',$o->ftn,url('system/addedit',$o->system_id)));
+							}
+						},
+					],
+					'point_id' => [
+						'required',
+						new FidoInteger,
+						function ($attribute,$value,$fail) use ($request) {
+							// Check that the host doesnt already exist
+							$o = Address::where(function($query) use ($request,$value) {
+								return $query
+									->where('zone_id',$request->zone_id)
+									->where('host_id',$request->host_id)
+									->where('node_id',$request->node_id)
+									->where('point_id',$value)
+									->where('id','<>',$request->submit);
+								})
+								->first();
+
+							if ($o && $o->count())
+								$fail(sprintf('%s already exists [<a href="%s">here</a>]',$o->ftn,url('system/addedit',$o->system_id)));
+						}
+					],
+					//'hub' => 'required|boolean',
+					'hub_id' => 'nullable|exists:addresses,id',
+					'submit' => 'nullable|exists:addresses,id',
+				];
+
+				break;
+		}
+
+		return array_merge($rules,
+			[
+				'action' => [
+					'required',
+					'in:region,host,node,update',
+				],
+				'zone_id' => [
+					'required',
+					'exists:zones,id',
+				],
+				'security' => [
+					'nullable',
+					'numeric',
+					'min:0',
+					'max:7',
+				]
+			]);
+	}
+}

app/Models/Address.php

@@ -61,7 +61,7 @@ class Address extends Model
 	// http://ftsc.org/docs/frl-1002.001
 	public const ADDRESS_FIELD_MAX = 0x7fff;	// Maximum value for a field in the address
 
-	protected $visible = ['zone_id','region_id','host_id','node_id','point_id','security'];
+	protected $visible = ['zone_id','region_id','host_id','hub_id','node_id','point_id','security'];
 
 	/* STATIC */
 
@@ -752,7 +752,7 @@ class Address extends Model
 
 	public function getIsPrivateAttribute(): bool
 	{
-		return $this->role & self::NODE_PVT;
+		return (! $this->system->address);
 	}
 
 	/**
@@ -787,6 +787,13 @@ class Address extends Model
 	 */
 	public function getRoleNameAttribute(): string
 	{
+		if ($this->getIsDownAttribute())
+			return 'DOWN';
+		if ($this->getIsHoldAttribute())
+			return 'HOLD';
+		if ($this->getIsPrivateAttribute())
+			return 'PVT';
+
 		switch ($this->role_id) {
 			case self::NODE_ZC:
 				return 'ZC';

app/Rules/FidoInteger.php

@@ -10,7 +10,7 @@ class FidoInteger implements Rule
 {
 	/**
 	 * Determine if the validation rule passes.
-	 * This will check that a number used for zone, net, host is between 1 and Address::ADDRESS_FIELD_MAX.
+	 * This will check that a number used for zone, net, host is between 0 and Address::ADDRESS_FIELD_MAX.
 	 *
 	 * @param string $attribute
 	 * @param mixed $value

resources/views/system/widget/form-address.blade.php

@@ -104,7 +104,7 @@
 					<span class="input-group-text">/0.0</span>
 					<span class="invalid-feedback" role="alert">
 						@error('region_id_new')
-							{{ $message }}
+							{!! $message !!}
 						@else
 							The region number is required.
 						@enderror
@@ -118,24 +118,18 @@
 				<div class="input-group has-validation">
 					<span class="input-group-text"><i class="bi bi-hash"></i></span>
 					<input type="text" class="form-control text-end @error('host_id_new') is-invalid @enderror" id="host_id_new" placeholder="Host #" name="host_id_new" value="{{ old('host_id_new') }}" @cannot('admin',$o)disabled @endcannot>
-					<span class="input-group-text p-0">/</span>
-					<input type="text" class="form-control @error('node_id_new') is-invalid @enderror" id="node_id_new" placeholder="Node #" name="node_id_new" value="{{ old('node_id_new') }}" @cannot('admin',$o)disabled @endcannot>
-					<span class="input-group-text">.0</span>
+					<span class="input-group-text">/0.0</span>
 					<span class="invalid-feedback" role="alert">
 						@error('host_id_new')
-							{{ $message }}
+							{!! $message !!}
 						@else
 							The host address is required.
 						@enderror
-						@error('node_id_new')
-							{{ $message }}
-						@else
-							The node address is required.
-						@enderror
 					</span>
 				</div>
 			</div>
 
+			{{--
 			<!-- Hub Checkbox -->
 			<div class="col-2 d-none" id="hub-checkbox">
 				<span class="label form-label">Hub</span>
@@ -149,6 +143,7 @@
 					</div>
 				</div>
 			</div>
+			--}}
 
 			<!-- Security -->
 			<div class="col-2 d-none" id="sec-level">
@@ -176,7 +171,7 @@
 						There were errors with the submission.
 						<ul>
 							@foreach ($errors->all() as $error)
-								<li>{{ $error }}</li>
+								<li>{!! $error !!}</li>
 							@endforeach
 						</ul>
 					</span>
@@ -240,8 +235,10 @@
 				if (! $('#hub-select').hasClass('d-none'))
 					$('#hub-select').addClass('d-none')
 
+				{{--
 				if (! $('#hub-checkbox').hasClass('d-none'))
 					$('#hub-checkbox').addClass('d-none');
+				--}}
 
 				if (! $('#node-address').hasClass('d-none'))
 					$('#node-address').addClass('d-none');
@@ -286,8 +283,10 @@
 				if (! $('#hub-select').hasClass('d-none'))
 					$('#hub-select').addClass('d-none');
 
+				{{--
 				if (! $('#hub-checkbox').hasClass('d-none'))
 					$('#hub-checkbox').addClass('d-none');
+				--}}
 
 				if (! $('#node-address').hasClass('d-none'))
 					$('#node-address').addClass('d-none');
@@ -374,8 +373,10 @@
 						if (! $('#host-address').hasClass('d-none'))
 							$('#host-address').addClass('d-none');
 
+					{{--
 						if (! $('#hub-checkbox').hasClass('d-none'))
 							$('#hub-checkbox').addClass('d-none');
+					--}}
 
 						if (! $('#node-address').hasClass('d-none'))
 							$('#node-address').addClass('d-none');
@@ -390,8 +391,10 @@
 						if ($('#host-address').hasClass('d-none'))
 							$('#host-address').removeClass('d-none');
 
+					{{--
 						if (! $('#hub-checkbox').hasClass('d-none'))
 							$('#hub-checkbox').addClass('d-none');
+					--}}
 
 						if (! $('#node-address').hasClass('d-none'))
 							$('#node-address').addClass('d-none');
@@ -413,8 +416,10 @@
 						if (! $('#host-address').hasClass('d-none'))
 							$('#host-address').addClass('d-none');
 
+					{{--
 						if ($('#hub-checkbox').hasClass('d-none'))
 							$('#hub-checkbox').removeClass('d-none');
+					--}}
 
 						if ($('#node-address').hasClass('d-none'))
 							$('#node-address').removeClass('d-none');
@@ -439,7 +444,7 @@
 							$('#hub_id').show();
 
 							if (modify && modify.responseJSON.hub_id)
-								$('#host_id').val(modify.responseJSON.hub_id).change();
+								$('#hub_id').val(modify.responseJSON.hub_id).change();
 
 							$('#hub-select').removeClass('d-none');
 							$('#node_id').prop('disabled',false);
@@ -462,6 +467,7 @@
 				if ($('#sec-level').hasClass('d-none'))
 					$('#sec-level').removeClass('d-none')
 
+				{{--
 				switch(this.value) {
 					// Not selected - enter a normal node
 					case '':
@@ -475,8 +481,10 @@
 						if (! $('#hub-checkbox').hasClass('d-none'))
 							$('#hub-checkbox').addClass('d-none');
 				}
+				--}}
 			});
 
+			{{--
 			$('#point_id').on('change',function() {
 				switch(this.value) {
 					case '0':
@@ -489,6 +497,7 @@
 							$('#hub-checkbox').addClass('d-none');
 				}
 			});
+			--}}
 		});
 	</script>
 @append