Pegase745 / sqlalchemy-datatables

SQLAlchemy integration of jQuery DataTables >= 1.10.x (Pyramid and Flask examples)
MIT License
159 stars 67 forks source link

use of self.columns index rather than mData value problematic #131

Open louking opened 4 years ago

louking commented 4 years ago

I'm having trouble lining up the self.columns index with the index which comes from datatables(js) in the search item. My indexes don't match sometimes because I have to post process the output_result to get an object like field: {id:idval, label:labelval} which is expected by datatables(js). Possibly this object can be returned directly by sqlalchemy-datatables but after several days of research I haven't been able to find the magic elixir to make that work.

example response snippet

columns[4][data]: assignee.name
columns[4][name]: assignee.name
columns[4][searchable]: true
columns[4][orderable]: true
columns[4][search][value]: Lou King
columns[4][search][regex]: true

Is there a reason you're assuming the datatables(js) index (4) matches the sqlalchemy-datatables index, rather than matching the configured mData with the data from the query parameter columns[4][data]: assignee.name in this case assignee.name?

E.g., see use of index in https://github.com/Pegase745/sqlalchemy-datatables/blob/049ab5f98f20ad37926fe86d5528da0c91cd462d/datatables/datatables.py#L140-L148

louking commented 4 years ago

Not sure, but this might be useful: https://github.com/orf/datatables/blob/dec3acc46898d611ea951b057cecd909c023d086/datatables/__init__.py#L59-L81

kartikeyas00 commented 4 years ago

Could you post your code? Like how are you post-processing the data?

louking commented 4 years ago

Could you post your code? Like how are you post-processing the data?

My code is pretty convoluted because it is embedded in a class which has grown over time. This class is used for both serverside use of datatables and non-serverside use. The serverside use uses sqlalchemy-datatables. So the code is pretty convoluted as to the hows and whys. But to answer your question directly, the post-processing is at https://github.com/louking/loutilities/blob/caeec07278e7a80417b0b193b907ea11aefeddac/loutilities/tables.py#L2721-L2746

Hmm, my check for error in output would be better before the post-processing. But I digress.

Here's the code directly

            output = rowTable.output_result()

            # kludge? to take items in object notation, and make into object
            # louking/members#152 (search for this to find required matching code
            # TODO: only handles one level deep - should we make this recursive?
            for row in output['data']:
                delfields = []
                addfields = {}
                for field in row:
                    splitobj = field.split('.')
                    # if there are multiple levels to the fieldname, need to make it into an object
                    if len(splitobj) > 1:
                        # this will cause exception if more than two levels; that's ok for now, see todo above
                        parent, child = splitobj
                        addfields.setdefault(parent,{})
                        addfields[parent][child] = row[field]
                        delfields.append(field)
                row.update(addfields)
                for field in delfields:
                    del row[field]

            # check for errors
            if 'error' in output:
                raise ParameterError(output['error'])

            self.output_result = output
kartikeyas00 commented 4 years ago

It seems like you are adding a new column in the output produced by the sqlalchemy-datatables extension, correct? I am sure it can definitely be handed in the sqlalchemy-datatables extension but I think that there is no one who is overseeing this library anymore.

Right now you can manipulate the request coming in which you feed in the datatables extension. By this I meant that you can remove certain columns from the response coming in. Does that make sense?

louking commented 4 years ago

I already have a workaround for this issue -- this is why I'm adding these to the end. I guess if I really want to make this extension work better for me I could fork it and modify at will. Sorry to hear there's nobody working on it anymore. I thought someone did take over from @Pagase745 (@tdamsma) but I guess that wasn't long lived.

tdamsma commented 4 years ago

I'm still around, but not too often. I missed this discussion, will have a look. PR's are always welcome

Pegase745 commented 4 years ago

Yeah @louking don't hesitate to send some PRs. I personally don't use this package anymore but it doesn't mean it should die :)

I will take some time to automate packaging and publishing via GitHub Actions, allowing easier releases.

louking commented 4 years ago

Thanks. I considered a PR for this one, but don't understand the reasoning behind the current design, and hesitate to make wholesale change suggestions. And I needed a quick "fix" for my application's progress so ended up with the workaround.

louking commented 4 years ago

Also I just realized the workaround I quoted is only for the object notation handling. My workaround for the index ordering is more complex and would be hard to find now. Also this might be a specific use case required for my largish datatables handling class, found in https://github.com/louking/loutilities/blob/master/loutilities/tables.py, supported by files in https://github.com/louking/loutilities/tree/master/loutilities/tables-assets

kartikeyas00 commented 3 years ago

@tdamsma @Pegase745 @louking I have opened a pull request to handle this issue and also offered some more improvements which would be nice to have. Tests has also been added. Please have a look. Thanks!