Skip to content

feat: bulk edit for table configurations and go security fix#378

Open
arsalan-datazip wants to merge 22 commits into
stagingfrom
feat/bulk-conf-edit
Open

feat: bulk edit for table configurations and go security fix#378
arsalan-datazip wants to merge 22 commits into
stagingfrom
feat/bulk-conf-edit

Conversation

@arsalan-datazip

@arsalan-datazip arsalan-datazip commented May 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Description

Earlier user can only configure one table at a time and enable its optimization.
This PR adds a bulk edit API wihich allows user to configure multiple tables and enable optimisation of all selected tables.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

@arsalan-datazip arsalan-datazip changed the title feat/bulk edit table conf feat: bulk edit table conf May 15, 2026
@arsalan-datazip arsalan-datazip changed the title feat: bulk edit table conf feat: bulk edit table configuration May 15, 2026
@arsalan-datazip arsalan-datazip changed the title feat: bulk edit table configuration feat: bulk edit table configurations May 15, 2026
@arsalan-datazip arsalan-datazip changed the title feat: bulk edit table configurations feat: bulk edit for table configurations May 15, 2026
badalprasadsingh
badalprasadsingh requested changes May 22, 2026
View reviewed changes

@badalprasadsingh badalprasadsingh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed.

Just to confirm: this feature is for multiple tables across a single database, and not across multiple databases as well, right?

Comment thread server/routes/router.go Outdated
Comment thread server/internal/constants/constants.go Outdated
Comment thread server/internal/services/optimization/terminal.go Outdated
Comment thread server/internal/services/optimization/terminal.go Outdated
Comment thread server/internal/services/optimization/terminal.go Outdated
Comment thread server/internal/services/optimization/terminal.go Outdated
Comment thread server/internal/services/optimization/terminal.go Outdated
Comment thread server/internal/services/optimization/terminal.go Outdated
Comment thread server/internal/services/optimization/terminal.go Outdated
Comment thread server/internal/services/optimization/terminal.go Outdated
badalprasadsingh
badalprasadsingh requested changes May 27, 2026
View reviewed changes

@badalprasadsingh badalprasadsingh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Second Review. Will test after this.

Single Table configuration Service Function is changed as well, please do check backward compatibility for that API.

Comment thread server/internal/constants/constants.go Outdated
Comment thread server/internal/models/dto/requests.go Outdated
Comment thread server/go.mod Outdated
@arsalan-datazip

arsalan-datazip commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Second Review. Will test after this.

Single Table configuration Service Function is changed as well, please do check backward compatibility for that API.

Will do backward compatibilty check
I have changed single Table conf because both SetProperties and BulkSetProperties were using the same logic to create map for properties so just to abstract it out I kept that in a single function .

badalprasadsingh
badalprasadsingh requested changes May 27, 2026
View reviewed changes
Comment thread server/internal/services/optimization/terminal.go Outdated
@badalprasadsingh

badalprasadsingh commented May 27, 2026

Copy link
Copy Markdown
Collaborator

This PR also fixes "Go Security Fix". Please add that in the title as well.

badalprasadsingh
badalprasadsingh requested changes May 27, 2026
View reviewed changes

@badalprasadsingh badalprasadsingh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Few more comments.

Comment thread server/internal/models/dto/response.go Outdated
Comment thread server/internal/services/optimization/terminal.go Outdated
Comment thread server/internal/services/optimization/terminal.go Outdated
@badalprasadsingh

badalprasadsingh commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator
image

Please return an error message in case the user does not selects any table.

This behaviour is required according to the PRD.

@arsalan-datazip arsalan-datazip changed the title feat: bulk edit for table configurations feat: bulk edit for table configurations and go security fix May 28, 2026
hash-data
hash-data reviewed May 31, 2026
View reviewed changes
Comment thread server/internal/services/optimization/terminal.go Outdated
hash-data
hash-data previously approved these changes Jun 9, 2026
View reviewed changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@badalprasadsingh badalprasadsingh badalprasadsingh requested changes

@hash-data hash-data hash-data left review comments

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@arsalan-datazip @badalprasadsingh @hash-data @deepanshupal09-datazip