emmett-framework / emmett

The web framework for inventors
Other
1.03k stars 70 forks source link

Empty select field returns empty string instead of None #465

Closed SvenKeimpema closed 10 months ago

SvenKeimpema commented 1 year ago

When submitting a form that has an empty select field, the Emmett framework's ORM module (emmett/orm/models.py) treats the empty string as a non-empty value. This probably occurs in the __set__ function at line 1154 where you are checking if the field is None which is not the case because the select html tag returns an empty string. Idk where the select field is parsed but it should probably need to check if it is an select html tag and then return None if nothing is selected.

Models:

class Requirement(Model):
    requirement_name = Field.string()
    description = Field.string()

class Taak(Model):
    #allow empty is set to true?
    refers_to({"requirement_id": "Requirement"},
              validation={'presence': True, 'allow': 'empty'})

controller:

@app.route("/add_taak")
@requires(auth.is_logged, url("/auth/login"))
async def taak():
    grid = await Taak.form()
    return dict(grid=grid)

config:

from emmett import App, url, redirect
from emmett.orm import Database
from emmett.sessions import SessionManager
from emmett.tools import Auth

app = App(__name__)
app.config.db.uri = "sqlite://storage.sqlite"
app.config.auth.hmac_key = "IJHOw2[038hr{}()Aujgf09uwWDO(00=-"
app.config.auth.single_template = True
app.config.auth.registration_verification = False

db = Database(app)
auth = Auth(app, db, user_model=User)

from .models.tables import *

db.define_models(Requirement,  Taak)

app.pipeline = [SessionManager.files(), db.pipe, auth.pipe]

auth_routes = auth.module(__name__)
gi0baro commented 1 year ago

I gonna inspect this carefully in the next few days.

Just a couple of things regarding the code you posted:

refers_to(
    {"requirement_id": "Requirement"},
    validation={'presence': True, 'allow': 'empty'}
)
SvenKeimpema commented 1 year ago

quick question can refers_to already be null/None/empty by default? however that should't matter in this case because it should give an may-not-be empty exception or something then.

gi0baro commented 1 year ago

refers_to already won't require the field to be filled (is on the differences compared to belongs_to. This is why I need some time to debug this, as it should work.

SvenKeimpema commented 1 year ago

This is a possible fix in RowReferenceInt(helpers.py):

class RowReferenceInt(RowReferenceMixin, int):
    def __new__(cls, id, table: Table, *args: Any, **kwargs: Any):
        try:
            rv = super().__new__(cls, id, *args, **kwargs)
        except ValueError:
            return None
        int.__setattr__(rv, '_refmeta', RowReferenceMeta(table, int))
        int.__setattr__(rv, '_refrecord', None)
        return rv

this isn't a great sollution however it does work with what i have tested