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

[Laravel] Define read-only model attributes #6875

Open
cay89 opened this issue Dec 17, 2024 · 8 comments
Open

[Laravel] Define read-only model attributes #6875

cay89 opened this issue Dec 17, 2024 · 8 comments

Comments

@cay89
Copy link
Contributor

cay89 commented Dec 17, 2024

API Platform version(s) affected: 4.0.10

Description

I can't find a way to define read-only model attributes for an API, which are only accessible through GET endpoints (e.g., an ID field). According to the documentation here, it's possible to make properties read-only using the ApiProperty metadata class, where the readable and writable options can be configured. However, I don't understand where or how to apply this in Laravel. Laravel doesn't have explicit properties for table columns—there's only an 'attributes' array and accessors/mutators for accessing them. So where should I use the ApiProperty configuration?

@toitzi
Copy link
Contributor

toitzi commented Dec 17, 2024

just create the property on your model (it is essentially what laravel does, just using the magic property way), and apply it there. It should work as expected then. For example if you add this to your user model (app/Models/User.php):

  #[APIProperty(writable: false)]
  protected string $email;

The E-Mail field will be available inside your read requests, but not inside, PUT,POST,PATCH
Alternativley you can also use attribute mutators (app/Models/User.php) - i think this would be the recommended way!:

#[ApiProperty(writable: false)]
public function email(): Attribute
{
    return Attribute::make(
        get: fn(string $value) => $value
    );
}

@toitzi
Copy link
Contributor

toitzi commented Dec 17, 2024

But it would be nice to use laravels "fillable" array here, it is used in some way right now i can see in the code but not for this purpose afaik. I'll happily try and add a PR for this tho.
@soyuka what do you think about taking laravels "fillable" property on models into account (i actually also stumbled upon this "problem")

EDIT: Using the fillable array, would also get rid of setting createdAt, updatedAt, etc.., which is great, because those usually should never be filled, they are automatically populated.

@soyuka
Copy link
Member

soyuka commented Dec 17, 2024

Indeed, fillable in our metadata should make writable to true and readable to false?

This should be done at:

return $propertyMetadata
->withBuiltinTypes([$type])
->withWritable($propertyMetadata->isWritable() ?? true)
->withReadable($propertyMetadata->isReadable() ?? false === $p['hidden']);

In the meantime using #[ApiProperty(writable: false)] should work. PR welcome @toitzi !

@cay89
Copy link
Contributor Author

cay89 commented Dec 17, 2024

just create the property on your model (it is essentially what laravel does, just using the magic property way), and apply it there. It should work as expected then. For example if you add this to your user model (app/Models/User.php):

  #[APIProperty(writable: false)]
  protected string $email;

The E-Mail field will be available inside your read requests, but not inside, PUT,POST,PATCH Alternativley you can also use attribute mutators (app/Models/User.php) - i think this would be the recommended way!:

#[ApiProperty(writable: false)]
public function email(): Attribute
{
    return Attribute::make(
        get: fn(string $value) => $value
    );
}

Ok, it seems this works on custom attributes, but with "built-ins" such as id, created_at, updated at... there is not working well.

Below is what it looks like in my case. I have an abstract parent class for attributes that available on every model and I need to use the ApiProperty attribute here too. What is the recommended way for this use-case?

#[
    ApiResource(
        operations: [
            new GetCollection(name: 'tenants'),
            new Post(name: 'tenants.create'),
        ],
        rules: [
            'companyName' => 'required|simple_text|max:128',
            'firstname' => 'required|simple_text|max:32',
            'lastname' => 'required|simple_text|max:32',
            'username' => 'required|simple_text|max:32|unique:tenants',
            'email' => 'required|email|max:64',
        ],
    ),
]
class Tenant extends AbstractModel
{
    /** @use HasFactory<TenantFactory> */
    use HasFactory;

    #[ApiProperty(writable: false)]
    public function slug(): Attribute
    {
        return Attribute::make(get: fn(?string $value) => $value);
    }

    /**
     * The attributes that are mass assignable.
     *
     * @var array<int, string>
     */
    protected $fillable = [
        'company_name',
        'firstname',
        'lastname',
        'username',
        'email',
    ];
}
abstract class AbstractModel extends Model
{
    use HasUuids;

    #[ApiProperty(writable: false)]
    public function id(): Attribute
    {
        return Attribute::make(get: fn(string $value) => $value);
    }

    #[ApiProperty(writable: false)]
    public function createdAt(): Attribute
    {
        return Attribute::make(get: fn(string $value) => $value);
    }

    #[ApiProperty(writable: false)]
    public function updatedAt(): Attribute
    {
        return Attribute::make(get: fn(string $value) => $value);
    }
}

The "slug" attribute disappears at POST endpoint, but the others... not.

image

@soyuka
Copy link
Member

soyuka commented Dec 17, 2024

You can specify property directly on the ApiProperty attribute:

#[
    ApiResource(
        ...
    ),
    ApiProperty(property: 'created_at', writeable: false)
]
class Tenant extends Model
{}

Because Eloquent properties are snake-cased I think you need the snake_case property to make this work. Can't dig too much into this right now but I'm sure that we'll come up with a solution!

@cay89
Copy link
Contributor Author

cay89 commented Dec 18, 2024

Ok, it's works:

# ...
#[ApiProperty(writable: false, property: 'created_at')]
class Tenant extends AbstractModel
{
// ...
}

But it's not:

#[ApiProperty(writable: false, property: 'created_at')]
abstract class AbstractModel extends Model
{
// ...
}

I don't want to apply it to each model separately, as it is the same for all of them. It would be nice if there was a solution for this.

@cay89
Copy link
Contributor Author

cay89 commented Dec 18, 2024

Ohh, I tried it this way and it seems good, I'm still testing it.

abstract class AbstractModel extends Model
{
    #[ApiProperty(writable: false)]
    public \DateTime $created_at;
}

@cay89
Copy link
Contributor Author

cay89 commented Dec 19, 2024

Ohh, I tried it this way and it seems good, I'm still testing it.

abstract class AbstractModel extends Model
{
    #[ApiProperty(writable: false)]
    public \DateTime $created_at;
}

The problem with this is that if I define the property, Laravel will no longer set its default value, so I lose the built-in automation that handles these fields.

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

No branches or pull requests

3 participants