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.

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: app-group page breaking for inactive users #4315

Merged
merged 4 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
8 changes: 7 additions & 1 deletion internal/sql/repository/AppListingRepository.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,13 @@ func (impl AppListingRepositoryImpl) FetchOverviewAppsByEnvironment(envId, limit

func (impl AppListingRepositoryImpl) FetchLastDeployedImage(appId, envId int) (*LastDeployed, error) {
var lastDeployed []*LastDeployed
query := `select ca.image as last_deployed_image, u.email_id as last_deployed_by from pipeline p
// we are adding a case in the query to concatenate the string "(inactive)" to the users' email id when user is inactive
query := `select ca.image as last_deployed_image,
case
when u.active = false then u.email_id || ' (inactive)'
ashishdevtron marked this conversation as resolved.
Show resolved Hide resolved
else u.email_id
end as last_deployed_by
from pipeline p
join cd_workflow cw on cw.pipeline_id = p.id
join cd_workflow_runner cwr on cwr.cd_workflow_id = cw.id
join ci_artifact ca on ca.id = cw.ci_artifact_id
Expand Down
18 changes: 14 additions & 4 deletions pkg/app/AppListingService.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ func (impl AppListingServiceImpl) FetchOverviewAppsByEnvironment(envId, limit, o
resp := &OverviewAppsByEnvironmentBean{}
env, err := impl.environmentRepository.FindById(envId)
if err != nil {
impl.Logger.Errorw("failed to fetch env", "err", err, "env id", envId)
return resp, err
}
resp.EnvironmentId = envId
Expand All @@ -242,18 +243,27 @@ func (impl AppListingServiceImpl) FetchOverviewAppsByEnvironment(envId, limit, o
resp.Type = NonProduction
}
resp.Description = env.Description
createdBy, err := impl.userRepository.GetById(env.CreatedBy)
if err != nil {
return resp, err
createdBy, err := impl.userRepository.GetByIdIncludeDeleted(env.CreatedBy)
if err != nil && err != pg.ErrNoRows {
impl.Logger.Errorw("error in fetching user for app meta info", "error", err, "env created by", env.CreatedBy)
return nil, err
}
if createdBy != nil && createdBy.Id > 0 {
if createdBy.Active {
resp.CreatedBy = fmt.Sprintf(createdBy.EmailId)
} else {
resp.CreatedBy = fmt.Sprintf("%s (inactive)", createdBy.EmailId)
}
}
resp.CreatedBy = createdBy.EmailId
envContainers, err := impl.appListingRepository.FetchOverviewAppsByEnvironment(envId, limit, offset)
if err != nil {
impl.Logger.Errorw("failed to fetch environment containers", "err", err, "env id", envId)
return resp, err
}
for _, envContainer := range envContainers {
lastDeployed, err := impl.appListingRepository.FetchLastDeployedImage(envContainer.AppId, envId)
if err != nil {
impl.Logger.Errorw("failed to fetch last deployed image", "err", err, "app id", envContainer.AppId, "env id", envId)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use camelCase for param keys like appId instead of app id

return resp, err
}
if lastDeployed != nil {
Expand Down
2 changes: 1 addition & 1 deletion wire_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading