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.

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

Jump to bottom

[Components] mitra #14368

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[Components] mitra #14368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 49 additions & 0 deletions components/mitra/actions/delete-data/delete-data.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import app from "../../mitra.app.mjs";

export default {
key: "mitra-delete-data",
name: "Delete Data",
description: "Deletes a record from a table in the Mitra database. [See the documentation]()", // Add documentation link if available
version: "0.0.1",
type: "action",
props: {
app,
tableName: {
propDefinition: [
app,
"tableName",
],
},
dimensionContentId: {
type: "string",
label: "Dimension Content ID",
description: "The unique identifier of the record to delete.",
},
},
methods: {
deleteData({
tableName, dimensionContentId, ...args
} = {}) {
return this.app.delete({
tableName,
path: `/${dimensionContentId}`,
...args,
});
},
},
async run({ $ }) {
const {
deleteData,
tableName,
dimensionContentId,
} = this;

const response = await deleteData({
tableName,
dimensionContentId,
});

$.export("$summary", "Succesfully deleted record from the Mitra database.");
return response;
},
};
Comment on lines +34 to +49
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix typo and consider adding error handling.

  1. There's a typo in the success message. Change "Succesfully" to "Successfully" on line 46.

  2. Consider adding error handling to the run method. This will make the action more robust and provide better feedback in case of failures.

Here's a suggested implementation with error handling:

async run({ $ }) {
  const {
    deleteData,
    tableName,
    dimensionContentId,
  } = this;

  try {
    const response = await deleteData({
      tableName,
      dimensionContentId,
    });

    $.export("$summary", `Successfully deleted record from the Mitra database.`);
    return response;
  } catch (error) {
    $.export("$summary", `Failed to delete record: ${error.message}`);
    throw error;
  }
}

47 changes: 47 additions & 0 deletions components/mitra/actions/get-data/get-data.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import mitra from "../../mitra.app.mjs";

export default {
key: "mitra-get-data",
name: "Get Data",
description: "Fetches data from the specified table, allowing dynamic filters via query parameters. [See the documentation](https://mitralab.io/docs/api)",
version: "0.0.1",
type: "action",
props: {
mitra,
tableName: {
propDefinition: [
mitra,
"tableName",
],
},
params: {
type: "object",
label: "Query Parameters",
description: "Dynamic filters for querying records (e.g., `status`, `hours_gt`).",
},
},
methods: {
fetchData({
tableName, ...args
} = {}) {
return this.app._makeRequest({
tableName,
...args,
});
},
},
async run({ $ }) {
const {
fetchData,
tableName,
params,
} = this;

const response = await fetchData({
tableName,
params,
});
$.export("$summary", "Succesfully fetched data from the database.");
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct the typo in the summary message.

There is a typo in the summary message on line 44. The word "Succesfully" should be spelled "Successfully".

Apply this diff to fix the typo:

-    $.export("$summary", "Succesfully fetched data from the database.");
+    $.export("$summary", "Successfully fetched data from the database.");
📝 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
$.export("$summary", "Succesfully fetched data from the database.");
$.export("$summary", "Successfully fetched data from the database.");

return response;
Comment on lines +40 to +45
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for the data fetching process.

Currently, the code does not handle potential errors from the fetchData call. To improve robustness, consider adding error handling to manage exceptions that may occur during data retrieval.

Example:

async run({ $ }) {
  const {
    fetchData,
    tableName,
    params,
  } = this;

  try {
    const response = await fetchData({
      tableName,
      params,
    });
    $.export("$summary", "Successfully fetched data from the database.");
    return response;
  } catch (error) {
    $.export("$summary", `Failed to fetch data: ${error.message}`);
    throw error;
  }
}

},
};
51 changes: 51 additions & 0 deletions components/mitra/actions/insert-data/insert-data.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import utils from "../../common/utils.mjs";
import app from "../../mitra.app.mjs";

export default {
key: "mitra-insert-data",
name: "Insert Data",
description: "Inserts one or more records into a table in the Mitra database.",
version: "0.0.7",
type: "action",
props: {
app,
tableName: {
propDefinition: [
app,
"tableName",
],
},
records: {
propDefinition: [
app,
"records",
],
},
},
methods: {
insertData({
tableName, ...args
} = {}) {
return this.app.post({
tableName,
...args,
});
},
},
async run({ $ }) {
const {
insertData,
tableName,
records,
} = this;

const response = await insertData({
$,
tableName,
data: utils.parseArray(records),
});

$.export("$summary", "Successfully inserted records into the Mitra database.");
return response;
},
};
51 changes: 51 additions & 0 deletions components/mitra/actions/update-data/update-data.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import app from "../../mitra.app.mjs";
import utils from "../../common/utils.mjs";

export default {
key: "mitra-update-data",
name: "Update Data",
description: "Updates one or more records in a table.",
version: "0.0.1",
type: "action",
props: {
app,
tableName: {
propDefinition: [
app,
"tableName",
],
},
records: {
propDefinition: [
app,
"records",
],
},
},
methods: {
updateData({
tableName, ...args
} = {}) {
return this.app.put({
tableName,
...args,
});
},
},
async run({ $ }) {
const {
updateData,
tableName,
records,
} = this;

const response = await updateData({
$,
tableName,
data: utils.parseArray(records),
});

$.export("$summary", "Successfully updated records into the Mitra database.");
return response;
},
};
50 changes: 50 additions & 0 deletions components/mitra/common/utils.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { ConfigurationError } from "@pipedream/platform";

function isJson(value) {
try {
JSON.parse(value);
} catch (e) {
return false;
}

return true;
}
Comment on lines +3 to +11
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure correct handling of non-string inputs in isJson function

The isJson function may not correctly handle non-string inputs like objects or numbers since JSON.parse expects a string input. Passing a non-string value will result in a TypeError. To improve robustness, consider adding a type check to ensure that the value is a string before attempting to parse it.

Apply this diff to fix the issue:

 function isJson(value) {
+  if (typeof value !== "string") {
+    return false;
+  }
   try {
     JSON.parse(value);
   } catch (e) {
     return false;
   }
   return true;
 }
📝 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
function isJson(value) {
try {
JSON.parse(value);
} catch (e) {
return false;
}
return true;
}
function isJson(value) {
if (typeof value !== "string") {
return false;
}
try {
JSON.parse(value);
} catch (e) {
return false;
}
return true;
}


function valueToObject(value) {
if (typeof(value) === "object") {
return value;
}

if (!isJson(value)) {
throw new ConfigurationError(`Make sure the custom expression contains a valid JSON object: \`${value}\``);
}

return JSON.parse(value);
}
Comment on lines +13 to +23
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle null values explicitly in valueToObject function

In the valueToObject function, typeof null returns "object", which means null values are returned as-is. This might lead to unexpected behavior if null is not an acceptable object in your context. Consider adding a check to handle null values explicitly.

Apply this diff to address the issue:

 function valueToObject(value) {
-  if (typeof(value) === "object") {
+  if (typeof value === "object" && value !== null) {
     return value;
   }
 
   if (!isJson(value)) {
     throw new ConfigurationError(`Make sure the custom expression contains a valid JSON object: \`${value}\``);
   }
 
   return JSON.parse(value);
 }
📝 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
function valueToObject(value) {
if (typeof(value) === "object") {
return value;
}
if (!isJson(value)) {
throw new ConfigurationError(`Make sure the custom expression contains a valid JSON object: \`${value}\``);
}
return JSON.parse(value);
}
function valueToObject(value) {
if (typeof value === "object" && value !== null) {
return value;
}
if (!isJson(value)) {
throw new ConfigurationError(`Make sure the custom expression contains a valid JSON object: \`${value}\``);
}
return JSON.parse(value);
}


function parseArray(value) {
try {
if (!value) {
return [];
}

if (Array.isArray(value)) {
return value;
}

const parsedValue = JSON.parse(value);

if (!Array.isArray(parsedValue)) {
throw new Error("Not an array");
}

return parsedValue;
Comment on lines +35 to +41
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Optimize parsing logic in parseArray function

Currently, the code attempts to parse the value with JSON.parse without checking if it's a string. If value is a non-string (e.g., number, object), JSON.parse might throw an error. To enhance the function's robustness, check if value is a string before parsing.

Apply this diff to optimize the parsing logic:

     if (Array.isArray(value)) {
       return value;
     }

+    if (typeof value !== "string") {
+      throw new Error("Provided value is not a string or an array");
+    }
+
     const parsedValue = JSON.parse(value);

     if (!Array.isArray(parsedValue)) {
       throw new Error("Not an array");
     }

Committable suggestion was skipped due to low confidence.


} catch (e) {
throw new ConfigurationError("Make sure the custom expression contains a valid array object");
}
}
Comment on lines +25 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Include original error message in ConfigurationError for better debugging

When catching exceptions in the parseArray function, the original error message is not included in the ConfigurationError, which might make debugging harder. Consider appending the original error message to provide more context.

Apply this diff to improve error messages:

   } catch (e) {
-    throw new ConfigurationError("Make sure the custom expression contains a valid array object");
+    throw new ConfigurationError(`Make sure the custom expression contains a valid array object. Error: ${e.message}`);
   }
 }
📝 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
function parseArray(value) {
try {
if (!value) {
return [];
}
if (Array.isArray(value)) {
return value;
}
const parsedValue = JSON.parse(value);
if (!Array.isArray(parsedValue)) {
throw new Error("Not an array");
}
return parsedValue;
} catch (e) {
throw new ConfigurationError("Make sure the custom expression contains a valid array object");
}
}
function parseArray(value) {
try {
if (!value) {
return [];
}
if (Array.isArray(value)) {
return value;
}
const parsedValue = JSON.parse(value);
if (!Array.isArray(parsedValue)) {
throw new Error("Not an array");
}
return parsedValue;
} catch (e) {
throw new ConfigurationError(`Make sure the custom expression contains a valid array object. Error: ${e.message}`);
}
}


export default {
parseArray: (value) => parseArray(value).map(valueToObject),
};
63 changes: 59 additions & 4 deletions components/mitra/mitra.app.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,66 @@
import { axios } from "@pipedream/platform";

export default {
type: "app",
app: "mitra",
propDefinitions: {},
propDefinitions: {
tableName: {
type: "string",
label: "Table Name",
description: "The name of the table in the database in case it was not set in the app.",
optional: true,
},
records: {
type: "string[]",
label: "Records",
description: "An array of records to insert or update, in JSON format.",
},
Comment on lines +14 to +17
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider changing records type to object[] for JSON records

The records property is currently defined with the type string[], but the description indicates it should be "An array of records to insert or update, in JSON format." To accurately represent JSON objects and to ensure proper handling of the data, consider changing the type to object[]. This change will allow for better validation and manipulation of the records as JSON objects.

Apply this diff to update the type:

  records: {
-   type: "string[]",
+   type: "object[]",
    label: "Records",
    description: "An array of records to insert or update, in JSON format.",
  },
📝 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
type: "string[]",
label: "Records",
description: "An array of records to insert or update, in JSON format.",
},
type: "object[]",
label: "Records",
description: "An array of records to insert or update, in JSON format.",
},

},
methods: {
// this.$auth contains connected account data
authKeys() {
console.log(Object.keys(this.$auth));
getUrl(tableName, path) {
const {
url,
table_name: defaultTableName,
} = this.$auth;
const baseUrl = `${url}/${tableName || defaultTableName}`;
return path
? `${baseUrl}${path}`
: baseUrl;
},
getHeaders(headers) {
return {
...headers,
"Content-Type": "application/json",
"Authorization": `Bearer ${this.$auth.api_key}`,
};
},
_makeRequest({
$ = this, path, headers, tableName, ...args
} = {}) {
return axios($, {
...args,
debug: true,
url: this.getUrl(tableName, path),
headers: this.getHeaders(headers),
});
},
post(args = {}) {
return this._makeRequest({
method: "POST",
...args,
});
},
put(args = {}) {
return this._makeRequest({
method: "PUT",
...args,
});
},
delete(args = {}) {
return this._makeRequest({
methot: "DELETE",
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix typo in 'methot' key to 'method'

There is a typo in the _makeRequest call within the delete method. The key 'methot' should be corrected to 'method' to ensure that the HTTP DELETE request is sent correctly.

Apply this diff to fix the typo:

return this._makeRequest({
-  methot: "DELETE",
+  method: "DELETE",
   ...args,
});
📝 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
methot: "DELETE",
method: "DELETE",

...args,
});
Comment on lines +59 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider renaming the delete method to avoid using a reserved keyword

While naming a method delete is allowed in JavaScript object literals, it can lead to confusion or potential issues because delete is a reserved keyword. To improve code readability and maintainability, consider renaming the method to something like deleteRecord or remove.

Apply this diff to rename the method:

- delete(args = {}) {
+ deleteRecord(args = {}) {
    return this._makeRequest({
      method: "DELETE",
      ...args,
    });
  },

Remember to update all references to this method accordingly.

📝 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
delete(args = {}) {
return this._makeRequest({
methot: "DELETE",
...args,
});
deleteRecord(args = {}) {
return this._makeRequest({
methot: "DELETE",
...args,
});

},
},
};
Loading