Skip to content

Conversation

@Ash-exp
Copy link
Contributor

@Ash-exp Ash-exp commented Mar 22, 2023

Description

Develop GET and PUT methods of Cluster Description.
Cluster Description Fields :

  • ClusterName
  • ClusterCreatedOn (Time Stamp)
  • ClusterCreatedBy (user email id)
  • ClusterNote Fields :
    • Description (Markup Text)
    • CreatedBy (user email id)
    • CreatedOn (Time Stamp)

Maintain audit history of cluster description note in database.

Fixes #2773
Tech Doc

How Has This Been Tested?

GET: /description?id={clusterId}

  • RBAC implementation for cluster get access.
  • return 404 for invalid clusterId.
  • Always Return clusterName, clusterCreatedOn, clusterCreatedBy if cluster exists.
  • Return NULL clusterNotes for none existing cluster notes.

PUT: /description/note

  • RBAC implementation for cluster update access.
  • return 404 for invalid clusterId
  • Return updated cluster notes on successful updation.
  • Audit gets created on CREATE and UPDATE methods implemented on cluster notes.

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.

Does this PR introduce a user-facing change?


@Ash-exp Ash-exp added the enhancement New feature or request label Mar 22, 2023
@Ash-exp Ash-exp self-assigned this Mar 22, 2023
return impl.dbConnection.Update(model)
}

func (impl ClusterNoteRepositoryImpl) Delete(model *ClusterNote) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we delete, use soft delete

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed delete method 👍

return clusterNote, err
}

func (impl ClusterNoteRepositoryImpl) FindByClusterIds(id []int) ([]ClusterNote, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

use Ids in params

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated 👍

return impl.dbConnection.Insert(model)
}

func (impl ClusterNoteHistoryRepositoryImpl) FindHistoryByNoteId(id []int) ([]ClusterNoteHistory, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

user pointer * array

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented 👍

if err != nil {
return err
}
return impl.clusterNoteRepository.Delete(model)
Copy link
Contributor

Choose a reason for hiding this comment

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

avoid hard delete

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed delete method 👍

}
clusterAudit.CreatedBy = userId
clusterAudit.UpdatedBy = userId
clusterAudit.CreatedOn = time.Now()
Copy link
Contributor

Choose a reason for hiding this comment

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

on update why created by and created on ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

created by and created on is required for saving the record to the audit history

Copy link
Contributor

Choose a reason for hiding this comment

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

createdBy and CreatedOn for audit logs will not change in update operation.
clusterAudit.CreatedBy = model.Id
clusterAudit.UpdatedBy = model.UpdatedBy

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

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

No Coverage information No Coverage information
3.3% 3.3% Duplication

clusterAudit.UpdatedOn = time.Now()
err = impl.clusterNoteHistoryRepository.SaveHistory(clusterAudit)
if err != nil {
return nil, err
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't return err, only log it if there is error in saving history

Copy link
Contributor Author

Choose a reason for hiding this comment

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

implemented 👍

}
clusterAudit.CreatedBy = userId
clusterAudit.UpdatedBy = userId
clusterAudit.CreatedOn = time.Now()
Copy link
Contributor

Choose a reason for hiding this comment

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

createdBy and CreatedOn for audit logs will not change in update operation.
clusterAudit.CreatedBy = model.Id
clusterAudit.UpdatedBy = model.UpdatedBy

clusterAudit.UpdatedOn = time.Now()
err = impl.clusterNoteHistoryRepository.SaveHistory(clusterAudit)
if err != nil {
return nil, err
Copy link
Contributor

Choose a reason for hiding this comment

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

As user main flow (saving cluster description) is not getting affected by audit logs, don't return error in this case, only log it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

implemented 👍

Description string `json:"description" validate:"required"`
UpdatedBy int `json:"updated_by"`
UpdatedOn time.Time `json:"updated_on"`
}
Copy link
Contributor

Choose a reason for hiding this comment

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

is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah this is required for unmarshalling of ClusterNoteRepository


-- add foreign key
ALTER TABLE "public"."cluster_note"
ADD FOREIGN KEY ("cluster_id") REFERENCES "public"."cluster" ("id");
Copy link
Contributor

Choose a reason for hiding this comment

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

add foreign key in table create query, alter query not required

Copy link
Contributor Author

Choose a reason for hiding this comment

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

implemented 👍

}
//RBAC enforcer Ends
bean, err := impl.clusterNoteService.FindByClusterId(i)
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

use join instead of two queries

Copy link
Contributor Author

Choose a reason for hiding this comment

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

implemented 👍

iamayushm
iamayushm previously approved these changes May 1, 2023
@sonarqubecloud
Copy link

sonarqubecloud bot commented May 5, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

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

No Coverage information No Coverage information
4.2% 4.2% Duplication

@Ash-exp Ash-exp merged commit 854de20 into main May 5, 2023
@Ash-exp Ash-exp deleted the feature-cluster-description branch May 5, 2023 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Readme/Metadata for clusters added on Devtron

4 participants