You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/12/19 01:01:25 UTC

[GitHub] [incubator-superset] ktmud commented on a change in pull request #12113: feat: display suggestions when query has missing parameters

ktmud commented on a change in pull request #12113:
URL: https://github.com/apache/incubator-superset/pull/12113#discussion_r546166190



##########
File path: superset-frontend/src/components/ErrorMessage/ParameterErrorMessage.tsx
##########
@@ -32,17 +33,76 @@ interface ParameterErrorExtra {
   }[];
 }
 
-const triggerMessage = t('This may be triggered by:');
+const maxDistanceForSuggestion = 2;
+const findMatches = (
+  undefined_parameters: string[],
+  template_keys: string[],
+) => {
+  const matches: { [undefined_parameter: string]: string[] } = {};
+  undefined_parameters.forEach(undefined_parameter => {
+    template_keys.forEach(template_key => {
+      if (
+        levenshtein(undefined_parameter, template_key) <=
+        maxDistanceForSuggestion
+      ) {
+        if (!matches[undefined_parameter]) {
+          matches[undefined_parameter] = [];
+        }
+        matches[undefined_parameter].push(`"${template_key}"`);
+      }
+    });
+  });
+  return matches;
+};
 
 function ParameterErrorMessage({
   error,
   source = 'sqllab',
 }: ErrorMessageComponentProps<ParameterErrorExtra>) {
   const { extra, level, message } = error;
 
+  const triggerMessage = tn(
+    'This was triggered by:',
+    'This may be triggered by:',
+    extra.issue_codes.length,
+  );
+
+  const matches = findMatches(
+    extra.undefined_parameters || [],
+    Object.keys(extra.template_parameters || []),
+  );
+
   const body = (
     <>
       <p>
+        {matches && (
+          <>
+            <p>{t('Did you mean:')}</p>
+            <ul>
+              {Object.entries(matches).map(
+                ([undefined_parameter, template_keys]) => (
+                  <li>
+                    {tn(
+                      '%(suggestion)s instead of "%(undefined)s?"',

Review comment:
       Translations will not be available until they were read from the bootstrap data via [`preamble.ts`](https://github.com/airbnb/incubator-superset/blob/9a59bdda486f90b8faeafdc79af3bcdbb7c3d250/superset-frontend%2Fsrc%2Fpreamble.ts#L32) (prepended to every JS entry point script).
   
   I think the usage here is fine, but I would recommend rename `undefined` to `undefined_vars` (including on the Python side), as `undefined` is a reserved word in JavaScript. This may cause confusion.
   
   Another solution is to instead of using translation strings, you could present them as a two column table with "Variable Used" and "Suggestion(s)".




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org