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

fix: raise notice to delete dependent fields after workspace reset #230

Merged
merged 3 commits into from
Dec 13, 2024
Merged

fix: raise notice to delete dependent fields after workspace reset #230

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8a961b5
fix: raise notice to delete dependent fields after workspace reset
JustARatherRidiculouslyLongUsername Dec 10, 2024
ba48fed
refactor: squish query into one-liner
JustARatherRidiculouslyLongUsername Dec 10, 2024
def3e25
fix: dependent field queries not printing the org id (#231)
JustARatherRidiculouslyLongUsername Dec 13, 2024
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
14 changes: 14 additions & 0 deletions scripts/sql/reset_db.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,20 @@ BEGIN
WHERE w.id = _workspace_id;
GET DIAGNOSTICS rcount = ROW_COUNT;
RAISE NOTICE 'Deleted % workspaces', rcount;

_org_id := (SELECT fyle_org_id FROM workspaces WHERE id = _workspace_id);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Variable assignment will always return NULL

The _org_id assignment happens after the workspaces table has been deleted (see line 202), making this SELECT statement always return NULL. Move this assignment to the beginning of the function, right after the variable declarations.

DECLARE
  rcount integer;
  _org_id varchar(255);
BEGIN
+ _org_id := (SELECT fyle_org_id FROM workspaces WHERE id = _workspace_id);
  RAISE NOTICE 'Deleting data from workspace % ', _workspace_id;

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


RAISE NOTICE E'\n\n\n\n\n\n\n\n\nSwitch to prod db and run the below queries to delete dependent fields


rollback;
begin;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Remove incorrect transaction control statements

The ROLLBACK statement will undo all the previous deletion operations, and the new BEGIN serves no purpose as it only wraps a NOTICE statement. These statements should be removed.

- rollback;
- begin;


delete from platform_schema.dependent_expense_field_mappings where expense_field_id in (select id from platform_schema.expense_fields where org_id =''%'' and type=''DEPENDENT_SELECT'');

delete from platform_schema.expense_fields where org_id = ''%'' and type = ''DEPENDENT_SELECT'';\n\n\n\n\n\n\n\n\n\n\n', _org_id, _org_id;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security: Fix SQL injection vulnerability and improve architecture

The current implementation has several issues:

  1. Using string formatting for SQL queries is vulnerable to SQL injection
  2. Manual execution in production is error-prone
  3. No validation of _org_id before use

Consider creating a separate function in the production database to handle this cleanup:

CREATE OR REPLACE FUNCTION cleanup_dependent_fields(IN _org_id varchar) RETURNS void AS $$
BEGIN
  IF _org_id IS NULL THEN
    RAISE EXCEPTION 'Organization ID cannot be null';
  END IF;

  DELETE FROM platform_schema.dependent_expense_field_mappings 
  WHERE expense_field_id IN (
    SELECT id 
    FROM platform_schema.expense_fields 
    WHERE org_id = _org_id 
    AND type = 'DEPENDENT_SELECT'
  );

  DELETE FROM platform_schema.expense_fields 
  WHERE org_id = _org_id 
  AND type = 'DEPENDENT_SELECT';
END;
$$ LANGUAGE plpgsql;

Then modify the notice to reference this function:

- delete from platform_schema.dependent_expense_field_mappings where expense_field_id in (select id from platform_schema.expense_fields where org_id =''%'' and type=''DEPENDENT_SELECT'');
- delete from platform_schema.expense_fields where org_id = ''%'' and type = ''DEPENDENT_SELECT'';\n\n\n\n\n\n\n\n\n\n\n'
+ SELECT cleanup_dependent_fields(''%'');\n\n'

JustARatherRidiculouslyLongUsername marked this conversation as resolved.
Show resolved Hide resolved


RETURN;
END
$$ LANGUAGE plpgsql;
Loading