sts-sponsors team mailing list archive

[Adam Collard <mp+439266@xxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing

mock is not the right name for what you have here, which is more like fake data. https://martinfowler.com/articles/mocksArentStubs.html


Additionally instead of having hard coded fake data like this, have you considered using https://faker.readthedocs.io/ or similar?

Diff comments:

> diff --git a/backend/msm/schema.py b/backend/msm/schema.py
> index 38a4ad5..10c726d 100644
> --- a/backend/msm/schema.py
> +++ b/backend/msm/schema.py
> @@ -2,9 +2,42 @@ from datetime import (
>      datetime,
>      timedelta,
>  )
> +from decimal import Decimal
>  from uuid import UUID
>  
> -from pydantic import BaseModel
> +from pydantic import (
> +    BaseModel,
> +    SecretStr,
> +)
> +from pydantic.fields import Field
> +
> +# see discussion at
> +# https://stackabuse.com/python-validate-email-address-with-regular-expressions-regex/
> +valid_email = r"""

'n' + 'o' * 2 ** 16

let us not ever validate emails with a regular expression.

> +    ([-!#-'*+/-9=?A-Z^-~]+(\.[-!#-'*+/-9=?A-Z^-~]+)*|
> +    \"([]!#-[^-~ \t]|(\\[\t -~]))+\")
> +    @
> +    ([-!#-'*+/-9=?A-Z^-~]+(\.[-!#-'*+/-9=?A-Z^-~]+)*|\[[\t -Z^-~]*])"""
> +
> +
> +class CreateUser(BaseModel):
> +    """
> +    A MAAS Site Manager User
> +    """
> +
> +    email: str = Field(

https://docs.pydantic.dev/usage/types/#pydantic-types

email: EmailStr

> +        title="email@xxxxxxxxxxx",
> +        max_length=250,
> +        min_length=3,
> +        regex=valid_email,
> +    )
> +    full_name: str
> +    password: SecretStr = Field(min_length=8, max_length=50)

hmm, where do these mins and maxes come from?

> +    disabled: bool
> +
> +
> +class User(CreateUser):
> +    id: int
>  
>  
>  class CreateSite(BaseModel):
> @@ -13,14 +46,13 @@ class CreateSite(BaseModel):
>      """
>  
>      name: str
> -    identifier: str
>      city: str | None
> -    latitude: str | None
> -    longitude: str | None
> +    latitude: str | Decimal | None

seems an unlikely union - do we want them to be str or Decimal? Given I don't think we ever want to capitalise them or translate them i suspect Decimal is a better choice

> +    longitude: str | Decimal | None
>      note: str | None
>      region: str | None
>      street: str | None
> -    timezone: str | None
> +    timezone: str | Decimal | None
>      url: str
>      # TODO: we will need to add tags
>  


-- 
https://code.launchpad.net/~thorsten-merten/maas-site-manager/+git/maas-site-manager/+merge/439266
Your team MAAS Committers is subscribed to branch ~thorsten-merten/maas-site-manager:MAASENG-1486-add-testdata.