__fillable__ and __guarded__ are mutually exclusive... why?

[circulon]

Describe the bug Currectly (in v 2.19.1) cannot specify both fillable and guarded in a model together. The documentation here: https://orm.masoniteproject.com/models#mass-assignment implies (to me) that its ok.

if used together in the same model the following exception is raised when the model is instantiated. SomeModel must specify either __fillable__ or __guarded__ properties, but not both.

This kind of defeats the purpose of being able to filter fields explicitly for different scenarios where they should or should not be mass assigned.

Additionally the Model.filter_mass_assignment() method actually uses both these attributes

@classmethod
def filter_mass_assignment(cls, dictionary: Dict[str, Any]) -> Dict[str, Any]:
    """
     Filters the provided dictionary in preparation for a mass-assignment operation
     Wrapper around filter_fillable() & filter_guarded(). Passed dictionary is not mutated.
    """
    return cls.filter_guarded(cls.filter_fillable(dictionary))

So my question is, is this a design choice or is this error a false negative bug where the idea could have been possibly to check a column name cannot be in both __fillable__ and __guarded__ at the same time?

The Scenario where both these would be used is during a create followed by an update or explicit .save() so the guarded columns could not be accidently changed usinf mass assignment.

To Reproduce Create a model that contains fillable and guarded

class Company(Model):
    __table__ = "company"

    __guarded__ = [
        "customer_ref",
    ]

    __fillable__ = [
        "name",
        "email",
        "phone",
        "active",
    ]

Instantiate the model

model = Company()
# raises "Company must specify either __fillable__ or __guarded__ properties, but not both."

Expected behavior Expect the model to be instantiated.

Desktop (please complete the following information):

• OS: Mac OSX
• Version 12

What database are you using?

• Masonite ORM 2.19.1

[circulon]

Looks like this came in as part of #830 "now raises if both fillable and guarded are defined on the base model. This mirrors Orator's behavior." commit : da1871b569984491e6f9c6fc7905752d1e42e1d9

Though looking throught the Orator docs and current code base I cannot seem to find any reference to these attributes being mutually exclusive?

@stoutput can you please provide an explanation? I found this in the Orator docs: You can also use the create method to save a model in a single line, but you will need to specify either the __fillable__ or __guarded__ property on the model since all models are protected against mass-assigment by default This implies you must at least provide a stipulation of columns to be ignored or included when doing mass assignments. It does not say you cannot use both.

Happy to discuss further to get better understanding.

[stoutput]

Hey @circulon! There's really no need to specify both. One is a blocklist, one is an allowlist. From the docs: The __guarded__ is the inverse and acts as “blacklist”. By specifying __fillable__ you are saying only those fields should be allowed to be set during a create or update - why would you also specify fields in __guarded__ rather than just omitting them from __fillable__? The same question applies vice-versa. I would be curious to see a valid real-world use-case if you can provide one.

The reason filter_mass_assignment filters both is because one or the other may be set, and rather than using conditionals (i.e. if (cls.getattr('__fillable__', None))), we just call through to apply filtering if the attribute is not empty.

[circulon]

@stoutput Yeah I see what you mean and understand that only one or the other is required for the exclude/include to be functional.

I was considering the design / description of the Model itself so apologies for the confusion.
By that I mean defining explicitly what is allowed and what is not, there is no ambiguity about which fields are available (automatically included/excluded) for which context ie mass assignment or single assignment. But this is just my opinion on an aspect of Model design.

So it looks like the docs need a little clarification added around this... I will have a look at that later.

Happy to discuss further if you like.

[josephmancuso]

i think this is resolved now from he last reply from @stoutput which makes sense to me