The action URLs routes currently cannot contain variable types or they will not work with the render_table macro

[greyli]

The action url routes currently cannot contain variable types or they will not work with the macro. Example: @app.route("/a/<int:b>/c")

Originally posted by @caffeinatedMike in #135 (comment)

[greyli]

Some additional info. When passing :primary_key as the primary key placeholder to an action URL in render_table. If the route contains a URL variable with a converter, for example, <int:foo_id>. Then, the int converter will try to convert the placeholder :primary_key to an integer, so the error happens (ValueError: invalid literal for int() with base 10: ':primary_key'). I can't think of a solution right now. @michaelbukachi Any ideas?

[michaelbukachi]

Hey @greyli it's because the placeholder must be a string because inside the render_table action it's replaced with the actual value.
Supposed you have the following endpoint:

@app.route('users/<user_id>')
def users(user_id)
...

then you pass url_for('users', user_id=':primary_key') to the view_url parameter of the render_table macro, this generate a url of the type <some host>/users/:primary_key

While rendering the rows, the macro will then replace :primary_key with the actual primary key value for that specific row.

Initially, I had thought of putting the url_for inside the macro, but then I realized it might have multiple parameters. From the route_name to all path parameters for that route. Taking into consideration the edit, delete URLs, the macro would require quite a lot of parameters.

In summary, the reason we can't add types to path parameters is that :primary_key needs to be set temporarily before being replaced.

[michaelbukachi]

We can have the view_url and related parameters take tuples/dicts instead and then move the url_for calls inside the macro. This way, no temporary variable would be needed so we could use types when defining path parameters.

[caffeinatedMike]

@michaelbukachi What about using a Custom Converter? That seems like a more practical approach that aligns with the idea of keeping the macros as hands-off and convenient as possible (not having to pass even more parameters). I think this way we could manage a custom converter and only have to worry about registering it once, when the extension is initialized. Thoughts?

[michaelbukachi]

Hmmm. Do you mean using something like <int(table=True):foo>. Wouldn't we have to override the converters for string and int?

[caffeinatedMike]

Maybe possibly using a custom one to avoid overriding the built-ins, like <model_int(ModelName):foo>?

[michaelbukachi]

Maybe possibly using a custom one to avoid overriding the built-ins, like <model_int(ModelName):foo>?

We would also have to factor strings. Strings are used frequently these days as primary keys especially when as uuids.

[caffeinatedMike]

@michaelbukachi Correct, sorry that I wasn't clear. I was implying we create a custom one for ints and another for strings. So, something like <model_int(ModelName):foo> and <model_str(ModelName):foo>

[michaelbukachi]

That works. @greyli what do you think?

[greyli]

Sounds good to me. We can discuss this further in the PR.

[caffeinatedMike]

After further consideration, I feel like it'd be ideal to keep the original int and string url converters. I think I may have come up with a possible solution that allows for this (correct me if I'm wrong @greyli @michaelbukachi).

The first thing that would need to happen is a re-working of the way we construct the urls with placeholders. Possible solution:

• Add a parameter to render_table called model where we pass in the model displayed in the table.
• Now having access to the db.Model in the view we can adopt the same approach that's used with render_pagination: adding endpoint and args as parameters.
• In this case though, args would be where we allow placeholders for fields, requiring any placeholder to begin with a colon (:)
• Then, in table.html while building each <tr> we use the action_pk_placeholder to grab the record using current_record = Model.query.get(action_pk_placeholder) and loop through args to fetch the fields' values using getattr(current_record, arg_placeholder) . I think the deepest I'd go in terms of lookups would be two-deep.
  Example:
  • :slug would grab the slug field of the current record
  • :portal.id would first grab the portal field of the current record (in this case portal is a relationship field), then grab the id from that (getattr('id', getattr('portal', {}), None))
• Finally, we use those collected values to build the correct url using url_for

Benefits of this approach:

• We resolve the inability to utilize paths with the built-in type-converters (<int:value> and <string:value>) when using this package
• We offer a much more extensible route-building experience that's flexible and aligns with an approach that is already implemented elsewhere in this package (render_pagination)
• This would take care of this bug, as well as the feature request Feature Request: Allow placeholders for other db model fields in render_table macro's action url_fors #145

Downside of this approach:

• The only downside I can think of is using model objects directly inside of templates, which I'm not sure if that is detrimental to rendering performance.

Thoughts?