search

michaeldyrynda / laravel-model-uuid

This package allows you to easily work with UUIDs in your Laravel models
MIT License
442 stars 46 forks source link

Allow to ignore empty UUID columns #110

Closed andrzejkupczyk closed 2 years ago

andrzejkupczyk commented 2 years ago

This allows to toggle resolving UUID for empty UUID columns (enabled by default for backward compatibility).

michaeldyrynda commented 2 years ago

Are you able to clarify what you’re trying to achieve exactly?

andrzejkupczyk commented 2 years ago

For example if UUID columns are nullable, you often do not want these columns to be filled automatically when creating a new model.

Current implementation:

$model = SomeModel::create(['optional_uuid' => null]);
$model->only($model->uuidColumns()); // array of automatically resolved UUIDs

Expected behaviour, with the $ignoreEmptyUuidColumns = true; property:

$model = SomeModel::create(['optional_uuid' => null]);
$model->only($model->uuidColumns()); // array of null values

Alternatively, an empty UUID (Nil) could be used in this case, but it would not be an ideal solution.

michaeldyrynda commented 2 years ago

Thanks for clearing that up!

I’m going to leave the behaviour as is for now, as the intent of this package is to generate UUID values for designated columns if they’re not already set i.e. the desire is for them to never be null.

It sounds as though you may even be using UUIDs as primary/foreign keys, which I don’t want to encourage with this package.

andrzejkupczyk commented 2 years ago

It sounds as though you may even be using UUIDs as primary/foreign keys, which I don’t want to encourage with this package.

Just to clarify, I'm using UUIDs columns to store external resource's keys. In short, the process responsible for syncing data from external database (via HTTP API) simply stores external rersources and their relationships within my database. However, some of those relationships are optional so, yeah…

Anyway, in case of nullable UUID column with an index, it would be a good idea not to create useless UUID in order to reduce length of the index, wouldn't it? At least it would be nice to have such an option.

michaeldyrynda commented 2 years ago

In that case, could you just skip adding them to the uuidColumns method and querying them directly?

Given they’re saved and retrieved independently of the core package functionality (generating UUID values for columns automatically) in your context, I’m not sure it makes much sense to use the package functionality for those columns.

andrzejkupczyk commented 2 years ago

Alright, I got it. Thanks for the advice and your time.