render_table without SQL-Alchemy and Action URLs

[Markus-]

Hi,

I tried to render a table with row-specific URLs without using a Model.
As I understand the code, the problem is, that if I use a string as URL it is more or less static. When I want to create a dynamic (row specific) url I need to hand over a Model (which I do not have).

I just use a JSON-Object as data and a dict as titles.

    products = requests.get("http://localhost:3000/product").json()
    titles = [('name', 'Product'),('long_name', 'Produkt Lang Name')]

    return render_template('products/index.html', products = products, titles = titles)

Without any action urls the rendering works as expected.

        {{ render_table(products, titles, primary_key='name', show_actions=True, custom_actions=[('Run', 'play-fill', 'modals')]) }}

Any idea how to add something dynamic?

... 'modals/' + row[0] + '/edit'

Thanks
Markus

[PanderMusubi]

I think the current implementation does not support that, but you might handle it inside the implementation of the supported action urls. By looking at the values passed, either forward to the desired URL or call the appropriate method in there (with some if-elif-elif-else). Does that help you?

[nickovs]

I ran into the same issue, so I looked into the code for the render_table() macro. Fixing this is pretty straightforward and in fact I've hacked a version for myself that allows actions on dicts and non-model objects.

The issue is that the internal macro build_url() that is used to build the action link URLs takes a very odd approach to finding the fields to be inserted into the link. All the calls to this macro are inside the loop that iterates over the rows, but rather than passing the row itself into the macro, it passes the model class and the primary key for the current row and then fetches the record all over again for every action while building the links.

Not only does this break the support for actions on non-SQLAlchemy records, but it also means that there are up to an extra 3 database queries for every row, despite the data already being there.

My quick hack this evening just changed the URL building macro from build_url(endpoint, model, pk, url_tuples) to build_url(record, endpoint, url_tuples), stripped out the lookup of the record with model.query.get(pk) and then fixed the four calls to the function to pass the row variable instead of the model and primary key.

This fix seems to work for my needs, although this is the first time I've looked at the bootstrap-flask code so there may be edge cases I'm not aware of. I will try to package this up as a PR and submit it, but that might need to wait until the weekend.

[Markus-]

I think we can close the issue. I tried to go the way as nick, but in the end I switched over to w3.css framework. Thanks for looking into.

Regards
Markus

[PanderMusubi]

Thanks, but reopening for Nick's PR.

[Markus-]

Sorry for this. I am new to PR / Issue Handling.

[PanderMusubi]

No worries. 😃

[nickovs]

@PanderMusubi I have a couple of questions regarding the use cases for this code.

Firstly, for the hack I did, I no longer used to model argument, so I ended up removing it from the parameters for the macro. This is a breaking API change. I'm guessing at the very least you'd like that parameter present, even if it's not used; is that correct?

Secondly, one thing that is possible with the old code, if you are using an SQLAlchemy database, is to pass a different model class to the class of the row data, as long as it has the same primary key. This is not a documented use case; it's just something that currently works. Since the model class is only used in the URL generation, in theory it is possible to pull the fields used for the URL arguments from a different database table, instead of using the row data that was passed in to the macro. I can see scenarios where someone might want to do this (although, IMHO, none of them represent good programming style). One option is therefore to make the model parameter optional, and then to use it if it is provided but to use the row data directly if the model class if not provided. Do you think that this would be useful? If so I'd update the documentation to make it clear that now you only need to give a model class if you want to do this. On the up side, this would offer some extra (rather niche) flexibility and preserve existing behaviour for existing code. On the down side, doing this would mean that existing code would not get the performance benefits of not making 3*N extra database queries for an N row table.

Let me know your preference on these to issues and I will package up a suitable PR.

[PanderMusubi]

I will let @greyli answer this first. Adding optional parameters is never a problem, breaking the API if we can't avoid it we can talk about and could result in a version 3 e.g., especially if the implementation is faster.

[nickovs]

I will look forward to @greyli 's opinion. In the mean time I'll change my code to be fully backward compatible, including support for using a different model class, with the performance improvements only showing if users remove the model argument from existing code. I will include an update to docs/macros.rst in the PR to clarify the usage.