Przeglądaj źródła

fix: omit display-name field from entity detail and list fields

Entity instance titles are derived from fields such as title or name.
Do not repeat that same field in API fields or the entity details table,
which previously showed the title twice.

Part of #996

Co-authored-by: Cursor <cursoragent@cursor.com>
jamesread 1 dzień temu
rodzic
commit
845df6e9d6

+ 0 - 6
frontend/resources/vue/views/EntityDetailsView.vue

@@ -40,12 +40,6 @@
             {{ entityType }}
           </router-link>
         </dd>
-        <dt v-if="entityDetails.title">
-          Title
-        </dt>
-        <dd v-if="entityDetails.title">
-          {{ entityDetails.title }}
-        </dd>
         <template v-if="entityDetails.fields">
           <template
             v-for="(value, key) in entityDetails.fields"

+ 12 - 0
service/internal/api/api.go

@@ -1733,8 +1733,12 @@ func entityListFields(data any, properties []config.EntityProperty) map[string]s
 		return nil
 	}
 
+	displayFieldKey := entities.DisplayNameFieldKey(data)
 	fields := make(map[string]string, len(properties))
 	for _, property := range properties {
+		if entityFieldIsDisplayName(property.Name, displayFieldKey) {
+			continue
+		}
 		fields[property.Name] = entityPropertyValue(data, property.Name)
 	}
 
@@ -1788,13 +1792,21 @@ func serializeEntityFields(data any) map[string]string {
 		return nil
 	}
 
+	displayFieldKey := entities.DisplayNameFieldKey(data)
 	fields := make(map[string]string)
 	for k, v := range dataMap {
+		if entityFieldIsDisplayName(k, displayFieldKey) {
+			continue
+		}
 		fields[k] = fmt.Sprintf("%v", v)
 	}
 	return fields
 }
 
+func entityFieldIsDisplayName(fieldName, displayFieldKey string) bool {
+	return displayFieldKey != "" && strings.EqualFold(fieldName, displayFieldKey)
+}
+
 func (api *oliveTinAPI) RestartAction(ctx ctx.Context, req *connect.Request[apiv1.RestartActionRequest]) (*connect.Response[apiv1.StartActionResponse], error) {
 	execReqLogEntry, err := api.restartActionLogEntry(req.Msg.ExecutionTrackingId)
 	if err != nil {

+ 70 - 0
service/internal/api/api_entities_list_test.go

@@ -119,6 +119,76 @@ func findEntityDefinition(definitions []*apiv1.EntityDefinition, title string) *
 	return nil
 }
 
+func TestGetEntityOmitsDisplayNameFieldWhenPropertiesUnset(t *testing.T) {
+	entities.ClearEntitiesOfType("vehicle")
+	entities.AddEntity("vehicle", "0", map[string]any{
+		"title":  "My Car",
+		"status": "parked",
+		"vin":    "123",
+	})
+	t.Cleanup(func() {
+		entities.ClearEntitiesOfType("vehicle")
+	})
+
+	cfg := config.DefaultConfig()
+	cfg.Entities = []*config.EntityFile{{Name: "vehicle"}}
+	cfg.Sanitize()
+
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+	ts, client := getNewTestServerAndClientWithExecutor(cfg, ex)
+	defer ts.Close()
+
+	resp, err := client.GetEntity(context.Background(), connect.NewRequest(&apiv1.GetEntityRequest{
+		Type:      "vehicle",
+		UniqueKey: "0",
+	}))
+	require.NoError(t, err)
+	require.NotNil(t, resp.Msg)
+
+	assert.Equal(t, "My Car", resp.Msg.Title)
+	assert.Equal(t, "parked", resp.Msg.Fields["status"])
+	assert.Equal(t, "123", resp.Msg.Fields["vin"])
+	assert.NotContains(t, resp.Msg.Fields, "title")
+}
+
+func TestGetEntityOmitsDisplayNamePropertyFromConfiguredFields(t *testing.T) {
+	entities.ClearEntitiesOfType("vehicle")
+	entities.AddEntity("vehicle", "0", map[string]any{
+		"title":  "My Car",
+		"status": "parked",
+	})
+	t.Cleanup(func() {
+		entities.ClearEntitiesOfType("vehicle")
+	})
+
+	cfg := config.DefaultConfig()
+	cfg.Entities = []*config.EntityFile{
+		{
+			Name: "vehicle",
+			Properties: []config.EntityProperty{
+				{Name: "title", Title: "Title"},
+				{Name: "status", Title: "Status"},
+			},
+		},
+	}
+	cfg.Sanitize()
+
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+	ts, client := getNewTestServerAndClientWithExecutor(cfg, ex)
+	defer ts.Close()
+
+	resp, err := client.GetEntity(context.Background(), connect.NewRequest(&apiv1.GetEntityRequest{
+		Type:      "vehicle",
+		UniqueKey: "0",
+	}))
+	require.NoError(t, err)
+
+	assert.Equal(t, "parked", resp.Msg.Fields["status"])
+	assert.NotContains(t, resp.Msg.Fields, "title")
+}
+
 func TestGetEntityRestrictsFieldsToConfiguredProperties(t *testing.T) {
 	entities.ClearEntitiesOfType("server")
 	entities.AddEntity("server", "0", map[string]any{

+ 8 - 0
service/internal/entities/entities_test.go

@@ -53,6 +53,14 @@ func TestGetEntityInstancesOrdered_lexicographicKeys(t *testing.T) {
 	assert.Equal(t, "zebra", ordered[2].UniqueKey)
 }
 
+func TestDisplayNameFieldKey(t *testing.T) {
+	assert.Equal(t, "title", DisplayNameFieldKey(map[string]any{"title": "Car"}))
+	assert.Equal(t, "name", DisplayNameFieldKey(map[string]any{"name": "Car"}))
+	assert.Equal(t, "Name", DisplayNameFieldKey(map[string]any{"Name": "Car"}))
+	assert.Equal(t, "", DisplayNameFieldKey(map[string]any{"status": "running"}))
+	assert.Equal(t, "", DisplayNameFieldKey("not a map"))
+}
+
 func TestGetEntityInstancesOrdered_emptyOrMissing(t *testing.T) {
 	ordered := GetEntityInstancesOrdered("nonexistent_type")
 	assert.Nil(t, ordered)

+ 52 - 15
service/internal/entities/storage.go

@@ -124,28 +124,65 @@ func AddEntity(entityName string, entityKey string, data any) {
 	rwmutex.Unlock()
 }
 
+var entityDisplayNameCandidates = []string{"title", "name", "id", "hostname", "host", "label"}
+
 //gocyclo:ignore
-func findEntityTitle(data any) string {
-	if mapData, ok := data.(map[string]any); ok {
-		keys := make(map[string]string)
+func entityDisplayNameLookup(data map[string]any) (fieldKey string, value string, found bool) {
+	keys := make(map[string]string, len(data))
+	for k := range data {
+		keys[strings.ToLower(k)] = k
+	}
+
+	for _, candidate := range entityDisplayNameCandidates {
+		lookupKey, exists := keys[strings.ToLower(candidate)]
+		if !exists {
+			continue
+		}
 
-		for k := range mapData {
-			lookupKey := strings.ToLower(k)
-			keys[lookupKey] = k
+		rawValue, ok := data[lookupKey]
+		if !ok {
+			continue
 		}
 
-		for _, key := range []string{"title", "name", "id", "hostname", "host", "label"} {
-			if lookupKey, exists := keys[strings.ToLower(key)]; exists {
-				if value, ok := mapData[lookupKey]; ok {
-					if valueStr, ok := value.(string); ok {
-						return valueStr
-					}
-				}
-			}
+		valueStr, ok := rawValue.(string)
+		if !ok {
+			continue
 		}
+
+		return lookupKey, valueStr, true
+	}
+
+	return "", "", false
+}
+
+// DisplayNameFieldKey returns the data-file field used as the entity instance title, or "".
+func DisplayNameFieldKey(data any) string {
+	mapData, ok := data.(map[string]any)
+	if !ok {
+		return ""
+	}
+
+	fieldKey, _, found := entityDisplayNameLookup(mapData)
+	if !found {
+		return ""
+	}
+
+	return fieldKey
+}
+
+//gocyclo:ignore
+func findEntityTitle(data any) string {
+	mapData, ok := data.(map[string]any)
+	if !ok {
+		return "Untitled Entity"
+	}
+
+	_, value, found := entityDisplayNameLookup(mapData)
+	if !found {
+		return "Untitled Entity"
 	}
 
-	return "Untitled Entity"
+	return value
 }
 
 func ClearEntitiesOfType(entityType string) {