-
-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add HasStatuses trait and supporting code #26
Changes from all commits
5b1c82e
baef1ac
641f2de
cc0eb73
0587533
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Tipoff\Statuses\Database\Factories; | ||
|
||
use Illuminate\Database\Eloquent\Factories\Factory; | ||
use Tipoff\Authorization\Models\User; | ||
use Tipoff\Statuses\Models\Status; | ||
use Tipoff\Statuses\Models\StatusRecord; | ||
|
||
class StatusRecordFactory extends Factory | ||
{ | ||
protected $model = StatusRecord::class; | ||
|
||
public function definition() | ||
{ | ||
$statusable = User::factory()->create(); | ||
$status = randomOrCreate(Status::class); | ||
|
||
return [ | ||
'status_id' => $status, | ||
'type' => $status->type, | ||
'statusable_type' => get_class($statusable), | ||
'statusable_id' => $statusable->id, | ||
'creator_id' => randomOrCreate(app('user')), | ||
]; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,10 +13,12 @@ public function up() | |
Schema::create('statuses', function (Blueprint $table) { | ||
$table->id(); | ||
$table->string('slug')->unique()->index(); | ||
$table->string('name')->unique(); | ||
$table->string('applies_to')->default('order'); // Values include 'order', 'slot', 'game', 'invoice', 'payment' | ||
$table->string('name'); | ||
$table->string('type'); // Typically, full class name for model using status | ||
$table->text('note')->nullable(); | ||
$table->timestamps(); | ||
|
||
$table->unique(['name', 'type']); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Uniqueness is not name alone - but name for a given type. Its certainly reasonable for two unrelated status types to have overlapping names. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. Thanks |
||
}); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Tipoff\Statuses\Exceptions; | ||
|
||
interface StatusException | ||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Tipoff\Statuses\Exceptions; | ||
|
||
use Throwable; | ||
|
||
class UnknownStatusException extends \InvalidArgumentException implements StatusException | ||
{ | ||
public function __construct(string $type, string $name, $code = 0, Throwable $previous = null) | ||
{ | ||
parent::__construct("Unknown status value '{$name}' for status type {$type}", $code, $previous); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,12 +4,87 @@ | |
|
||
namespace Tipoff\Statuses\Models; | ||
|
||
use Carbon\Carbon; | ||
use Illuminate\Database\Eloquent\Builder; | ||
use Illuminate\Support\Collection; | ||
use Illuminate\Support\Str; | ||
use Tipoff\Support\Models\BaseModel; | ||
use Tipoff\Support\Traits\HasPackageFactory; | ||
|
||
/** | ||
* @property int id | ||
* @property string name | ||
* @property string type | ||
* @property string slug | ||
* @property string note | ||
* @property Carbon created_at | ||
* @property Carbon updated_at | ||
*/ | ||
class Status extends BaseModel | ||
{ | ||
use HasPackageFactory; | ||
|
||
protected $casts = []; | ||
protected $casts = [ | ||
'id' => 'integer', | ||
]; | ||
|
||
protected $fillable = [ | ||
'type', | ||
'name', | ||
]; | ||
|
||
public static function publishStatuses(string $type, array $names): Collection | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method would be used in migrations to establish valid statuses for a given type. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that. Thanks |
||
{ | ||
return collect($names) | ||
->map(function (string $name) use ($type) { | ||
return static::createStatus($type, $name); | ||
}); | ||
} | ||
|
||
public static function createStatus(string $type, string $name, ?string $note = null): self | ||
{ | ||
/** @var Status $status */ | ||
$status = static::query()->firstOrNew([ | ||
'type' => $type, | ||
'name' => $name, | ||
]); | ||
|
||
if ($note) { | ||
$status->note = $note; | ||
} | ||
|
||
$status->save(); | ||
|
||
return $status; | ||
} | ||
|
||
public static function findStatus(string $type, string $name): ?self | ||
{ | ||
/** @var Status $status */ | ||
$status = static::query() | ||
->byType($type) | ||
->where('name', '=', $name) | ||
->first(); | ||
|
||
return $status; | ||
} | ||
|
||
protected static function boot() | ||
{ | ||
parent::boot(); | ||
|
||
static::creating(function (Status $status) { | ||
$status->slug = $status->slug ?: Str::slug("{$status->type}-{$status->name}"); | ||
}); | ||
} | ||
|
||
public function scopeByType(Builder $query, string $type): Builder | ||
{ | ||
return $query->where('type', '=', $type); | ||
} | ||
|
||
public function __toString() | ||
{ | ||
return $this->name; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Tipoff\Statuses\Models; | ||
|
||
use Carbon\Carbon; | ||
use Illuminate\Database\Eloquent\Model; | ||
use Tipoff\Authorization\Models\User; | ||
use Tipoff\Support\Models\BaseModel; | ||
use Tipoff\Support\Traits\HasCreator; | ||
use Tipoff\Support\Traits\HasPackageFactory; | ||
|
||
/** | ||
* @property int id | ||
* @property Status status | ||
* @property Model statusable | ||
* @property User creator | ||
* @property Carbon created_at | ||
* // Raw Relations | ||
* @property int|null creator_id | ||
*/ | ||
class StatusRecord extends BaseModel | ||
{ | ||
use HasPackageFactory; | ||
use HasCreator; | ||
|
||
const UPDATED_AT = null; | ||
|
||
protected $casts = [ | ||
'id' => 'integer', | ||
'creator_id' => 'integer', | ||
]; | ||
|
||
public function status() | ||
{ | ||
return $this->belongsTo(Status::class); | ||
} | ||
|
||
public function statusable() | ||
{ | ||
return $this->morphTo(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Tipoff\Statuses\Traits; | ||
|
||
use Illuminate\Support\Collection; | ||
use Tipoff\Statuses\Exceptions\UnknownStatusException; | ||
use Tipoff\Statuses\Models\Status; | ||
use Tipoff\Statuses\Models\StatusRecord; | ||
|
||
/** | ||
* @property Collection statusables | ||
*/ | ||
trait HasStatuses | ||
{ | ||
// When true, new statuses will be added automatically on first use | ||
// When false, exception occurs if unknown status value is used | ||
protected bool $dynamicStatusCreation = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be pulled if you only want to allow pre-defined statuses to be used. Included only because it was easy to add. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's interesting. Not sure if it is necessary since Nova allows a create resource option from within another model. |
||
|
||
public function getStatusHistory(?string $type = null): Collection | ||
{ | ||
return $this->statusRecords() | ||
->where('type', '=', $type ?? get_class($this)) | ||
->orderBy('created_at', 'desc') | ||
->orderBy('id', 'desc') | ||
->get(); | ||
} | ||
|
||
public function getStatus(?string $type = null): ?Status | ||
{ | ||
$statusRecord = $this->getStatusHistory($type)->first(); | ||
|
||
return $statusRecord ? $statusRecord->status : null; | ||
} | ||
|
||
public function setStatus(string $name, ?string $type = null): Status | ||
{ | ||
$type = $type ?? get_class($this); | ||
|
||
$status = $this->getStatusByName($name, $type); | ||
|
||
// Check if desired status already set | ||
if ($currentStatus = $this->getStatus($type)) { | ||
if ($currentStatus->id === $status->id) { | ||
return $currentStatus; | ||
} | ||
} | ||
Comment on lines
+43
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @drewroberts - want to call this out. If the requested status is the same as the current status, a new record is NOT created. If you always want a record, even if the requested status matches current status, it can be removed and tests updated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is a correct implementation for what I want. Thank you! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @joshtorres Could you make sure this logic is also added to |
||
|
||
// Set status always creates new record, leaving full history behind | ||
$statusable = new StatusRecord(); | ||
$statusable->type = $type; | ||
$statusable->statusable()->associate($this); | ||
|
||
$statusable->status()->associate($status)->save(); | ||
$this->load('statusRecords'); | ||
|
||
return $statusable->status; | ||
} | ||
|
||
public function statusRecords() | ||
{ | ||
return $this->morphMany(StatusRecord::class, 'statusable'); | ||
} | ||
|
||
private function getStatusByName(string $name, string $type): Status | ||
{ | ||
if ($this->dynamicStatusCreation) { | ||
return Status::createStatus($type, $name); | ||
} | ||
|
||
if ($status = Status::findStatus($type, $name)) { | ||
return $status; | ||
} | ||
|
||
throw new UnknownStatusException($type, $name); | ||
} | ||
} |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note change to 'type' here. The
HasStatuses
trait defaults to using the fully qualified model name as the type.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chx2 Take a look at the changes in this PR for what you're doing in tipoff/bookings with Status:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pdbreen You prefer using
type
instead ofapplies_to
for this field?CC: @sl0wik
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do. Its somewhat tied to potentially allowing a single model to have multiple, different types of statuses. But, even if that's not allowed, I think
type
is still a more appropriate term for this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great. Thanks for the explanation