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

[Components] clarify - new components #14762

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

[Components] clarify - new components #14762

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
70 changes: 70 additions & 0 deletions components/clarify/actions/create-company/create-company.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import app from "../../clarify.app.mjs";
import constants from "../../common/constants.mjs";

export default {
key: "clarify-create-company",
name: "Create Company",
description: "Creates a new company record in the Clarify system. [See the documentation](https://api.getclarify.ai/swagger#/default/createRecord).",
version: "0.0.1",
type: "action",
props: {
app,
workspace: {
propDefinition: [
app,
"workspace",
],
},
name: {
propDefinition: [
app,
"companyName",
],
},
domain: {
propDefinition: [
app,
"companyDomain",
],
},
},
methods: {
createCompany({
workspace, ...args
} = {}) {
return this.app.post({
path: `/workspaces/${workspace}/objects/${constants.OBJECT_ENTITY.COMPANY}/records`,
...args,
});
},
},
async run({ $ }) {
const {
createCompany,
workspace,
name,
domain,
} = this;

const response = await createCompany({
$,
workspace,
data: {
data: {
type: "object",
attributes: {
name,
domains: {
items: [
domain,
],
},
},
},
},
});
Comment on lines +49 to +65
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 handling potential API errors

The API call could fail for various reasons (e.g., duplicate company name, invalid domain). Consider adding proper error handling.

-    const response = await createCompany({
-      $,
-      workspace,
-      data: {
-        data: {
-          type: "object",
-          attributes: {
-            name,
-            domains: {
-              items: [
-                domain,
-              ],
-            },
-          },
-        },
-      },
-    });
+    try {
+      const response = await createCompany({
+        $,
+        workspace,
+        data: {
+          data: {
+            type: "object",
+            attributes: {
+              name,
+              domains: {
+                items: [
+                  domain,
+                ],
+              },
+            },
+          },
+        },
+      });
+      return response;
+    } catch (error) {
+      throw new Error(`Failed to create company: ${error.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
const response = await createCompany({
$,
workspace,
data: {
data: {
type: "object",
attributes: {
name,
domains: {
items: [
domain,
],
},
},
},
},
});
try {
const response = await createCompany({
$,
workspace,
data: {
data: {
type: "object",
attributes: {
name,
domains: {
items: [
domain,
],
},
},
},
},
});
return response;
} catch (error) {
throw new Error(`Failed to create company: ${error.message}`);
}


$.export("$summary", "Successfully created company.");
return response;
},
};
79 changes: 79 additions & 0 deletions components/clarify/actions/find-user/find-user.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import app from "../../clarify.app.mjs";

export default {
key: "clarify-find-user",
name: "Find User",
description: "Searches within the Clarify system for a user based on the given 'email' prop. Returns the found user. [See the documentation](https://api.getclarify.ai/swagger#/default/getUsers).",
version: "0.0.1",
type: "action",
Comment on lines +1 to +8
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

The API documentation link in the description is broken

The documentation link https://api.getclarify.ai/swagger returns a 404 Not Found error. This needs to be updated with the correct API documentation URL.

🔗 Analysis chain

LGTM! Verify the API documentation link.

The module metadata is well-structured with clear naming and description.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the API documentation link is accessible
curl -I https://api.getclarify.ai/swagger

Length of output: 1255

props: {
app,
workspace: {
propDefinition: [
app,
"workspace",
],
},
email: {
type: "string",
label: "Email",
description: "The email of the user to search for.",
},
firstName: {
type: "string",
label: "First Name",
description: "The first name of the user to search for.",
optional: true,
},
lastName: {
type: "string",
label: "Last Name",
description: "The last name of the user to search for.",
optional: true,
},
},
methods: {
getUsers({
workspace, ...args
} = {}) {
return this.app._makeRequest({
path: `/workspaces/${workspace}/users`,
...args,
});
},
},
Comment on lines +35 to +44
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 API requests.

The getUsers method should include error handling for API failures and workspace validation.

 methods: {
   getUsers({
     workspace, ...args
   } = {}) {
+    if (!workspace) {
+      throw new Error("Workspace ID is required");
+    }
+    try {
       return this.app._makeRequest({
         path: `/workspaces/${workspace}/users`,
         ...args,
       });
+    } catch (error) {
+      throw new Error(`Failed to fetch users: ${error.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
methods: {
getUsers({
workspace, ...args
} = {}) {
return this.app._makeRequest({
path: `/workspaces/${workspace}/users`,
...args,
});
},
},
methods: {
getUsers({
workspace, ...args
} = {}) {
if (!workspace) {
throw new Error("Workspace ID is required");
}
try {
return this.app._makeRequest({
path: `/workspaces/${workspace}/users`,
...args,
});
} catch (error) {
throw new Error(`Failed to fetch users: ${error.message}`);
}
},
},

async run({ $ }) {
const {
getUsers,
workspace,
email,
firstName,
lastName,
} = this;

const response = await getUsers({
$,
workspace,
params: {
limit: 1000,
},
});

const userFound = response.data.find(({ attributes }) =>
attributes.email === email
|| attributes.firstName?.toLowerCase()?.includes(firstName?.toLowerCase())
|| attributes.lastName?.toLowerCase()?.includes(lastName?.toLowerCase())
|| attributes?.name?.first_name?.toLowerCase()?.includes(firstName?.toLowerCase())
|| attributes?.name?.last_name?.toLowerCase()?.includes(lastName?.toLowerCase()));

if (!userFound) {
$.export("$summary", "No user found with the given attributes.");
return {
success: false,
};
}

$.export("$summary", "Successfully found user.");
return userFound;
},
Comment on lines +45 to +78
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

Critical issues in user search implementation.

Several issues need to be addressed:

  1. The hard-coded limit of 1000 users might miss results in large workspaces
  2. Multiple matching users are silently ignored (only first match is returned)
  3. No pagination handling for large result sets
  4. Complex optional chaining could be simplified
 async run({ $ }) {
   const {
     getUsers,
     workspace,
     email,
     firstName,
     lastName,
   } = this;

+  const allUsers = [];
+  let page = 1;
+  const PAGE_SIZE = 100;
+
+  // Fetch all users with pagination
+  while (true) {
     const response = await getUsers({
       $,
       workspace,
       params: {
-        limit: 1000,
+        limit: PAGE_SIZE,
+        page,
       },
     });
+    
+    if (!response.data?.length) break;
+    allUsers.push(...response.data);
+    page++;
+  }

-  const userFound = response.data.find(({ attributes }) =>
+  const matchingUsers = allUsers.filter(({ attributes }) =>
     attributes.email === email
-    || attributes.firstName?.toLowerCase()?.includes(firstName?.toLowerCase())
-    || attributes.lastName?.toLowerCase()?.includes(lastName?.toLowerCase())
-    || attributes?.name?.first_name?.toLowerCase()?.includes(firstName?.toLowerCase())
-    || attributes?.name?.last_name?.toLowerCase()?.includes(lastName?.toLowerCase()));
+    || (firstName && this.matchName(attributes.firstName, firstName))
+    || (firstName && this.matchName(attributes?.name?.first_name, firstName))
+    || (lastName && this.matchName(attributes.lastName, lastName))
+    || (lastName && this.matchName(attributes?.name?.last_name, lastName))
+  );

-  if (!userFound) {
+  if (!matchingUsers.length) {
     $.export("$summary", "No user found with the given attributes.");
     return {
       success: false,
     };
   }

-  $.export("$summary", "Successfully found user.");
+  if (matchingUsers.length > 1) {
+    $.export("$summary", `Found ${matchingUsers.length} matching users. Returning all matches.`);
+  } else {
+    $.export("$summary", "Successfully found user.");
+  }
-  return userFound;
+  return matchingUsers;
 },
+
+// Add helper method for name matching
+matchName(value, search) {
+  if (!value || !search) return false;
+  return this.caseSensitive
+    ? value.includes(search)
+    : value.toLowerCase().includes(search.toLowerCase());
+},

Committable suggestion skipped: line range outside the PR's diff.

};
72 changes: 72 additions & 0 deletions components/clarify/actions/update-person/update-person.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import app from "../../clarify.app.mjs";
import constants from "../../common/constants.mjs";

export default {
key: "clarify-update-person",
name: "Update Person",
description: "Updates an existing person record in the Clarify system. [See the documentation](https://api.getclarify.ai/swagger#/default/updateRecord).",
version: "0.0.1",
type: "action",
props: {
app,
workspace: {
propDefinition: [
app,
"workspace",
],
},
personId: {
propDefinition: [
app,
"personId",
({ workspace }) => ({
workspace,
}),
],
},
companyId: {
propDefinition: [
app,
"companyId",
({ workspace }) => ({
workspace,
}),
],
},
},
Comment on lines +10 to +36
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 adding optional update fields as props

The current implementation only allows updating the companyId. According to the PR objectives, this action should support "optional properties for the fields to be updated". Consider adding more updateable fields as optional props.

Example implementation:

 props: {
   app,
   workspace: {
     propDefinition: [
       app,
       "workspace",
     ],
   },
   personId: {
     propDefinition: [
       app,
       "personId",
       ({ workspace }) => ({
         workspace,
       }),
     ],
   },
   companyId: {
     propDefinition: [
       app,
       "companyId",
       ({ workspace }) => ({
         workspace,
       }),
     ],
+    optional: true,
   },
+  email: {
+    type: "string",
+    label: "Email",
+    description: "The email address of the person",
+    optional: true,
+  },
+  name: {
+    type: "string",
+    label: "Name",
+    description: "The name of the person",
+    optional: 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
props: {
app,
workspace: {
propDefinition: [
app,
"workspace",
],
},
personId: {
propDefinition: [
app,
"personId",
({ workspace }) => ({
workspace,
}),
],
},
companyId: {
propDefinition: [
app,
"companyId",
({ workspace }) => ({
workspace,
}),
],
},
},
props: {
app,
workspace: {
propDefinition: [
app,
"workspace",
],
},
personId: {
propDefinition: [
app,
"personId",
({ workspace }) => ({
workspace,
}),
],
},
companyId: {
propDefinition: [
app,
"companyId",
({ workspace }) => ({
workspace,
}),
],
optional: true,
},
email: {
type: "string",
label: "Email",
description: "The email address of the person",
optional: true,
},
name: {
type: "string",
label: "Name",
description: "The name of the person",
optional: true,
},
},

methods: {
updatePerson({
workspace, personId, ...args
}) {
return this.app.patch({
path: `/workspaces/${workspace}/objects/${constants.OBJECT_ENTITY.PERSON}/records/${personId}`,
...args,
});
},
},
async run({ $ }) {
const {
updatePerson,
workspace,
personId,
companyId,
} = this;

const response = await updatePerson({
$,
workspace,
personId,
data: {
data: {
type: "object",
attributes: {
companyId,
},
},
},
});

$.export("$summary", "Successfully updated person.");
return response;
},
};
Loading
Loading