-
Notifications
You must be signed in to change notification settings - Fork 0
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
Script for deps #188
Script for deps #188
Conversation
…check for current_state
* Fyle Card <> Vendor Mapping setup * Added script to add mapping_settings * Fix post release script * Modified script, added additional test case * lint fix * modify post-release script
* Fyle Card <> Vendor Mapping setup * Added script to add mapping_settings * Fix post release script * Projects and Deps fields disable v1 * Modified script, added additional test case * lint fix * modify post-release script * bump accounting-mapping version * modify the variable_name, add conditional update * Add example objects * Added loggers * Added test cases * Modify test case, add filter in dep field settings
* Fyle Card <> Vendor Mapping setup * Added script to add mapping_settings * Fix post release script * Projects and Deps fields disable v1 * Modified script, added additional test case * lint fix * modify post-release script * bump accounting-mapping version * modify the variable_name, add conditional update * Add example objects * Added loggers * Added test cases * Dependent Field optimizations * fix failing test * Fix post-release script * Fix the merged issues
|
WalkthroughThis update introduces a SQL script file ( Changes
Poem
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- scripts/004_disable_deps_db_and_fyle.sql (1 hunks)
Files skipped from review due to trivial changes (1)
- scripts/004_disable_deps_db_and_fyle.sql
|
|
|
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: 3
Outside diff range and nitpick comments (1)
scripts/004_disable_deps_db_and_fyle.sql (1)
1-11
: Ensure all future modifications and plans are tracked in project management tools rather than in code comments for better visibility and management.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- scripts/004_disable_deps_db_and_fyle.sql (1 hunks)
Additional comments not posted (2)
scripts/004_disable_deps_db_and_fyle.sql (2)
35-37
: The file paths are hardcoded which could lead to errors in different environments. Consider making these paths configurable through environment variables or configuration files.
[REFACTOR_SUGGESTion]Also applies to: 95-97
60-74
: Verify that the deletion criteria are correctly capturing all intended records and not affecting unintended ones. Testing with actual data in a controlled environment is recommended.Also applies to: 111-115
rollback; | ||
begin; | ||
|
||
create temp table temp_cost_category ( | ||
cost_category_name TEXT | ||
); | ||
|
||
--- update the path here | ||
\copy temp_cost_category (cost_category_name) | ||
from '/Users/hrishabh/Desktop/cost_category_org_name.csv' WITH CSV HEADER; | ||
|
||
--- update the workspace id here | ||
delete from cost_category | ||
where name in ( | ||
select cost_category_name from temp_cost_category | ||
) | ||
and workspace_id = _workspace_id; | ||
|
||
---- Fyle DB ---- | ||
--- update the org id here | ||
rollback; | ||
begin; | ||
|
||
delete from platform_schema.dependent_expense_field_values | ||
where expense_field_value in ( | ||
select cost_category_name from temp_cost_category | ||
) | ||
and org_id = _org_id; |
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.
Ensure consistency in transaction management. Each operation block starts with a rollback
followed by begin
, which might not be necessary if the previous transactions are always meant to be rolled back. Clarify the transaction strategy with the team.
|
||
------ Cost Code ------ | ||
rollback; | ||
begin; |
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.
It seems there are redundant rollback
statements before each begin
. If this is intentional to ensure a clean state, it might be more efficient to handle this at the session or application level.
Also applies to: 58-58, 68-68, 89-89, 109-109
------ Cost Code ------ | ||
rollback; | ||
begin; | ||
|
||
create temp table temp_cost_code ( | ||
cost_code_name TEXT | ||
); | ||
|
||
--- update the path here | ||
\copy temp_cost_code (cost_code_name) | ||
from '/Users/hrishabh/Desktop/cost_code_org_name.csv' WITH CSV HEADER; | ||
|
||
--- update the workspace id here | ||
delete from destination_attributes | ||
where attribute_type = 'COST_CODE' | ||
and value in ( | ||
select cost_code_name from temp_cost_code | ||
) and workspace_id = _workspace_id; | ||
|
||
--- update the workspace id here | ||
delete from cost_category | ||
where cost_code_name in ( | ||
select cost_code_name from temp_cost_code | ||
) | ||
and workspace_id = _workspace_id; |
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.
Consider using parameterized queries or prepared statements to prevent SQL injection risks, especially for values derived from external sources like CSV files.
Summary by CodeRabbit