-
Notifications
You must be signed in to change notification settings - Fork 186
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.
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
🐛 Fix project connectors inside code #486
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughThe recent changes involve minor formatting adjustments in the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant ProjectsService
participant ConnectorsDatabase
User->>ProjectsService: Request to update project
ProjectsService->>ConnectorsDatabase: Fetch active connectors
ConnectorsDatabase-->>ProjectsService: Return active connectors
ProjectsService->>ConnectorsDatabase: Update project-connectors table
ProjectsService-->>User: Return update confirmation
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
- Enhanced
createProject
method to updateproject-connectors
table with active connectors - Imported utilities from
@panora/shared
for dynamic property construction - Formatting adjustments in
connections-strategies.service.ts
(indentation, single quotes)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/api/src/@core/connections-strategies/connections-strategies.service.ts (4 hunks)
- packages/api/src/@core/projects/projects.service.ts (2 hunks)
Additional context used
Biome
packages/api/src/@core/connections-strategies/connections-strategies.service.ts
[error] 226-228: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause. (lint/correctness/noSwitchDeclarations)The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
GitHub Check: Build and Test (18.x)
packages/api/src/@core/connections-strategies/connections-strategies.service.ts
[warning] 88-88:
'value_' is assigned a value but never used
[warning] 348-348:
'updateCS' is assigned a value but never used
[warning] 375-375:
'value_' is assigned a value but never used
Additional comments not posted (4)
packages/api/src/@core/projects/projects.service.ts (2)
7-12
: Ensure that the newly added imports are utilized effectively in the file.
53-72
: The logic for handling active connectors has been added. This includes updating the project-connectors table based on the provided connectors array. Ensure that theupdateData
object is correctly populated and that all necessary fields are included in the database operation.Verification successful
The
updateData
object inprojects.service.ts
is correctly initialized and populated with necessary fields before being used in the database operation. The initialization includesid_project_connector
andid_project
, and additional properties are added based onconnector.vertical
andconnector.name
.
packages/api/src/@core/projects/projects.service.ts:53
:const updateData: any = {
packages/api/src/@core/projects/projects.service.ts:66
:updateData[propertyName + connector.name] = true;
packages/api/src/@core/projects/projects.service.ts:72
:data: updateData,
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `updateData` object is correctly populated and that all necessary fields are included in the database operation. # Test: Search for the `updateData` object usage. Expect: Correct population and inclusion of necessary fields. rg --type typescript $'updateData'Length of output: 68
Script:
#!/bin/bash # Description: Verify that the `updateData` object is correctly populated and that all necessary fields are included in the database operation. # Test: Search for the `updateData` object usage. Expect: Correct population and inclusion of necessary fields. rg --type ts 'updateData'Length of output: 1116
packages/api/src/@core/connections-strategies/connections-strategies.service.ts (2)
Line range hint
341-387
: The logging statements have been updated to use double quotes, which is consistent with TypeScript best practices. However, ensure that these logs provide enough context for debugging and consider adding more descriptive messages or additional logging if necessary.
233-235
: The usage ofCONNECTORS_METADATA
to dynamically fetch scopes based on provider and vertical is a good practice. However, ensure that thescopes
property is always defined inCONNECTORS_METADATA
to avoid potential runtime errors.Verification successful
The search results indicate that the
scopes
property is defined in multiple places, particularly within thepackages/shared/src/connectors/metadata.ts
file. This suggests that thescopes
property is indeed present in theCONNECTORS_METADATA
.
packages/shared/src/connectors/metadata.ts
: Multiple instances ofscopes
are defined here.Given this information, it appears that the
scopes
property is consistently defined in the relevant metadata, which should prevent potential runtime errors related to undefinedscopes
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `scopes` property is always defined in `CONNECTORS_METADATA`. # Test: Search for the `scopes` property definition in `CONNECTORS_METADATA`. Expect: Definition present. rg --type typescript $'scopes'Length of output: 64
Script:
#!/bin/bash # Description: Verify that the `scopes` property is always defined in `CONNECTORS_METADATA`. # Test: Search for the `scopes` property definition in `CONNECTORS_METADATA`. Expect: Definition present. rg --type ts 'scopes'Length of output: 15826
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Updated terminology from 'vertical' to 'category'
- Corrected file paths for metadata references
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
docs/open-source/contributors.mdx (1)
Line range hint
92-92
: Correct abbreviation usage.- (oauth, api key, basic etc) which is built by the Panora team + (oauth, api key, basic, etc.) which is built by the Panora teamIn American English, abbreviations like "etc." require a period. This change aligns with standard writing conventions.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/open-source/contributors.mdx (5 hunks)
Additional context used
LanguageTool
docs/open-source/contributors.mdx
[uncategorized] ~66-~66: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...Ie: Slack, Hubspot, Jira, Shopify ..._ First choose wisely which category the 3rd pa...
[typographical] ~89-~89: Consider adding a comma after ‘Actually’ for more clarity. (RB_LY_COMMA)
Context: ...ng the integration has not been built. Actually an integration is built in 2 parts : -...
[style] ~92-~92: In American English, abbreviations like “etc.” require a period. (ETC_PERIOD)
Context: ...ntication part** (oauth, api key, basic etc) which is built by the Panora team - th...
Additional comments not posted (3)
docs/open-source/contributors.mdx (3)
86-86
: Add a comma for clarity.- Actually an integration is built in 2 parts : + Actually, an integration is built in 2 parts:Adding a comma after "Actually" improves the readability and flow of the sentence.
Likely invalid or redundant comment.
77-78
: Ensure the link is accessible.Please run this script to ensure that the link provided in the documentation is accessible and correct.
Verification successful
The link to
categories.ts
in the documentation is accessible.
- The file
packages/shared/src/categories.ts
exists at the specified path.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the accessibility of the link to categories.ts. # Test: Check if the file exists at the specified path. if [[ -f "packages/shared/src/categories.ts" ]]; then echo "The link to categories.ts is accessible." else echo "The link to categories.ts is not accessible. Please update or fix the link." fiLength of output: 135
234-234
: Verify the script functionality.Please run this script to ensure that it functions correctly and integrates the new service without issues.
**Each custom 3rd party environment variable must be of the form `PROVIDER_category_SOFTWAREMODE_ATTRIBUTE` where** | ||
|
||
- `PROVIDER` is any 3rd party name | ||
- `VERTICAL` is for example [CRM, TICKETING, MARKETINGAUTOMATION, ...] | ||
- `category` is for example [CRM, TICKETING, MARKETINGAUTOMATION, ...] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarify the environment variable format.
The explanation of the environment variable format is clear, but it might be helpful to provide a brief description or link to documentation on what each component (PROVIDER
, category
, SOFTWAREMODE
, ATTRIBUTE
) represents. This could improve understanding for new contributors.
@@ -64,7 +64,7 @@ | |||
|
|||
_Ie: Slack, Hubspot, Jira, Shopify ..._ | |||
|
|||
First choose wisely which vertical the 3rd party belongs to among these: | |||
First choose wisely which category the 3rd party belongs to among these: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comma for clarity.
- Ie: Slack, Hubspot, Jira, Shopify ...
+ Ie: Slack, Hubspot, Jira, Shopify, ...
This minor punctuation addition improves the readability of the list.
Committable suggestion was skipped due to low confidence.
No description provided.