WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

fix: TASK-#15126 Airtable queries able to show the total count of rec… #34467

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.

Sign up for GitHub

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

Open
Naveen-Goud wants to merge 6 commits into
base: release
Choose a base branch
from
Open

fix: TASK-#15126 Airtable queries able to show the total count of rec… #34467

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1389812
fix: TASK-#15126 Airtable queries able to show the total count of rec…
Naveen-Goud Jun 25, 2024
74b7b08
fix: removed duplicate code
Naveen-Goud Jun 25, 2024
d49ebe5
fix: added a function to avoid code duplication
Naveen-Goud Jun 26, 2024
c2b4caf
fix: updated the function ParseResponseBody
Naveen-Goud Jun 26, 2024
28468bb
fix:removed else block from function
Naveen-Goud Jun 27, 2024
97784ad
Merge branch 'release' into FE_TASK-#15126
Naveen-Goud Aug 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,15 @@ function QueryDebuggerTabs({
if (isString(actionResponse.body)) {
try {
// Try to parse response as JSON array to be displayed in the Response tab
output = JSON.parse(actionResponse.body);
const parsedOutput = JSON.parse(actionResponse.body);
console.log("Parsed output:", parsedOutput);
if (Array.isArray(parsedOutput)) {
output = parsedOutput;
} else if (parsedOutput.records && Array.isArray(parsedOutput.records)) {
output = parsedOutput.records;
} else {
output = [parsedOutput];
}
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 25, 2024

Choose a reason for hiding this comment

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

Refactor JSON parsing logic for clarity and error handling.

The current approach to parsing the JSON response and handling different data structures is a bit convoluted. It's great to see the handling of different formats, but this can be made cleaner and more robust.

  • Error Handling: There's a catch block for JSON parsing errors, which is good. However, the error itself is not logged or handled beyond creating a default output structure. This might obscure the underlying issue when debugging.
  • Code Duplication: The logic to check and assign output based on whether it's an array or contains a records array is repeated for both parsed JSON objects and other object types.

Consider refactoring this to reduce duplication and improve error visibility.

function parseResponseData(responseBody) {
  try {
    const parsed = JSON.parse(responseBody);
    if (Array.isArray(parsed)) {
      return parsed;
    } else if (parsed.records && Array.isArray(parsed.records)) {
      return parsed.records;
    }
    return [parsed];
  } catch (e) {
    console.error("Error parsing response:", e);
    return [{ response: responseBody }];
  }
}

// Usage
output = parseResponseData(actionResponse.body);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const parsedOutput = JSON.parse(actionResponse.body);
console.log("Parsed output:", parsedOutput);
if (Array.isArray(parsedOutput)) {
output = parsedOutput;
} else if (parsedOutput.records && Array.isArray(parsedOutput.records)) {
output = parsedOutput.records;
} else {
output = [parsedOutput];
}
function parseResponseData(responseBody) {
try {
const parsed = JSON.parse(responseBody);
if (Array.isArray(parsed)) {
return parsed;
} else if (parsed.records && Array.isArray(parsed.records)) {
return parsed.records;
}
return [parsed];
} catch (e) {
console.error("Error parsing response:", e);
return [{ response: responseBody }];
}
}
// Usage
output = parseResponseData(actionResponse.body);

} catch (e) {
// In case the string is not a JSON, wrap it in a response object
output = [
Expand All @@ -162,7 +170,15 @@ function QueryDebuggerTabs({
];
}
} else {
output = actionResponse.body as any;
if (typeof actionResponse.body === "object" && actionResponse.body !== null) {
if (Array.isArray(actionResponse.body)) {
output = actionResponse.body;
} else if ('records' in actionResponse.body && Array.isArray((actionResponse.body as any).records)) {
output = (actionResponse.body as any).records;
} else {
output = [actionResponse.body];
}
}
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Refactor suggestion: Simplify and unify response handling logic.

The changes introduced to handle different types of actionResponse.body are a good step towards making the system more robust. However, as previously commented, this block still suffers from code duplication and could be clearer.

Considering the previous comments and the complexity of handling various data structures, it might be beneficial to centralize this logic into a single function. This would not only reduce duplication but also enhance maintainability and clarity.

Consider the following refactor:

function parseResponseData(responseBody) {
  if (typeof responseBody === 'string') {
    try {
      return JSON.parse(responseBody);
    } catch (e) {
      return [{ response: responseBody }];
    }
  } else if (responseBody && typeof responseBody === 'object') {
    if (Array.isArray(responseBody)) {
      return responseBody;
    } else if ('records' in responseBody && Array.isArray(responseBody.records)) {
      return responseBody.records;
    }
    return [responseBody];
  }
  return [];
}

// Usage
output = parseResponseData(actionResponse.body);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (typeof actionResponse.body === "object" && actionResponse.body !== null) {
if (Array.isArray(actionResponse.body)) {
output = actionResponse.body;
} else if ('records' in actionResponse.body && Array.isArray((actionResponse.body as any).records)) {
output = (actionResponse.body as any).records;
} else {
output = [actionResponse.body];
}
}
function parseResponseData(responseBody) {
if (typeof responseBody === 'string') {
try {
return JSON.parse(responseBody);
} catch (e) {
return [{ response: responseBody }];
}
} else if (responseBody && typeof responseBody === 'object') {
if (Array.isArray(responseBody)) {
return responseBody;
} else if ('records' in responseBody && Array.isArray(responseBody.records)) {
return responseBody.records;
}
return [responseBody];
}
return [];
}
// Usage
output = parseResponseData(actionResponse.body);

Copy link

@akshayvijayjain akshayvijayjain Jun 25, 2024

Choose a reason for hiding this comment

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

@Naveen-Goud Thank you for changes.

Please update the code as per coderabbit suggestion and do comment if the suggestion is invalid.

Copy link
Contributor

@coderabbitai coderabbitai bot Jun 25, 2024

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

}
}

Expand Down