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

Added template helpers functions to encode and decode special charact… #6747

Conversation

JVickery-TBS
Copy link
Contributor

…ers in the filters for datatables view.

Fixes #

Proposed fixes:

Encoding and decoding special characters for the datatables view filters. Special characters in fanstatic throw ansii encoding errors.

Features:

  • includes tests covering changes
  • includes updated documentation
  • includes user-visible changes
  • includes API changes
  • includes bugfix for possible backport

Please [X] all the boxes above that apply

@amercader amercader self-assigned this Mar 15, 2022
Copy link
Member

@amercader amercader left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @JVickery-TBS, see comment below.

Also we should a small test for this. Something in ckan/tests/lib/test_helpers.py that checks that the output of the functions is the expected one.
Cheers


def get_helpers(self):
return{
'encode_datatables_request_filters': helpers.encode_datatables_request_filters
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JVickery-TBS This sounds like a valuable helper for other view plugins as well. Shall we rename them to encode/decode_view_request_filters move it to ckan/lib/helpers.py so it's available to everybody?

def encode_datatables_request_filters():
if 'filters' not in request.params:
return ''
return escape(quote(str(request.params['filters'].encode('utf-8')).encode('ascii')))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use request.args instead, as that's the Flask supported name. Same in the function below

…n global helpers. Modified the encoding function. Wrote tests for the two new helpers. Modified ckanext/datatablesview templates and methods to use new global helpers.
…and colons prior to decoding, returning a dictionary now. Added todo comments.
Copy link
Member

@amercader amercader left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good @JVickery-TBS, see a small comment and can you fix the conflicts with master before we merge? cheers

@@ -60,6 +60,18 @@ this.ckan.module('resource-view-filters', function (jQuery) {
}

function _appendDropdowns(dropdowns, resourceId, template, fields, filters) {
//TODO: handle decoding the filters object to be able to properly available in the current filter dropdowns
console.log(filters);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe remove this to avoid noise

Suggested change
console.log(filters);
console.log(filters);

…alues of the filters. Modified the remove filter event to remove the correct index. Modified the decode_view_request_filters method, fixing nested arrays. Modified datatables views blueprint merge_filters method to work with the new filters helper. Modified the test for the new helper method return.
@JVickery-TBS
Copy link
Contributor Author

@amercader Hey!

Okay I believe I have accomplished the desired affect here with the view filters. There were cases in which users may have colons (:) and/or pipes (|) in their data. This would break the filters because the filters are flattened. The filter names and values are separated by colons and multiple filters are separated by pipes.

So what will be done now is that the filter names and values will be URI encoded in JS before they are flattened into the colon and pipe string. The new helper now does the separation of the colons and pipes, THEN decodes the filter names and values. The helper now returns a dictionary, the properties being the filter names, and the values being arrays containing the filter values.

Ultimately this will allows for special accented characters in filter names and values. It will also allow for colons and pipes in the filter names and values.

@amercader
Copy link
Member

Thanks @JVickery-TBS, can you add a changes/6747.misc file with a short summary for the changelog? Thanks

@JVickery-TBS
Copy link
Contributor Author

@amercader changes/6747.misc should be there now. And I just fixed some syntax stuff and added/modified some annotations as to pass all of the tests.

@amercader amercader merged commit 38cec27 into ckan:master Apr 28, 2022
@amercader
Copy link
Member

Thanks @JVickery-TBS

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

Successfully merging this pull request may close these issues.

2 participants