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

URLFor and AbsoluteURLFor fail if URL attribute is None #68

Closed
ocervell opened this issue Aug 31, 2017 · 3 comments

Comments

@ocervell
Copy link

commented Aug 31, 2017

This issue has been reported before in #18 but I wanted to provide a simple example showing how this is a problem for a lot of use cases.

Example code

class User(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    group_id = Column(db.Integer, db.ForeignKey('groups.id'), default=None)
    group = db.relationship(Group)

class UserSchema(ma.ModelSchema):
   class Meta:
      model = User
   group = ma.AbsoluteURLFor('groups', id='<group_id>')

class Group(db.Model):
   id = Column(Integer, primary_key=True)

Issue

Currently, the AbsoluteURLFor and URLFor methods will fail with a werkzeug.routing.BuildError if group_id is not set on the User model.

I know this problem originates directly from Flask url_for method, that would fail if id is None, but there should be a way in Flask-Marshmallow to skip serialization for this field when None.

Fix

I am using a temporary fix which is to add a custom handler for BuildError errors.
The following will replace any BuildError by an empty string:

handler = lambda error, endpoint, values: ''
app.url_build_error_handlers.append(handler)

There is two problems with this:

  • Too broad: disables all BuildError, even for normal routes.
  • Limited: handler function cannot return None (which for me would be the accepted return value), because Flask will re-raise the BuildError in that case.

Solutions

  • Add a flag skip_if_errors to allow skipping a field if encountering a BuildError
  • Modify the implementation to catch that usecase and return None
@znatty22

This comment has been minimized.

Copy link

commented Apr 5, 2018

Hey @ocervell I'm running into this issue as well. I see there is a PR #72 out to fix a similar issue with HyperlinkRelated, but I don't see anything to address this issue with Hyperlinks. Do you know if this issue has gotten any attention?

@ocervell

This comment has been minimized.

Copy link
Author

commented Aug 20, 2018

It doesn't seem that there is much attention for this unfortunately...

sloria added a commit that referenced this issue Mar 10, 2019

@sloria sloria closed this in #125 Mar 10, 2019

@sloria

This comment has been minimized.

Copy link
Member

commented Mar 10, 2019

Sorry for the long delay on this. This is now fixed in 0.10.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.