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

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

Open
louking opened this issue Jul 23, 2020 · 10 comments
Open

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

louking opened this issue Jul 23, 2020 · 10 comments

Comments

@louking
Copy link
Collaborator

louking commented Jul 23, 2020

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

# per columns filters:
for i in range(len(self.columns)):
filter_expr = None
value = self.params.get('columns[{:d}][search][value]'.format(i),
'')
if value:
search_func = SEARCH_METHODS[self.columns[i].search_method]
filter_expr = search_func(self.columns[i].sqla_expr, value)
self.filter_expressions.append(filter_expr)

@louking
Copy link
Collaborator Author

louking commented Jul 23, 2020

@kartikeyas00
Copy link

kartikeyas00 commented Aug 25, 2020

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

@louking
Copy link
Collaborator Author

louking commented Aug 25, 2020

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
Copy link

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
Copy link
Collaborator Author

louking commented Aug 25, 2020

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
Copy link
Collaborator

tdamsma commented Aug 25, 2020

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

@Pegase745
Copy link
Owner

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
Copy link
Collaborator Author

louking commented Aug 26, 2020

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
Copy link
Collaborator Author

louking commented Aug 26, 2020

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
Copy link

@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!

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

4 participants