Skip to content
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

No repeatable way to use "null means unset" #5

Open
jwadhams opened this issue May 6, 2024 · 0 comments · May be fixed by #6
Open

No repeatable way to use "null means unset" #5

jwadhams opened this issue May 6, 2024 · 0 comments · May be fixed by #6

Comments

@jwadhams
Copy link
Collaborator

jwadhams commented May 6, 2024

We have code in a JsonModel like

public function setZipAttribute($value): void
    {
        // Null will fail validation but the intent can be inferred to mean "delete"
        if (null === $value) {
            unset($this->attributes['zip']);
            return;
        }
        $this->attributes['zip'] = $value;
    }

We're using this so that PATCH requests can send {"zip":null} and it's interpreted to mean "remove the ZIP" or "go back to default ZIP behavior for this account"

We could use cast classes for this, but \Illuminate\Database\Eloquent\Concerns\HasAttributes::setAttributeMarkedMutatedAttributeValuemakes it impossible (for everything I've tried) for any return value of a cast to unset an attribute entirely. (This is because Eloquent is by default thinking about structured relational databases, there's no sane reason to remove a column from the UPDATE query, that won't unset it, that will fail to modify it at all, so PHP null and MySQL null are good compliments, in a way that PHP null and Json "attribute is missing" are not.)

I'd like to try a new cast on JsonModels like "nullUnsets" and encapsulate that behavior into JsonModel::castAttribute (which otherwise inherits from \Illuminate\Database\Eloquent\Concerns\HasAttributes::castAttribute)

If there's a downside of this approach, its that Laravel doesn't have a reason to stack casts, so we'd have to fragment this further like stringOrUnset or floatOrUnset if that becomes important later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant