Fix larastan on level 2 #547
Conversation
There was a problem hiding this comment.
Hey @alaincodes24
Thank you very much, looks really good. I have left a few comments in the code.
I have a more general request (which is a little hard to explain, but let me try):
In this PR I would like us to not change any of the application logic. In particular, I see you are adding few relations here and there. I assume this is required because some pieces of the code are seemingly using these relations.
However, could we instead change the pieces of code that are using wrong references instead? I understand that some relations are waaaay to complicated (for example the whole Address<>GeographicalInformation block). If you notice some of these which could be improved, could you document your finding by creating an improvement ticket on Github?
So, in short, could you try to make it run on Larastan Level 2 without adding any relations?
| ->allOnConnection('redis') | ||
| ->onQueue(config('services.queues.payment')); | ||
| } else { | ||
| // TODO: LOG ERROR |
There was a problem hiding this comment.
Is this a ToDo for this PR or later?
| @@ -86,4 +87,8 @@ public function findBySerialNumber(string $meterSerialNumber): ?self { | |||
| public function getId(): int { | |||
| return $this->id; | |||
| } | |||
|
|
|||
| public function geo(): MorphOne { | |||
| return $this->morphOne(GeographicalInformation::class, 'owner'); | |||
There was a problem hiding this comment.
I don't think Meter is directly related to GeoGraphicalInformation. Instead it's Address<>Device<>Meter
| @@ -125,4 +126,8 @@ public function livingInClusterQuery(int $clusterId) { | |||
| public function getId(): int { | |||
| return $this->id; | |||
| } | |||
|
|
|||
| public function meters() { | |||
| return $this->hasMany(Meter::class); | |||
There was a problem hiding this comment.
The relation here is Person<>Device<>Meter
| @@ -145,4 +153,8 @@ public function originalCash(): BelongsToMorph { | |||
| public function originalWaveMoney(): BelongsToMorph { | |||
| return BelongsToMorph::build($this, WaveMoneyTransaction::class, 'originalTransaction'); | |||
| } | |||
|
|
|||
| public function meter() { | |||
| return $this->belongsTo(Meter::class, 'message', 'serial_number'); | |||
There was a problem hiding this comment.
The relation here is Transaction<>Device<>Meter
src/backend/app/Models/Device.php
Outdated
| public function manufacturer(): BelongsTo { | ||
| return $this->belongsTo(Manufacturer::class); | ||
| } | ||
|
|
||
| public function getManufacturerAttribute() { | ||
| return $this->manufacturer->name ?? 'Unknown Manufacturer'; // Adjust as needed | ||
| } | ||
|
|
||
| public function connectionType(): BelongsTo { | ||
| return $this->belongsTo(ConnectionType::class); | ||
| } | ||
|
|
||
| public function connectionGroup(): BelongsTo { | ||
| return $this->belongsTo(ConnectionGroup::class); | ||
| } | ||
| } |
There was a problem hiding this comment.
All these seem like relations that should be in the underlying device implementation. For example connectionType only makes sense for Meter but not for SHS. So, it shouldn't be on Device.
This PR addresses the listed errors of
composer run larastancloses #411
Please confirm you have completed any of the necessary steps below.