-
Notifications
You must be signed in to change notification settings - Fork 21
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
Ashish egov patch 2 #1166
Ashish egov patch 2 #1166
Conversation
Update Listener.ts
…IGIT-Frontend into campaign-for-test
WalkthroughWalkthroughThe recent updates focus on simplifying the database and application architecture by removing redundant foreign key constraints, modifying Kafka message handling, and enhancing internal function logic. Notably, the migration script cleans up database relationships, while changes in the API and configuration streamline message processing and improve error logging. This evolution enhances maintainability, clarity, and flexibility within the system, paving the way for more efficient campaign mapping operations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API
participant Kafka Listener
participant Database
User->>API: Request to create campaign mapping
API->>Database: Validate and store mapping data
Database-->>API: Confirmation of data storage
API->>Kafka Listener: Send mapping data for processing
Kafka Listener->>Kafka: Produce message to Kafka topic
Kafka-->>Kafka Listener: Message acknowledgement
Kafka Listener->>API: Acknowledge message processing
API-->>User: Confirm campaign mapping created
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 Configuration 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.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (3)
utilities/project-factory/migration/main/V20240725155100__remove_constraint_process_details.sql (1)
1-1
: Add a more descriptive comment.The comment should include more details about why the constraints are being removed and any potential impacts.
- -- Migration script to remove the foreign key constraint + -- Migration script to remove redundant foreign key constraints from the eg_cm_campaign_process and eg_cm_resource_activity tables. + -- This is done to simplify the database schema and improve performance.utilities/project-factory/src/server/kafka/Listener.ts (2)
79-80
: Improve error logging message.The error logging message should be more descriptive.
- console.log('Error coming for message : ', modifiedMessages); + console.error('Error while sending message to Kafka: ', modifiedMessages);
88-88
: Improve error logging message.The error logging message should be more descriptive.
- throwError("COMMON", 400, "KAKFA_ERROR", "Some error occured in kafka"); + throwError("COMMON", 400, "KAFKA_ERROR", "An error occurred while sending message to Kafka");
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- utilities/project-factory/migration/main/V20240725155100__remove_constraint_process_details.sql (1 hunks)
- utilities/project-factory/src/server/api/genericApis.ts (2 hunks)
- utilities/project-factory/src/server/config/index.ts (1 hunks)
- utilities/project-factory/src/server/kafka/Listener.ts (5 hunks)
- utilities/project-factory/src/server/utils/campaignMappingUtils.ts (1 hunks)
Additional comments not posted (6)
utilities/project-factory/migration/main/V20240725155100__remove_constraint_process_details.sql (2)
2-2
: Ensure the constraint name is correct.Double-check that
fk_campaignId
is the correct name of the foreign key constraint in theeg_cm_campaign_process
table.Verification successful
The constraint name is correct.
The foreign key constraint
fk_campaignId
is correctly referenced in theeg_cm_campaign_process
table.
utilities/project-factory/migration/main/V20240625141100__process_details_ddl.sql
: Defines the constraintfk_campaignId
.utilities/project-factory/migration/main/V20240725155100__remove_constraint_process_details.sql
: Drops the constraintfk_campaignId
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the foreign key constraint name in the eg_cm_campaign_process table. # Test: Check the constraint name. Expect: The constraint name should be fk_campaignId. psql -c "\d+ eg_cm_campaign_process"Length of output: 110
Script:
#!/bin/bash # Search for the definition of the eg_cm_campaign_process table and its constraints in the codebase. rg --context 10 'CREATE TABLE eg_cm_campaign_process'Length of output: 55
Script:
#!/bin/bash # List all SQL files and search within them for mentions of the eg_cm_campaign_process table and its constraints. fd -e sql -x rg 'eg_cm_campaign_process' {}Length of output: 166
Script:
#!/bin/bash # Search for the definition of the fk_campaignId constraint in the codebase. rg 'fk_campaignId'Length of output: 371
3-3
: Ensure the constraint name is correct.Double-check that
eg_cm_resource_activity_resourceDetailsId_fkey
is the correct name of the foreign key constraint in theeg_cm_resource_activity
table.utilities/project-factory/src/server/kafka/Listener.ts (1)
Line range hint
29-29
:
Remove unused topic from topicNames array.Ensure that the removed topic is not referenced elsewhere in the code.
Verification successful
Remove unused topic from topicNames array.
The topic
KAFKA_PROCESS_CAMPAIGN_MAPPING_TOPIC
is not referenced elsewhere in the codebase, and its removal from thetopicNames
array does not impact the functionality.
topicNames
array definition and usages confirm that the removed topic is not indirectly referenced.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the removed topic is not referenced elsewhere in the code. # Test: Search for the removed topic. Expect: No occurrences of the removed topic. rg --type js 'KAFKA_PROCESS_CAMPAIGN_MAPPING_TOPIC'Length of output: 51
Script:
#!/bin/bash # Description: Verify that the removed topic is not referenced elsewhere in the code. # Test: Search for the removed topic across all file types and include case-insensitive and partial matches. rg -i 'KAFKA_PROCESS_CAMPAIGN_MAPPING_TOPIC'Length of output: 44
Script:
#!/bin/bash # Description: Verify that the `topicNames` array does not indirectly reference the removed topic. # Test: Search for the definition of the `topicNames` array. rg 'topicNames' -A 3 # Test: Search for usages of the `topicNames` array. rg 'topicNames'Length of output: 901
utilities/project-factory/src/server/config/index.ts (1)
58-58
: Remove unused Kafka topic.Ensure that the removed topic is not referenced elsewhere in the code.
Verification successful
The unused Kafka topic
KAFKA_UPDATE_RESOURCE_DETAILS_TOPIC
is not referenced elsewhere in the codebase. It is safe to remove it.
- No further occurrences of
KAFKA_UPDATE_RESOURCE_DETAILS_TOPIC
were found in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the removed topic is not referenced elsewhere in the code. # Test: Search for the removed topic. Expect: No occurrences of the removed topic. rg --type js 'KAFKA_PROCESS_CAMPAIGN_MAPPING_TOPIC'Length of output: 51
Script:
#!/bin/bash # Description: Verify that the removed topic is not referenced elsewhere in the code. # Test: Search for the removed topic. Expect: No occurrences of the removed topic. rg --type js 'KAFKA_UPDATE_RESOURCE_DETAILS_TOPIC'Length of output: 50
utilities/project-factory/src/server/utils/campaignMappingUtils.ts (1)
418-434
: LGTM! But verify the function usage in the codebase.The code changes are approved. The renaming of the parameter from
messageObject
tomappingObject
improves clarity.However, ensure that all function calls to
processMapping
match the new parameter name.Verification successful
Function usage verified and matches the new parameter name.
The call to
processMapping
correctly uses the updated parameter namemappingObject
.
utilities/project-factory/src/server/api/genericApis.ts
:await processMapping(mappingObject)
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `processMapping` match the new parameter name. # Test: Search for the function usage. Expect: Only occurrences of the new parameter name. rg --type typescript -A 5 $'processMapping'Length of output: 77
Script:
#!/bin/bash # Description: Verify all function calls to `processMapping` match the new parameter name. # Test: Search for the function usage. Expect: Only occurrences of the new parameter name. rg 'processMapping' -A 5Length of output: 2082
Script:
#!/bin/bash # Description: Verify the parameter name in the call to `processMapping` in `genericApis.ts`. # Test: Check the parameter name in the call to `processMapping`. rg 'await processMapping' -A 5 utilities/project-factory/src/server/api/genericApis.tsLength of output: 214
utilities/project-factory/src/server/api/genericApis.ts (1)
887-888
: LGTM! But verify the function usage in the codebase.The code changes are approved. The renaming of the variable from
produceMessage
tomappingObject
improves clarity.However, ensure that all function calls to
createRelatedEntity
match the new variable name.Verification successful
Verification successful!
All references to
createRelatedEntity
and the variablemappingObject
are consistent and correctly updated in the codebase.
- The variable
mappingObject
is used appropriately within theprocessMapping
function and other relevant parts of the code.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `createRelatedEntity` match the new variable name. # Test: Search for the function usage. Expect: Only occurrences of the new variable name. rg --type typescript -A 5 $'createRelatedEntity'Length of output: 82
Script:
#!/bin/bash # Description: Verify all function calls to `createRelatedEntity` match the new variable name. # Find all TypeScript files and search for the function usage. fd -e ts -x rg 'createRelatedEntity' {}Length of output: 297
Script:
#!/bin/bash # Description: Verify all function calls to `createRelatedEntity` match the new variable name. # Find all TypeScript files and search for the term `mappingObject` to ensure consistency. fd -e ts -x rg 'mappingObject' {}Length of output: 1656
* Update Listener.ts * added new branch * Update Listener.ts * fixed mapping kafka error * mapping kafka fixed * fix kafka * fix kafka * Removing foreign key constraint * Update publishProjectFactory.yml
No description provided.