review comments

Author: iamayushm

internal/sql/repository/deploymentConfig/repository.go

@@ -124,8 +124,9 @@ func (impl *RepositoryImpl) UpdateAll(tx *pg.Tx, configs []*DeploymentConfig) ([
 func (impl *RepositoryImpl) GetByAppIdAndEnvId(tx *pg.Tx, appId, envId int) (*DeploymentConfig, error) {
 	result := &DeploymentConfig{}
 	var connection orm.DB
-	connection = tx
-	if tx == nil {
+	if tx != nil {
+		connection = tx
+	} else {
 		connection = impl.dbConnection
 	}
 	err := connection.Model(result).
@@ -146,8 +147,9 @@ func (impl *RepositoryImpl) GetByAppIdAndEnvId(tx *pg.Tx, appId, envId int) (*De
 func (impl *RepositoryImpl) GetAppLevelConfigForDevtronApps(tx *pg.Tx, appId int) (*DeploymentConfig, error) {
 	result := &DeploymentConfig{}
 	var connection orm.DB
-	connection = tx
-	if tx == nil {
+	if tx != nil {
+		connection = tx
+	} else {
 		connection = impl.dbConnection
 	}
 	err := connection.Model(result).

pkg/chart/ChartService.go

@@ -965,12 +965,17 @@ func (impl *ChartServiceImpl) ConfigureGitOpsRepoUrlForApp(appId int, repoUrl, c
 	if err != nil {
 		return nil, err
 	}
+	committed := false
 	tx, err := impl.chartRepository.StartTx()
 	if err != nil {
 		impl.logger.Errorw("error in starting transaction to update charts", "error", err)
 		return nil, err
 	}
-	defer impl.chartRepository.RollbackTx(tx)
+	defer func() {
+		if !committed {
+			impl.chartRepository.RollbackTx(tx)
+		}
+	}()
 	var updatedCharts []*chartRepoRepository.Chart
 	for _, ch := range charts {
 		if !ch.IsCustomGitRepository {
@@ -1003,6 +1008,7 @@ func (impl *ChartServiceImpl) ConfigureGitOpsRepoUrlForApp(appId int, repoUrl, c
 		impl.logger.Errorw("error in committing transaction to update charts", "error", err)
 		return nil, err
 	}
+	committed = true
 
 	return deploymentConfig, nil
 }

pkg/pipeline/CiCdPipelineOrchestrator.go

@@ -1370,7 +1370,7 @@ func (impl CiCdPipelineOrchestratorImpl) DeleteApp(appId int, userId int32) erro
 	// Rollback tx on error.
 	defer tx.Rollback()
 	// deleting deployment config first as it is dependent on app
-	appDeploymentConfig, err := impl.deploymentConfigService.GetAndMigrateConfigIfAbsentForDevtronApps(nil, appId, 0)
+	appDeploymentConfig, err := impl.deploymentConfigService.GetAndMigrateConfigIfAbsentForDevtronApps(tx, appId, 0)
 	if err != nil && !errors.Is(err, pg.ErrNoRows) {
 		impl.logger.Errorw("error in fetching environment deployment config by appId and envId", "appId", appId, "err", err)
 		return err