Explorar el Código

fix: Entity ordering (#886, #762)

jamesread hace 3 meses
padre
commit
5ff6b5d080

+ 3 - 3
AGENTS.md

@@ -13,6 +13,7 @@ If you are looking for OliveTin's AI policy, you can find it in `AI.md`.
 - **Frontend (Vue 3)**: `frontend/` (served by the service)
 - **Integration tests**: `integration-tests/`
 - **Protos/Generated**: `proto/`, `service/gen/...`
+- **Specs**: `specs/` — Markdown specs that define how code should behave in human-readable form. When changing behavior in a spec-covered area, keep implementation and tests aligned with the spec; do not reference code or symbols in specs (English only).
 
 ### How to Run
 - Run the server (dev):
@@ -62,11 +63,10 @@ If you are looking for OliveTin's AI policy, you can find it in `AI.md`.
 ### Contributing Checklist
 - Review the contributing guidelines at `CONTRIBUTING.adoc`.
 - Review the AI guidance in `AI.md`.
-- Review the pull request template at `.github/PULL_REQUEST_TEMPLATE.md`. 
+- Review the pull request template at `.github/PULL_REQUEST_TEMPLATE.md`.
+- When changing behaviour covered by a spec in `specs/`, ensure implementation and tests match the spec.
 
 ### Troubleshooting
 - API tests failing with content-type errors: ensure Connect handler is served under `/api/` and the client targets that base URL.
 - Executor panics: check for nil `Binding/Action` and add guards in step functions.
 - Integration timeouts: wait for `loaded-dashboard` and use selectors matching the Vue UI.
-
-

+ 5 - 5
integration-tests/tests/entityFilesWithLongIntsUseStandardForm/entityFilesWithLongIntsUseStandardForm.js

@@ -2,8 +2,8 @@
 import { describe, it, before, after } from 'mocha'
 import { expect } from 'chai'
 import { By, until, Condition } from 'selenium-webdriver'
-import { 
-  getRootAndWait, 
+import {
+  getRootAndWait,
   getActionButtons,
   takeScreenshotOnFailure,
 } from '../../lib/elements.js'
@@ -29,8 +29,8 @@ describe('config: entities', function () {
     expect(buttons).to.not.be.null
     expect(buttons).to.have.length(5)
 
-    // Test INT with 10 numbers
-    const buttonInt10 = await buttons[2]   
+    // Entity buttons are in numeric key order (0,1,2,3,4); first row is "INT with 10 numbers"
+    const buttonInt10 = await buttons[0]
     expect(await buttonInt10.getAttribute('title')).to.be.equal('Test me INT with 10 numbers')
     await buttonInt10.click()
 
@@ -49,7 +49,7 @@ describe('config: entities', function () {
     // Check that the execution completed successfully by looking at the status
     const statusElement = await webdriver.findElement(By.id('execution-dialog-status'))
     const statusText = await statusElement.getText()
-    
+
     // The status should indicate success (not "Executing..." or "Failed")
     expect(statusText).to.not.include('Executing')
     expect(statusText).to.not.include('Failed')

+ 1 - 3
service/internal/api/apiActions.go

@@ -185,9 +185,7 @@ func buildChoices(arg config.ActionArgument) []*apiv1.ActionArgumentChoice {
 func buildChoicesEntity(firstChoice config.ActionArgumentChoice, entityTitle string) []*apiv1.ActionArgumentChoice {
 	ret := []*apiv1.ActionArgumentChoice{}
 
-	entList := entities.GetEntityInstances(entityTitle)
-
-	for _, ent := range entList {
+	for _, ent := range entities.GetEntityInstancesOrdered(entityTitle) {
 		ret = append(ret, &apiv1.ActionArgumentChoice{
 			Value: tpl.ParseTemplateOfActionBeforeExec(firstChoice.Value, ent),
 			Title: tpl.ParseTemplateOfActionBeforeExec(firstChoice.Title, ent),

+ 26 - 0
service/internal/api/api_test.go

@@ -450,3 +450,29 @@ func bindingIdsFromComponent(c *apiv1.DashboardComponent) []string {
 	}
 	return append(ids, bindingIdsInDashboardContents(c.Contents)...)
 }
+
+func TestOrderTopLevelDashboardComponents_RegularFieldsetsPreserveConfigOrder(t *testing.T) {
+	zebra := &apiv1.DashboardComponent{Title: "Zebra", Type: "fieldset", EntityType: ""}
+	alpha := &apiv1.DashboardComponent{Title: "Alpha", Type: "fieldset", EntityType: ""}
+	root := &apiv1.DashboardComponent{Title: "Actions", Type: "fieldset", EntityType: ""}
+	components := []*apiv1.DashboardComponent{zebra, alpha, root}
+
+	out := orderTopLevelDashboardComponents(components)
+
+	require.Len(t, out, 3)
+	assert.Same(t, zebra, out[0], "first must be Zebra (config order)")
+	assert.Same(t, alpha, out[1], "second must be Alpha (config order)")
+	assert.Same(t, root, out[2], "third must be root Actions fieldset")
+}
+
+func TestOrderTopLevelDashboardComponents_SortablesSorted(t *testing.T) {
+	entityBeta := &apiv1.DashboardComponent{Title: "Beta", Type: "fieldset", EntityType: "server"}
+	entityAlpha := &apiv1.DashboardComponent{Title: "Alpha", Type: "fieldset", EntityType: "server"}
+	components := []*apiv1.DashboardComponent{entityBeta, entityAlpha}
+
+	out := orderTopLevelDashboardComponents(components)
+
+	require.Len(t, out, 2)
+	assert.Equal(t, "Alpha", out[0].Title, "sortables ordered by title")
+	assert.Equal(t, "Beta", out[1].Title)
+}

+ 3 - 4
service/internal/api/dashboard_entities.go

@@ -11,9 +11,8 @@ import (
 func buildEntityFieldsets(entityTitle string, tpl *config.DashboardComponent, rr *DashboardRenderRequest) []*apiv1.DashboardComponent {
 	ret := make([]*apiv1.DashboardComponent, 0)
 
-	entities := entities.GetEntityInstances(entityTitle)
-
-	for _, ent := range entities {
+	orderedEntities := entities.GetEntityInstancesOrdered(entityTitle)
+	for _, ent := range orderedEntities {
 		fs := buildEntityFieldset(tpl, ent, rr)
 
 		if len(fs.Contents) > 0 {
@@ -30,7 +29,7 @@ func buildEntityFieldset(component *config.DashboardComponent, ent *entities.Ent
 		Type:       "fieldset",
 		Contents:   removeFieldsetIfHasNoLinks(buildEntityFieldsetContents(component.Contents, ent, component.Entity, rr)),
 		CssClass:   tpl.ParseTemplateOfActionBeforeExec(component.CssClass, ent),
-		Action:     rr.findAction(component.Title),
+		Action:     rr.findActionForEntity(component.Title, ent),
 		EntityType: component.Entity,
 		EntityKey:  ent.UniqueKey,
 	}

+ 72 - 8
service/internal/api/dashboards.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"sort"
+	"strconv"
 
 	apiv1 "github.com/OliveTin/OliveTin/gen/olivetin/api/v1"
 	acl "github.com/OliveTin/OliveTin/internal/acl"
@@ -113,7 +114,7 @@ func buildDashboardFromConfig(dashboard *config.DashboardComponent, rr *Dashboar
 func buildDashboardFromConfigWithEntity(dashboard *config.DashboardComponent, rr *DashboardRenderRequest, entity *entities.Entity) *apiv1.Dashboard {
 	return &apiv1.Dashboard{
 		Title:    dashboard.Title,
-		Contents: sortActions(removeNulls(getDashboardComponentContentsWithEntity(dashboard, rr, entity))),
+		Contents: orderTopLevelDashboardComponents(removeNulls(getDashboardComponentContentsWithEntity(dashboard, rr, entity))),
 	}
 }
 
@@ -148,33 +149,49 @@ func buildDefaultDashboard(rr *DashboardRenderRequest) *apiv1.Dashboard {
 			continue
 		}
 
-		fieldset.Contents = append(fieldset.Contents, &apiv1.DashboardComponent{
+		comp := &apiv1.DashboardComponent{
 			Type:   "link",
 			Title:  action.Title,
 			Icon:   action.Icon,
 			Action: action,
-		})
+		}
+		if binding.Entity != nil {
+			comp.EntityKey = binding.Entity.UniqueKey
+		}
+		fieldset.Contents = append(fieldset.Contents, comp)
 	}
 
 	if len(fieldset.Contents) > 0 {
-		fieldset.Contents = sortActions(fieldset.Contents)
+		fieldset.Contents = sortDashboardComponents(fieldset.Contents)
 		db.Contents = append(db.Contents, fieldset)
 	}
 
 	return db
 }
 
-func sortActions(components []*apiv1.DashboardComponent) []*apiv1.DashboardComponent {
+func entityKeyLess(a, b string) bool {
+	ai, errA := strconv.ParseInt(a, 10, 64)
+	bi, errB := strconv.ParseInt(b, 10, 64)
+	if errA == nil && errB == nil {
+		return ai < bi
+	}
+	return a < b
+}
+
+//gocyclo:ignore
+func sortDashboardComponents(components []*apiv1.DashboardComponent) []*apiv1.DashboardComponent {
 	sort.Slice(components, func(i, j int) bool {
 		if components[i].Action == nil || components[j].Action == nil {
 			return components[i].Title < components[j].Title
 		}
 
-		if components[i].Action.Order == components[j].Action.Order {
-			return components[i].Action.Title < components[j].Action.Title
-		} else {
+		if components[i].Action.Order != components[j].Action.Order {
 			return components[i].Action.Order < components[j].Action.Order
 		}
+		if components[i].EntityKey != components[j].EntityKey {
+			return entityKeyLess(components[i].EntityKey, components[j].EntityKey)
+		}
+		return components[i].Action.Title < components[j].Action.Title
 	})
 
 	return components
@@ -194,6 +211,53 @@ func removeNulls(components []*apiv1.DashboardComponent) []*apiv1.DashboardCompo
 	return ret
 }
 
+func isRegularFieldset(component *apiv1.DashboardComponent, index int, totalLen int) bool {
+	if component == nil || component.Type != "fieldset" || component.EntityType != "" {
+		return false
+	}
+	return index != totalLen-1
+}
+
+func partitionTopLevelComponents(components []*apiv1.DashboardComponent) (regular, sortables []*apiv1.DashboardComponent, isRegular []bool) {
+	regular = make([]*apiv1.DashboardComponent, 0)
+	sortables = make([]*apiv1.DashboardComponent, 0)
+	isRegular = make([]bool, len(components))
+	for i, c := range components {
+		anchor := isRegularFieldset(c, i, len(components))
+		isRegular[i] = anchor
+		if anchor {
+			regular = append(regular, c)
+		} else {
+			sortables = append(sortables, c)
+		}
+	}
+	return regular, sortables, isRegular
+}
+
+func mergeOrderedTopLevelComponents(regular, sortables []*apiv1.DashboardComponent, isRegular []bool) []*apiv1.DashboardComponent {
+	out := make([]*apiv1.DashboardComponent, 0, len(isRegular))
+	regIdx, sortIdx := 0, 0
+	for _, anchor := range isRegular {
+		if anchor {
+			out = append(out, regular[regIdx])
+			regIdx++
+		} else {
+			out = append(out, sortables[sortIdx])
+			sortIdx++
+		}
+	}
+	return out
+}
+
+func orderTopLevelDashboardComponents(components []*apiv1.DashboardComponent) []*apiv1.DashboardComponent {
+	if len(components) == 0 {
+		return components
+	}
+	regular, sortables, isRegular := partitionTopLevelComponents(components)
+	sortDashboardComponents(sortables)
+	return mergeOrderedTopLevelComponents(regular, sortables, isRegular)
+}
+
 func getDashboardComponentContentsWithEntity(dashboard *config.DashboardComponent, rr *DashboardRenderRequest, entity *entities.Entity) []*apiv1.DashboardComponent {
 	ret := make([]*apiv1.DashboardComponent, 0)
 	rootFieldset := createRootFieldset()

+ 44 - 1
service/internal/entities/entities_test.go

@@ -1,8 +1,10 @@
 package entities
 
 import (
-	// "github.com/stretchr/testify/assert"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestLoadObjectPerLineJsonFile(t *testing.T) {
@@ -16,3 +18,44 @@ func TestLoadObjectPerLineJsonFile(t *testing.T) {
 		assert.Equal(t, "1234567890", GetEntity("testrow", "0"), "Value should match expected value")
 	*/
 }
+
+func TestGetEntityInstancesOrdered_numericKeys(t *testing.T) {
+	ClearEntitiesOfType("order_test")
+	defer ClearEntitiesOfType("order_test")
+
+	AddEntity("order_test", "2", map[string]any{"title": "Second"})
+	AddEntity("order_test", "0", map[string]any{"title": "Zeroth"})
+	AddEntity("order_test", "10", map[string]any{"title": "Tenth"})
+	AddEntity("order_test", "1", map[string]any{"title": "First"})
+
+	ordered := GetEntityInstancesOrdered("order_test")
+	require.Len(t, ordered, 4, "should return 4 entities")
+	assert.Equal(t, "0", ordered[0].UniqueKey, "first key should be 0")
+	assert.Equal(t, "1", ordered[1].UniqueKey, "second key should be 1")
+	assert.Equal(t, "2", ordered[2].UniqueKey, "third key should be 2")
+	assert.Equal(t, "10", ordered[3].UniqueKey, "fourth key should be 10 (numeric order)")
+}
+
+func TestGetEntityInstancesOrdered_lexicographicKeys(t *testing.T) {
+	ClearEntitiesOfType("order_test_lex")
+	defer ClearEntitiesOfType("order_test_lex")
+
+	AddEntity("order_test_lex", "zebra", map[string]any{"title": "Z"})
+	AddEntity("order_test_lex", "alpha", map[string]any{"title": "A"})
+	AddEntity("order_test_lex", "beta", map[string]any{"title": "B"})
+
+	ordered := GetEntityInstancesOrdered("order_test_lex")
+	require.Len(t, ordered, 3, "should return 3 entities")
+	assert.Equal(t, "alpha", ordered[0].UniqueKey)
+	assert.Equal(t, "beta", ordered[1].UniqueKey)
+	assert.Equal(t, "zebra", ordered[2].UniqueKey)
+}
+
+func TestGetEntityInstancesOrdered_emptyOrMissing(t *testing.T) {
+	ordered := GetEntityInstancesOrdered("nonexistent_type")
+	assert.Nil(t, ordered)
+
+	ClearEntitiesOfType("empty_test")
+	ordered = GetEntityInstancesOrdered("empty_test")
+	assert.Nil(t, ordered)
+}

+ 45 - 0
service/internal/entities/storage.go

@@ -10,6 +10,8 @@ package entities
  */
 
 import (
+	"sort"
+	"strconv"
 	"strings"
 	"sync"
 )
@@ -64,6 +66,49 @@ func GetEntityInstances(entityName string) entityInstancesByKey {
 	return make(entityInstancesByKey, 0)
 }
 
+func GetEntityInstancesOrdered(entityName string) []*Entity {
+	instances := GetEntityInstances(entityName)
+	if len(instances) == 0 {
+		return nil
+	}
+
+	keys := make([]string, 0, len(instances))
+	for key := range instances {
+		keys = append(keys, key)
+	}
+	sort.Slice(keys, func(i, j int) bool {
+		return compareEntityKeys(keys[i], keys[j]) < 0
+	})
+
+	result := make([]*Entity, 0, len(keys))
+	for _, key := range keys {
+		result = append(result, instances[key])
+	}
+	return result
+}
+
+//gocyclo:ignore
+func compareEntityKeys(a, b string) int {
+	ai, errA := strconv.ParseInt(a, 10, 64)
+	bi, errB := strconv.ParseInt(b, 10, 64)
+	if errA == nil && errB == nil {
+		if ai < bi {
+			return -1
+		}
+		if ai > bi {
+			return 1
+		}
+		return 0
+	}
+	if a < b {
+		return -1
+	}
+	if a > b {
+		return 1
+	}
+	return 0
+}
+
 func AddEntity(entityName string, entityKey string, data any) {
 	rwmutex.Lock()
 

+ 1 - 1
service/internal/executor/executor_actions.go

@@ -146,7 +146,7 @@ func registerAction(e *Executor, configOrder int, action *config.Action, req *Re
 }
 
 func registerActionsFromEntities(e *Executor, configOrder int, entityTitle string, tpl *config.Action, req *RebuildActionMapRequest) {
-	for _, ent := range entities.GetEntityInstances(entityTitle) {
+	for _, ent := range entities.GetEntityInstancesOrdered(entityTitle) {
 		registerActionFromEntity(e, configOrder, tpl, ent, req)
 	}
 }

+ 52 - 0
specs/dashboard-component-ordering.md

@@ -0,0 +1,52 @@
+# Spec: Dashboard component ordering
+
+This spec describes how dashboard components (fieldsets, entity fieldsets, actions, and other elements) are ordered in OliveTin. It documents the current behaviour so that it can be reasoned about and kept consistent.
+
+---
+
+## 1. Implementation
+
+### 1.1 Two ways dashboards are built
+
+Dashboards are built in two ways:
+
+- **Default dashboard:** Used when there is no dashboard configuration. A single fieldset titled "Actions" is created and filled with actions that are not already on a configured dashboard.
+- **Config dashboard:** Built from the dashboard configuration (e.g. under dashboards or dashboards.d). The structure is derived by walking the config tree, which produces a mix of fieldsets and a special root fieldset titled "Actions" that holds any loose items.
+
+Ordering rules differ slightly between these two cases.
+
+### 1.2 Top-level dashboard contents (config dashboards)
+
+**Fieldsets without entities:** Fieldsets that are not tied to an entity type appear at the top level in **config order**. Their position in the dashboard matches the order in which they are defined in the config.
+
+**Other top-level components:** All other top-level components (including the root "Actions" fieldset and entity fieldset groups) are **sorted** before being shown. Sort order:
+
+1. If a component has no linked action, it is ordered by its title (alphabetically).
+2. Otherwise, components are ordered first by the action's order value (lower values first).
+3. If order values are equal, components are ordered by entity key: if both keys are whole numbers they are compared numerically; otherwise they are compared alphabetically.
+4. If still equal, components are ordered by the action's title (alphabetically).
+
+The root "Actions" fieldset is the single fieldset created by the build to hold loose items; it is identified by reference (not by position). When present it is added last to the list, then the sort is applied among that fieldset and entity-related components. So that fieldset can appear anywhere among those according to the rules above. When there are no loose items the root is not present, and the last component in the list is not treated as the root—so a fieldset without entities in the last position keeps config order. Regular fieldsets stay in config order and are not reordered.
+
+### 1.3 Entity fieldsets (order of fieldsets per entity type)
+
+When a fieldset in the config is tied to an entity type (e.g. "Server" or "Project"), one fieldset is built per entity instance. Those fieldsets are shown in **entity key order**.
+
+**Entity key order:**
+
+- If both keys are whole numbers: **numeric** order (e.g. 2 before 10).
+- Otherwise: **alphabetical** (lexicographic) order.
+
+So the order of entity fieldsets (e.g. one per server, one per project) is determined by this entity key order, not by config or insertion order.
+
+### 1.4 Contents inside fieldsets
+
+**Default dashboard:** The single "Actions" fieldset's contents are sorted. The same rules as for top-level components apply: order value first, then entity key (numeric then alphabetical), then action title.
+
+**Config dashboards:** For all fieldsets (the root "Actions" fieldset, entity fieldsets, and regular fieldsets), the contents are **not** sorted. They keep the order from the config:
+
+- **Root "Actions" fieldset:** Items appear in the order they are listed in the config (loose items that are not inside a fieldset).
+- **Entity fieldset contents:** The order comes from the template's contents in the config.
+- **Regular (non-entity) fieldset contents:** The order comes from the config, including for nested structure.
+
+So within any config-defined fieldset, the order of actions and other child components is the **config order**.