fix(licenses): enforce min one obligation requirement#191
Open
krishi-agrawal wants to merge 1 commit intofossology:mainfrom
Open
fix(licenses): enforce min one obligation requirement#191krishi-agrawal wants to merge 1 commit intofossology:mainfrom
krishi-agrawal wants to merge 1 commit intofossology:mainfrom
Conversation
Signed-off-by: Krishi Agrawal <krishi.agrawal26@gmail.com>
Kaushl2208
requested changes
Jan 13, 2026
Member
Kaushl2208
left a comment
Kaushl2208
left a comment
There was a problem hiding this comment.
Hey @krishi-agrawal ,
- The issue is just not about restrictions to the License creation, You will need to make some logic changes to the endpoints as well both (Create and Update).
- If you see the test cases are failing since the changes made. Please dont forget to update or write the test cases for your changes.
- If you need to discuss the logic change, We can discuss it over the issue.
| c.JSON(http.StatusBadRequest, er) | ||
| return errors.New("license must have at least one obligation") | ||
| } | ||
|
|
Member
There was a problem hiding this comment.
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.
Comment on lines
+466
to
+476
| 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") | ||
| } |
Member
There was a problem hiding this comment.
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?
abhayrajjais01
added a commit
to abhayrajjais01/LicenseDb
that referenced
this pull request
Jan 22, 2026
…ossology#136) - Add validation to CreateLicense endpoint to reject licenses without obligations - Add validation to UpdateLicense endpoint to prevent removing all obligations - Add validation to ImportLicenses flow for imported licenses - Update existing tests to include obligations - Add new test case for license creation without obligations - Create getTestObligation() helper function for tests Addresses all feedback from PR fossology#191: - Validation occurs before database transactions (not after) - UpdateLicense endpoint now validates obligations - All test cases updated and passing - Clear, user-friendly error messages Fixes fossology#136
abhayrajjais01
added a commit
to abhayrajjais01/LicenseDb
that referenced
this pull request
Jan 22, 2026
…ossology#136) - Add validation to CreateLicense endpoint to reject licenses without obligations - Add validation to UpdateLicense endpoint to prevent removing all obligations - Add validation to ImportLicenses flow for imported licenses - Update existing tests to include obligations - Add new test case for license creation without obligations - Create getTestObligation() helper function for tests Addresses all feedback from PR fossology#191: - Validation occurs before database transactions (not after) - UpdateLicense endpoint now validates obligations - All test cases updated and passing - Clear, user-friendly error messages Fixes fossology#136 Signed-off-by: abhayrajjais01 <abhayraj916146@gmail.com>
abhayrajjais01
added a commit
to abhayrajjais01/LicenseDb
that referenced
this pull request
Jan 25, 2026
…ossology#136) - Add validation to CreateLicense endpoint to reject licenses without obligations - Add validation to UpdateLicense endpoint to prevent removing all obligations - Add validation to ImportLicenses flow for imported licenses - Update existing tests to include obligations - Add new test case for license creation without obligations - Create getTestObligation() helper function for tests Addresses all feedback from PR fossology#191: - Validation occurs before database transactions (not after) - UpdateLicense endpoint now validates obligations - All test cases updated and passing - Clear, user-friendly error messages Fixes fossology#136 Signed-off-by: abhayrajjais01 <abhayraj916146@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
when creating, updating, or importing licenses.
Submitter Checklist
References