Skip to content

Commit 74e1e75

Browse files
ShashwatDadhichShashwatDadhich
committed
code review comments incorporated
1 parent 2f6be75 commit 74e1e75

File tree

5 files changed

+25
-25
lines changed

5 files changed

+25
-25
lines changed

‎internal/sql/repository/pipelineConfig/MaterialRepository.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,16 @@ type GitMaterial struct {
5252

5353
type MaterialRepository interface {
5454
MaterialExists(url string) (bool, error)
55-
SaveMaterial(material *GitMaterial, tx *pg.Tx) error
56-
UpdateMaterial(material *GitMaterial, tx *pg.Tx) error
55+
SaveMaterial(tx *pg.Tx, material *GitMaterial) error
56+
UpdateMaterial(tx *pg.Tx, material *GitMaterial) error
5757
Update(materials []*GitMaterial) error
5858
FindByAppId(appId int) ([]*GitMaterial, error)
5959
FindById(Id int) (*GitMaterial, error)
6060
FindByAppIdAndGitMaterialId(appId, id int) (*GitMaterial, error)
6161
UpdateMaterialScmId(material *GitMaterial) error
6262
FindByAppIdAndCheckoutPath(appId int, checkoutPath string) (*GitMaterial, error)
6363
FindByGitProviderId(gitProviderId int) (materials []*GitMaterial, err error)
64-
MarkMaterialDeleted(material *GitMaterial, tx *pg.Tx) error
64+
MarkMaterialDeleted(tx *pg.Tx, material *GitMaterial) error
6565
FindNumberOfAppsWithGitRepo(appIds []int) (int, error)
6666
FindByAppIds(appIds []int) ([]*GitMaterial, error)
6767
}
@@ -113,11 +113,11 @@ func (repo MaterialRepositoryImpl) MaterialExists(url string) (bool, error) {
113113
return exists, err
114114
}
115115

116-
func (repo MaterialRepositoryImpl) SaveMaterial(material *GitMaterial, tx *pg.Tx) error {
116+
func (repo MaterialRepositoryImpl) SaveMaterial(tx *pg.Tx, material *GitMaterial) error {
117117
return tx.Insert(material)
118118
}
119119

120-
func (repo MaterialRepositoryImpl) UpdateMaterial(material *GitMaterial, tx *pg.Tx) error {
120+
func (repo MaterialRepositoryImpl) UpdateMaterial(tx *pg.Tx, material *GitMaterial) error {
121121
return tx.Update(material)
122122
}
123123

@@ -164,7 +164,7 @@ func (repo MaterialRepositoryImpl) FindByGitProviderId(gitProviderId int) (mater
164164
return materials, err
165165
}
166166

167-
func (repo MaterialRepositoryImpl) MarkMaterialDeleted(material *GitMaterial, tx *pg.Tx) error {
167+
func (repo MaterialRepositoryImpl) MarkMaterialDeleted(tx *pg.Tx, material *GitMaterial) error {
168168
material.Active = false
169169
return tx.Update(material)
170170
}

‎pkg/pipeline/CiCdPipelineOrchestrator.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,7 +1302,7 @@ func (impl CiCdPipelineOrchestratorImpl) CreateMaterials(createMaterialRequest *
13021302
var materials []*bean.GitMaterial
13031303
for _, inputMaterial := range createMaterialRequest.Material {
13041304
inputMaterial.UpdateSanitisedGitRepoUrl()
1305-
m, err := impl.createMaterial(inputMaterial, createMaterialRequest.AppId, createMaterialRequest.UserId, tx)
1305+
m, err := impl.createMaterial(tx, inputMaterial, createMaterialRequest.AppId, createMaterialRequest.UserId)
13061306
inputMaterial.Id = m.Id
13071307
if err != nil {
13081308
return nil, err
@@ -1316,7 +1316,7 @@ func (impl CiCdPipelineOrchestratorImpl) CreateMaterials(createMaterialRequest *
13161316
}
13171317
err = impl.transactionManager.CommitTx(tx)
13181318
if err != nil {
1319-
impl.logger.Errorw("error in committing tx Create material", "err", err)
1319+
impl.logger.Errorw("error in committing tx Create material", "err", err, "materials", materials)
13201320
return nil, err
13211321
}
13221322
impl.logger.Debugw("all materials are ", "materials", materials)
@@ -1329,7 +1329,7 @@ func (impl CiCdPipelineOrchestratorImpl) UpdateMaterial(updateMaterialDTO *bean.
13291329
return nil, err
13301330
}
13311331
defer tx.Rollback()
1332-
updatedMaterial, err := impl.updateMaterial(updateMaterialDTO, tx)
1332+
updatedMaterial, err := impl.updateMaterial(tx, updateMaterialDTO)
13331333
if err != nil {
13341334
impl.logger.Errorw("err", "err", err)
13351335
return nil, err
@@ -1478,7 +1478,7 @@ func (impl CiCdPipelineOrchestratorImpl) validateCheckoutPathsForMultiGit(allPat
14781478
return nil
14791479
}
14801480

1481-
func (impl CiCdPipelineOrchestratorImpl) updateMaterial(updateMaterialDTO *bean.UpdateMaterialDTO, tx *pg.Tx) (*pipelineConfig.GitMaterial, error) {
1481+
func (impl CiCdPipelineOrchestratorImpl) updateMaterial(tx *pg.Tx, updateMaterialDTO *bean.UpdateMaterialDTO) (*pipelineConfig.GitMaterial, error) {
14821482
existingMaterials, err := impl.materialRepository.FindByAppId(updateMaterialDTO.AppId)
14831483
if err != nil {
14841484
impl.logger.Errorw("err", "err", err)
@@ -1519,19 +1519,19 @@ func (impl CiCdPipelineOrchestratorImpl) updateMaterial(updateMaterialDTO *bean.
15191519
currentMaterial.FilterPattern = updateMaterialDTO.Material.FilterPattern
15201520
currentMaterial.AuditLog = sql.AuditLog{UpdatedBy: updateMaterialDTO.UserId, CreatedBy: currentMaterial.CreatedBy, UpdatedOn: time.Now(), CreatedOn: currentMaterial.CreatedOn}
15211521

1522-
err = impl.materialRepository.UpdateMaterial(currentMaterial, tx)
1522+
err = impl.materialRepository.UpdateMaterial(tx, currentMaterial)
15231523

15241524
if err != nil {
15251525
impl.logger.Errorw("error in updating material", "material", currentMaterial, "err", err)
15261526
return nil, err
15271527
}
15281528

1529-
err = impl.gitMaterialHistoryService.CreateMaterialHistory(currentMaterial, tx)
1529+
err = impl.gitMaterialHistoryService.CreateMaterialHistory(tx, currentMaterial)
15301530

15311531
return currentMaterial, nil
15321532
}
15331533

1534-
func (impl CiCdPipelineOrchestratorImpl) createMaterial(inputMaterial *bean.GitMaterial, appId int, userId int32, tx *pg.Tx) (*pipelineConfig.GitMaterial, error) {
1534+
func (impl CiCdPipelineOrchestratorImpl) createMaterial(tx *pg.Tx, inputMaterial *bean.GitMaterial, appId int, userId int32) (*pipelineConfig.GitMaterial, error) {
15351535
basePath := path.Base(inputMaterial.Url)
15361536
basePath = strings.TrimSuffix(basePath, ".git")
15371537
material := &pipelineConfig.GitMaterial{
@@ -1545,12 +1545,12 @@ func (impl CiCdPipelineOrchestratorImpl) createMaterial(inputMaterial *bean.GitM
15451545
FilterPattern: inputMaterial.FilterPattern,
15461546
AuditLog: sql.AuditLog{UpdatedBy: userId, CreatedBy: userId, UpdatedOn: time.Now(), CreatedOn: time.Now()},
15471547
}
1548-
err := impl.materialRepository.SaveMaterial(material, tx)
1548+
err := impl.materialRepository.SaveMaterial(tx, material)
15491549
if err != nil {
15501550
impl.logger.Errorw("error in saving material", "material", material, "err", err)
15511551
return nil, err
15521552
}
1553-
err = impl.gitMaterialHistoryService.CreateMaterialHistory(material, tx)
1553+
err = impl.gitMaterialHistoryService.CreateMaterialHistory(tx, material)
15541554
return material, err
15551555
}
15561556

‎pkg/pipeline/CiMaterialConfigService.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,13 +132,13 @@ func (impl *CiMaterialConfigServiceImpl) DeleteMaterial(request *bean.UpdateMate
132132

133133
defer tx.Rollback()
134134

135-
err = impl.materialRepo.MarkMaterialDeleted(existingMaterial, tx)
135+
err = impl.materialRepo.MarkMaterialDeleted(tx, existingMaterial)
136136
if err != nil {
137137
impl.logger.Errorw("error in deleting git material", "gitMaterial", existingMaterial)
138138
return err
139139
}
140140

141-
err = impl.gitMaterialHistoryService.MarkMaterialDeletedAndCreateHistory(existingMaterial, tx)
141+
err = impl.gitMaterialHistoryService.MarkMaterialDeletedAndCreateHistory(tx, existingMaterial)
142142

143143
var materials []*pipelineConfig.CiPipelineMaterial
144144
for _, pipeline := range pipelines {

‎pkg/pipeline/history/GitMaterialHistoryService.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ import (
99
)
1010

1111
type GitMaterialHistoryService interface {
12-
CreateMaterialHistory(inputMaterial *pipelineConfig.GitMaterial, tx *pg.Tx) error
12+
CreateMaterialHistory(tx *pg.Tx, inputMaterial *pipelineConfig.GitMaterial) error
1313
CreateDeleteMaterialHistory(materials []*pipelineConfig.GitMaterial) error
14-
MarkMaterialDeletedAndCreateHistory(material *pipelineConfig.GitMaterial, tx *pg.Tx) error
14+
MarkMaterialDeletedAndCreateHistory(tx *pg.Tx, material *pipelineConfig.GitMaterial) error
1515
}
1616

1717
type GitMaterialHistoryServiceImpl struct {
@@ -28,7 +28,7 @@ func NewGitMaterialHistoryServiceImpl(gitMaterialHistoryRepository repository.Gi
2828
}
2929
}
3030

31-
func (impl GitMaterialHistoryServiceImpl) CreateMaterialHistory(inputMaterial *pipelineConfig.GitMaterial, tx *pg.Tx) error {
31+
func (impl GitMaterialHistoryServiceImpl) CreateMaterialHistory(tx *pg.Tx, inputMaterial *pipelineConfig.GitMaterial) error {
3232

3333
material := &repository.GitMaterialHistory{
3434
GitMaterialId: inputMaterial.Id,
@@ -42,7 +42,7 @@ func (impl GitMaterialHistoryServiceImpl) CreateMaterialHistory(inputMaterial *p
4242
FilterPattern: inputMaterial.FilterPattern,
4343
AuditLog: sql.AuditLog{UpdatedBy: inputMaterial.UpdatedBy, CreatedBy: inputMaterial.CreatedBy, UpdatedOn: inputMaterial.UpdatedOn, CreatedOn: inputMaterial.CreatedOn},
4444
}
45-
err := impl.gitMaterialHistoryRepository.SaveGitMaterialHistory(material, tx)
45+
err := impl.gitMaterialHistoryRepository.SaveGitMaterialHistory(tx, material)
4646
if err != nil {
4747
impl.logger.Errorw("error in saving create/update history for git repository")
4848
}
@@ -87,11 +87,11 @@ func (impl GitMaterialHistoryServiceImpl) CreateDeleteMaterialHistory(materials
8787

8888
}
8989

90-
func (impl GitMaterialHistoryServiceImpl) MarkMaterialDeletedAndCreateHistory(material *pipelineConfig.GitMaterial, tx *pg.Tx) error {
90+
func (impl GitMaterialHistoryServiceImpl) MarkMaterialDeletedAndCreateHistory(tx *pg.Tx, material *pipelineConfig.GitMaterial) error {
9191

9292
material.Active = false
9393

94-
err := impl.CreateMaterialHistory(material, tx)
94+
err := impl.CreateMaterialHistory(tx, material)
9595

9696
if err != nil {
9797
impl.logger.Errorw("error in saving delete history for git material repository")

‎pkg/pipeline/history/repository/GitMaterialHistoryRepository.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ type GitMaterialHistory struct {
2121
}
2222

2323
type GitMaterialHistoryRepository interface {
24-
SaveGitMaterialHistory(material *GitMaterialHistory, tx *pg.Tx) error
24+
SaveGitMaterialHistory(tx *pg.Tx, material *GitMaterialHistory) error
2525
SaveDeleteMaterialHistory(materials []*GitMaterialHistory) error
2626
}
2727

@@ -35,7 +35,7 @@ func NewGitMaterialHistoryRepositoyImpl(dbConnection *pg.DB) *GitMaterialHistory
3535
}
3636
}
3737

38-
func (repo GitMaterialHistoryRepositoryImpl) SaveGitMaterialHistory(material *GitMaterialHistory, tx *pg.Tx) error {
38+
func (repo GitMaterialHistoryRepositoryImpl) SaveGitMaterialHistory(tx *pg.Tx, material *GitMaterialHistory) error {
3939
return tx.Insert(material)
4040
}
4141

0 commit comments

Comments
 (0)