Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
41 changes: 40 additions & 1 deletion pkg/api/licenses.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,19 @@ func CreateLicense(c *gin.Context) {
return
}

// Validate that at least one obligation is provided
if input.Obligations == nil || len(*input.Obligations) == 0 {
er := models.LicenseError{
Status: http.StatusBadRequest,
Message: "can not create license with these field values",
Error: "license must have at least one obligation",
Path: c.Request.URL.Path,
Timestamp: time.Now().Format(time.RFC3339),
}
c.JSON(http.StatusBadRequest, er)
return
}

lic := input.ConvertToLicenseDB()

lic.UserId = userId
Expand All @@ -271,7 +284,7 @@ func CreateLicense(c *gin.Context) {
return result.Error
}

if err := tx.Preload("User").First(&lic).Error; err != nil {
if err := tx.Preload("User").Preload("Obligations").First(&lic).Error; err != nil {
er := models.LicenseError{
Status: http.StatusInternalServerError,
Message: "Failed to create license",
Expand All @@ -283,6 +296,19 @@ func CreateLicense(c *gin.Context) {
return result.Error
}

// Validate that license has at least one obligation after creation
if len(lic.Obligations) == 0 {
er := models.LicenseError{
Status: http.StatusBadRequest,
Message: "can not create license with these field values",
Error: "license must have at least one obligation",
Path: c.Request.URL.Path,
Timestamp: time.Now().Format(time.RFC3339),
}
c.JSON(http.StatusBadRequest, er)
return errors.New("license must have at least one obligation")
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This check here is kinda stale. Error message says: Cant create license with these field values. But it has already been created:

_ = db.DB.Transaction(func(tx *gorm.DB) error {
    result := tx.Create(&lic)

Also there are multiple checks after that too. So this looks like more of a defensive check. Can be removed.

if err := utils.AddChangelogsForLicense(tx, userId, &lic, &models.LicenseDB{}); err != nil {
er := models.LicenseError{
Status: http.StatusInternalServerError,
Expand Down Expand Up @@ -436,6 +462,19 @@ func UpdateLicense(c *gin.Context) {
return err
}

// Validate that license has at least one obligation after update
if len(newLicense.Obligations) == 0 {
er := models.LicenseError{
Status: http.StatusBadRequest,
Message: "can not update license with these field values",
Error: "license must have at least one obligation",
Path: c.Request.URL.Path,
Timestamp: time.Now().Format(time.RFC3339),
}
c.JSON(http.StatusBadRequest, er)
return errors.New("license must have at least one obligation")
}
Comment on lines +466 to +476
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I didnt see a step in UpdateLicense endpoint to updated or support obligation support too. Logically it should be there and when the user decides to update the License with Obligation we should put a check for that.

Also this check doesnt make sense bcause: Line 453, we are already preLoading the obligations for the license, can we update the logic and check somewhere there?


if err := utils.AddChangelogsForLicense(tx, userId, &newLicense, &oldLicense); err != nil {
er := models.LicenseError{
Status: http.StatusInternalServerError,
Expand Down
5 changes: 5 additions & 0 deletions pkg/models/licenses.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,11 @@ func (dto *LicenseImportDTO) ConvertToLicenseDB() LicenseDB {

l.ExternalRef = datatypes.NewJSONType(ext)

// Set obligations if provided
if dto.Obligations != nil {
l.Obligations = *dto.Obligations
}

return l
}

Expand Down
35 changes: 35 additions & 0 deletions pkg/utils/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,13 @@ func InsertOrUpdateLicenseOnImport(lic *models.LicenseImportDTO, userId uuid.UUI
return message, importStatus
}

// Validate that at least one obligation is provided for import
if lic.Obligations == nil || len(*lic.Obligations) == 0 {
message = "license must have at least one obligation"
importStatus = IMPORT_FAILED
return message, importStatus
}

_ = db.DB.Transaction(func(tx *gorm.DB) error {
license := lic.ConvertToLicenseDB()
license.UserId = userId
Expand Down Expand Up @@ -167,6 +174,20 @@ func InsertOrUpdateLicenseOnImport(lic *models.LicenseImportDTO, userId uuid.UUI
lic.Id = &newLicense.Id
lic.Shortname = newLicense.Shortname

// Reload license with obligations to check final state
if err := tx.Preload("Obligations").Where(models.LicenseDB{Id: newLicense.Id}).First(&newLicense).Error; err != nil {
message = fmt.Sprintf("failed to update license: %s", err.Error())
importStatus = IMPORT_FAILED
return errors.New(message)
}

// Validate that license has at least one obligation after update
if len(newLicense.Obligations) == 0 {
message = "license must have at least one obligation"
importStatus = IMPORT_FAILED
return errors.New(message)
}

if err := AddChangelogsForLicense(tx, userId, &newLicense, &oldLicense); err != nil {
message = fmt.Sprintf("failed to update license: %s", err.Error())
importStatus = IMPORT_FAILED
Expand All @@ -182,6 +203,20 @@ func InsertOrUpdateLicenseOnImport(lic *models.LicenseImportDTO, userId uuid.UUI
return errors.New(message)
}

// Reload license with obligations to check final state
if err := tx.Preload("Obligations").Where(models.LicenseDB{Id: license.Id}).First(&license).Error; err != nil {
message = fmt.Sprintf("failed to import license: %s", err.Error())
importStatus = IMPORT_FAILED
return errors.New(message)
}

// Validate that license has at least one obligation after creation
if len(license.Obligations) == 0 {
message = "license must have at least one obligation"
importStatus = IMPORT_FAILED
return errors.New(message)
}

// for setting api response
lic.Id = &license.Id
lic.Shortname = license.Shortname
Expand Down
Loading