tuttle-dev / tuttle

🪰 Tuttle - time and money management for freelancers
GNU General Public License v3.0
61 stars 12 forks source link

Ui/Model: Add field validation / format methods #210

Open vlad-ed-git opened 1 year ago

vlad-ed-git commented 1 year ago

Fields, such as Vat Rate under Contract, should have validation (and formatting where necessary) mechanisms to validate and format the user input according to the expected standard. E.g. keep decimals fixed, allow only numbers, etc

clstaudt commented 1 year ago

Working on using sqlmodel/ pydantic for validation #207

Code example - note the pattern

try:
    model_instance = Model.validate(
       dict(
          spam="Spam",
          eggs="Eggs",
       )
    )
except ValidationError:
  ...
class TestContact:
    def test_valid_instantiation(self):
        contact = Contact.validate(
            dict(
                first_name="Sam",
                last_name="Lowry",
                email="sam.lowry@miniinf.gov",
                company="Ministry of Information",
            )
        )
        assert store_and_retrieve(contact)

    def test_invalid_email(self):
        with pytest.raises(ValidationError):
            Contact.validate(
                dict(
                    first_name="Sam",
                    last_name="Lowry",
                    email="27B-",
                    company="Ministry of Information",
                )
            )
clstaudt commented 1 year ago

@vlad-ed-git Tell me whether I am right about this: In order to be serious about decoupling view and data model, view code should not be concerned with data validation. More specifically, there should be nothing in the view code that duplicates any of the definitions in model, or adds any definitions, e.g.:

I believe there is currently some code in the view that tries to do data validation, e.g.

        if not self.contact.first_name or not self.contact.last_name:
            self.on_error_callback("First and last name cannot be empty")
            return

Goal: Remove all code of this kind from the view. Let pydantic / sqlmodel do validation via Model.validate method.

Check out pydantic validators.

vlad-ed-git commented 1 year ago

@clstaudt Some definitions, such as whether a field is optional or not, are hinted on the View. And after all, any change to add or remove a field from a model will require a corresponding change in the Ui. For example. if we stop asking the user for their name and deleted that field from the User model, then the name form field must be removed from the Ui. *Also the input type defines the keyboard type (though not as meaningful for desktop apps) but still, if a number is expected, then the text field input's keyboard type is set to Number (this is why on your phone when you are typing an email in a form, the displayed keyboard is different from when you are typing a phone number for example). Flet being cross-platform offers the same mechanism, though, for our desktop app, it is not as useful (perhaps not useful at all). But apart from these, yes, validation is best done on the model, so you are correct. We should replace the current data validation checks on the view.

vlad-ed-git commented 1 year ago

@clstaudt One more thing, consider a required field Rate: Rate of enumeration. To be valid it should be 1. not empty, 2. numeric (not alphanumeric or just letters), and perhaps some other check like the number of decimal places. Therefore the Ui (actually its corresponding Intent class) must know if a field is invalid then what exactly is the issue so it can display the appropriate error message. The implementation of validate must take this into account and not raise generic errors.

vlad-ed-git commented 1 year ago

@clstaudt By the way, there was an issue in which you mentioned something about having a Ui form that is generated from the model. This is something that Django does. I do not know if there is a library for it (and even if there is, I doubt it will convert python to flet's flutter) but we could implement this ourselves in the future.

clstaudt commented 1 year ago

Flet being cross-platform offers the same mechanism, though, for our desktop app, it is not as useful (perhaps not useful at all).

We build a desktop app with no mobile version planned. Seems like there's no need to spend time on the keyboards.

By the way, there was an issue in which you mentioned something about having a Ui form that is generated from the model. This is something that Django does. I do not know if there is a library for it (and even if there is, I doubt it will convert python to flet's flutter) but we could implement this ourselves in the future.

I bet ChatGPT does well in generating form code from model code, if it is prompted with good examples.

clstaudt commented 1 year ago

Therefore the Ui (actually its corresponding Intent class) must know if a field is invalid then what exactly is the issue so it can display the appropriate error message. The implementation of validate must take this into account and not raise generic errors.

@vlad-ed-git How about the view inspecting the ValidationError and handling / displaying all of its errors?

from tabulate import tabulate

def print_validation_errors(ve: ValidationError):
    errors = ve.errors()
    table = []
    for error in errors:
        field_name = error.get('loc')[0]
        error_message = error.get('msg')
        table.append([field_name, error_message])
    print(tabulate(table, headers=["Field Name", "Error Message"]))

try:
    client = Client.validate(dict(name="Ministry of Information"))
except ValidationError as ve:
    print_validation_errors(ve)
vlad-ed-git commented 1 year ago

@clstaudt I am not sure about the rest of that code, but if the validation error is such that this part works, then great:

    errors = ve.errors()
    table = []
    for error in errors:
        field_name = error.get('loc')[0]
        error_message = error.get('msg')
clstaudt commented 1 year ago

The rest of the code is just for example output. This is working:

    def test_missing_name(self):
        """Test that a ValidationError is raised when the name is missing."""
        with pytest.raises(ValidationError):
            Client.validate(dict())

        try:
            client = Client.validate(dict())
        except ValidationError as ve:
            for error in ve.errors():
                field_name = error.get("loc")[0]
                error_message = error.get("msg")
                assert field_name == "name"
clstaudt commented 1 year ago

@vlad-ed-git Can we review this together?