Skip to content

Conversation

@adi6859
Copy link
Contributor

@adi6859 adi6859 commented Mar 24, 2023

Description

In this PR I have to store notes.txt in db for helm charts deployed via gitOps and fetch from db if present .

Fixes Ab2484

How Has This Been Tested?

  1. first,I have tested locally.
  2. Then tested by deploying image on vm.
  3. there are two test cases here:-
    a. when notes is not present in db.
    b. when notes is present in db.
    c.for all these test test cases , I have checked for timing and it is working fine , if fetching from db it is taking less time and when fetching from kubelink it is taking more time.
    3.I have also done testing by putting multiple breakpoints.

Checklist:

  • The title of the PR states what changed and the related issues number (used for the release note).
  • Does this PR requires documentation updates?
  • I've updated documentation as required by this PR.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have tested it for all user roles.
  • I have added all the required unit/api test cases.

@adi6859 adi6859 requested a review from vikramdevtron March 24, 2023 05:51
@adi6859 adi6859 changed the title feature:store notes.txt in db and fetch from db feat:store notes.txt in db and fetch from db Mar 24, 2023
@adi6859 adi6859 requested a review from iamayushm March 24, 2023 11:33
handler.Logger.Infow("request payload, FetchNotesForArgoInstalledApp, app store", "installedAppId", installedAppId, "envId", envId)

notes, appName, err := handler.installedAppService.FindNotesForArgoApplication(installedAppId, envId)
notes, err := handler.installedAppService.FetchNotesFromdb(installedAppId, envId, token, handler.checkNotesAuth)
Copy link
Contributor

Choose a reason for hiding this comment

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

change the function name, fetchChartNotes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}
return model, nil
}
func (impl InstalledAppRepositoryImpl) FetchNotesFromdb(installedAppId int) (*InstalledApps, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

FetchNotes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return repoName
}

func (impl AppStoreDeploymentServiceImpl) AppStoreDeployOperationNotesUpdate(installAppId int, notes string) (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

UpdateNotesForInstalledApp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return "", nil
}

if installedApp.Notes == "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

put comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

isValidAuth := checkNotesAuth(token, appName, envId)
if !isValidAuth {
impl.logger.Errorw("unauthorized user", "isValidAuth", isValidAuth)
return "", nil
Copy link
Contributor

Choose a reason for hiding this comment

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

return error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

impl.logger.Errorw("error fetching installed app version in installed app service", "err", err)
return "", err
}
appStoreAppVersion, err := impl.appStoreApplicationVersionRepository.FindById(installedAppVerison.AppStoreApplicationVersion.Id)
Copy link
Contributor

Choose a reason for hiding this comment

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

we are already getting appStoreAppVersion by installedAppVerison, err := impl.installedAppRepository.GetInstalledAppVersionByInstalledAppIdAndEnvId(installedAppId, envId) , no need to fetch it again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok,changes done

iamayushm
iamayushm previously approved these changes Mar 27, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@adi6859 adi6859 merged commit 9af6cda into main Mar 27, 2023
@adi6859 adi6859 deleted the store-notes-db branch March 27, 2023 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants