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

QuerySelectField query parameter ignored when the query returns no result. #135

Open
TrilceAC opened this issue Nov 29, 2018 · 1 comment

Comments

@TrilceAC
Copy link
Contributor

According to the documentation:

The query property on the field can be set from within a view to assign a query per-instance to the field. If the property is not set, the query_factory callable passed to the field constructor will be called to obtain a query

But this fails if the query returns no results. In those cases, the query_factory is used. If there is one, the choices of the QuerySelectField are those returned by the query_factory. Otherwise, when no query_factory has been passed, rendering the field raises the following exception: TypeError: 'NoneType' object is not callable.

This is unfortunate in some cases, like for example when dealing with forms to handle trees, which is my case. Since a Tree has no cycle, I want to avoid that the edition of an element sets its parent to itself or one of its children. Therefore, if there is a single tree, when editing a root element, it should have no candidate for being it parent.

I provide a simplified code which is not tested, i.e. some variables have been renamed and mistakes can occur:

class Node(Model):
    MAXLEN_NAME = 128

    __tablename__ = 'node'
    id = Column(Integer, primary_key=True, index=True)
    name = Column(
        Unicode(MAXLEN_NAME),
        nullable=False,
        index=True,
    )

    parent_id = Column(
        Integer,
        ForeignKey('node.id'),
        index=True
    )
    parent = relationship(
        'Node',
        remote_side='Node.id',
        backref='children'
    )

    def descendants(self):
        """Somehow get the descendants."""
        # OPTIMIZE !!!
        return (
            frozenset(self.children) if self.children else frozenset() | {
                desc.descendant() for desc in self.children
            }
        )

This is the form:

class NodeForm(OrderFormMixin, ModelForm):

    class Meta:

        model = Node
        strip_string_fields = True
        order = (
            'name',
            'parent',
            'submit',
            '*'
        )
    parent = QuerySelectField(
        label='Parent',
        query_factory=lambda: (
            Node.query
            .order_by(Node.name)
            .all()
        ),
        allow_blank=True,
        description='direct parent of the node.'
    )
    enviar = SubmitField()

And finally the view...

@blueprint.route('/edit/<int:node_id>', methods=('GET', 'POST'))
def edit(node_id):
    node = Node.query.options(
        joinedload(Node.parent)
    ).get_or_404(node_id)

    form = NodeForm(obj=node)

    # Do not put invalid options on the QuerySelectField
    # THIS FAILS if there is no candidate, i.e. the query returns []
    form.parent.query = (
        Node.query
        .filter(
            Node.id != node.id,
            Node.id.notin_(
                desc.id for desc in node.descendant()
            )
        )
        .order_by(Node.name)
        .all()
    )

    if form.validate_on_submit():
        form.populate_obj(node)
        db.session.commit()
        return redirect(url_for('.index'))

    return render_template(
        'node/alta.html.j2',
        title='Editar ubicación {0}'.format(node),
        form=form
    )
@TrilceAC
Copy link
Contributor Author

I've found the bug. The problem occurs when deciding whether either query or query_factory should be used: _get_object_list should be fixed as follows:

    def _get_object_list(self):
        if self._object_list is None:
            # query = self.query or self.query_factory() # <--- FAILS when query returns [].
            query = self.query if (self.query or self.query==[]) else self.query_factory() # <--- FIX!
            get_pk = self.get_pk
            self._object_list = list(
                (text_type(get_pk(obj)), obj) for obj in query
            )
        return self._object_list

TrilceAC pushed a commit to TrilceAC/wtforms-alchemy that referenced this issue Nov 29, 2018
There was a bug when deciding to use either query or query_factory in QuerySelectField. Queries that returned no value where silently ignored in favor of query_factory. This has been fixed. 

Affected classes:
QuerySelectField.
GroupedQuerySelectField
GroupedQuerySelectMultipleField

This solves issue kvesteri#135
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

1 participant