fix: implement entity field in /{software/,}logs

italia · Nov 3, 2022 · a9fdd39 · a9fdd39
1 parent de88cdc
commit a9fdd39
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 8 deletions.
diff --git a/developers-italia.oas.yaml b/developers-italia.oas.yaml
@@ -1577,6 +1577,12 @@ components:
           type: string
           maxLength: 255
           pattern: '.*'
+          description: >
+            The resource this log is about (fe. a particular Publisher / Software).
+            It might be absent if the log is not about a specific resource or if
+            the resource doesn't exist yet, like in case of the error log caused by
+            failure to create it.
+          example: /software/7589be36-f046-45c6-9223-b7de9dbf06cd
           readOnly: true
       required:
         - id

diff --git a/internal/handlers/logs.go b/internal/handlers/logs.go
@@ -221,11 +221,13 @@ func (p *Log) PostSoftwareLog(ctx *fiber.Ctx) error {
 		return common.ErrorWithValidationErrors(fiber.StatusUnprocessableEntity, "can't create Log", "invalid format", err)
 	}
 
+	table := models.Software{}.TableName()
+
 	log := models.Log{
 		ID:         utils.UUIDv4(),
 		Message:    logReq.Message,
-		EntityID:   software.ID,
-		EntityType: models.Software{}.TableName(),
+		EntityID:   &software.ID,
+		EntityType: &table,
 	}
 
 	if err := p.db.Create(&log).Error; err != nil {

diff --git a/internal/models/models.go b/internal/models/models.go
@@ -25,8 +25,9 @@ type Log struct {
 	DeletedAt gorm.DeletedAt `json:"-" gorm:"index"`
 
 	// Entity this Log entry is about (fe. Publisher, Software, etc.)
-	EntityID   string `json:"-"`
-	EntityType string `json:"-"`
+	EntityID   *string `json:"-"`
+	EntityType *string `json:"-"`
+	Entity     string  `json:"entity,omitempty" gorm:"->;type:GENERATED ALWAYS AS (CASE WHEN entity_id IS NULL THEN NULL ELSE ('/' || entity_type || '/' || entity_id) END);default:(-);"` //nolint:lll
 }
 
 type Publisher struct {

diff --git a/main_test.go b/main_test.go
@@ -2101,6 +2101,8 @@ func TestSoftwareEndpoints(t *testing.T) {
 					_, err = time.Parse(time.RFC3339, log["updatedAt"].(string))
 					assert.Nil(t, err)
 
+					assert.Equal(t, "/software/c353756e-8597-4e46-a99b-7da2e141603b", log["entity"])
+
 					for key := range log {
 						assert.Contains(t, []string{"id", "createdAt", "updatedAt", "message", "entity"}, key)
 					}
@@ -2112,8 +2114,6 @@ func TestSoftwareEndpoints(t *testing.T) {
 
 					prevCreatedAt = &createdAt
 				}
-
-				// TODO assert.NotEmpty(t, firstLog["entity"])
 			},
 		},
 		{
@@ -2185,6 +2185,8 @@ func TestSoftwareEndpoints(t *testing.T) {
 				_, err = time.Parse(time.RFC3339, response["updatedAt"].(string))
 				assert.Nil(t, err)
 
+				assert.Equal(t, "/software/c353756e-8597-4e46-a99b-7da2e141603b", response["entity"])
+
 				// TODO: check the record was actually created in the database
 			},
 		},
@@ -2533,13 +2535,25 @@ func TestLogsEndpoints(t *testing.T) {
 					_, err = time.Parse(time.RFC3339, log["updatedAt"].(string))
 					assert.Nil(t, err)
 
+					// Only certain logs from the fixtures have an associated entity.
+					//
+					// FIXME: This is ugly, see the issue about improving tests:
+					// https://github.com/italia/developers-italia-api/issues/91
+					if log["id"] == "2dfb2bc2-042d-11ed-9338-d8bbc146d165" ||
+						log["id"] == "12f30d9e-042e-11ed-8ddc-d8bbc146d165" ||
+						log["id"] == "18a70362-042e-11ed-b793-d8bbc146d165" {
+						assert.Equal(t, "/software/c353756e-8597-4e46-a99b-7da2e141603b", log["entity"])
+					} else if log["id"] == "53650508-042e-11ed-9b84-d8bbc146d165" {
+						assert.Equal(t, "/publishers/2ded32eb-c45e-4167-9166-a44e18b8adde", log["entity"])
+					} else {
+						assert.Nil(t, log["entity"])
+					}
+
 					var prevCreatedAt *time.Time = nil
 					for key := range log {
 						assert.Contains(t, []string{"id", "createdAt", "updatedAt", "message", "entity"}, key)
 					}
 
-					// TODO assert.NotEmpty(t, firstLog["entity"])
-
 					// Check the logs are ordered by descending createdAt
 					if prevCreatedAt != nil {
 						assert.GreaterOrEqual(t, *prevCreatedAt, createdAt)
@@ -2708,6 +2722,8 @@ func TestLogsEndpoints(t *testing.T) {
 				_, err = time.Parse(time.RFC3339, response["updatedAt"].(string))
 				assert.Nil(t, err)
 
+				assert.Nil(t, response["entity"])
+
 				// TODO: check the record was actually created in the database
 			},
 		},

diff --git a/test/testdata/fixtures/logs.yml b/test/testdata/fixtures/logs.yml
@@ -27,6 +27,8 @@
 - id: 53650508-042e-11ed-9b84-d8bbc146d165
   created_at: 2010-02-15 23:59:59
   updated_at: 2010-12-31 23:59:59
+  entity_id: 2ded32eb-c45e-4167-9166-a44e18b8adde
+  entity_type: publishers
   message: A log message
 
 - id: 55438aac-042e-11ed-848a-d8bbc146d165