betodealmeida commented on a change in pull request #18056:
URL: https://github.com/apache/superset/pull/18056#discussion_r806390564



##########
File path: superset-frontend/src/views/CRUD/utils.test.tsx
##########
@@ -171,3 +172,21 @@ test('does not ask for password when the import type is 
wrong', () => {
   };
   expect(hasTerminalValidation(error.errors)).toBe(true);
 });
+
+test('successfully modified rison to encode correctly', () => {
+  const problemCharacters = '& # ? ^ { } [ ] | " = + `';
+
+  const testObject = problemCharacters.split(' ').reduce((a, c) => {
+    // eslint-disable-next-line no-param-reassign
+    a[c] = c;
+    return a;
+  }, {});
+
+  const actualEncoding = rison.encode(testObject);
+
+  const expectedEncoding =
+    
"('\"':'\"','#':'#','&':'&','+':'+','=':'=','?':'?','[':'[',']':']','^':'^','`':'`','{':'{','|':'|','}':'}')";

Review comment:
       Can we make this more readable?

##########
File path: superset-frontend/src/views/CRUD/utils.tsx
##########
@@ -33,6 +33,35 @@ import { FetchDataConfig } from 'src/components/ListView';
 import SupersetText from 'src/utils/textUtils';
 import { Dashboard, Filters } from './types';
 
+// Modifies the rison encoding slightly to match the backend's
+// rison encoding/decoding. Applies globally. Code pulled from rison.js

Review comment:
       Nit, can you add here that `rison.js` is licensed under the MIT license, 
so we know we can reuse it? We also need to have a link to the original 
project, since the MIT license requires crediting.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to