Fix critical collection persistence bug where initial items disappeared after first enhancement by implementing database-first pattern with template-based reconstruction

2025-09-22 21:50:18 +02:00
parent 09823d3e4d
commit 5f494b8aa8
5 changed files with 312 additions and 27 deletions
--- a/internal/content/client.go
+++ b/internal/content/client.go
@@ -188,3 +188,11 @@ func (c *HTTPClient) GetCollectionItems(siteID, collectionID string) ([]engine.C
 func (c *HTTPClient) CreateCollectionTemplate(siteID, collectionID, name, htmlTemplate string, isDefault bool) (*engine.CollectionTemplateItem, error) {
 	return nil, fmt.Errorf("collection operations not implemented in HTTPClient")
 }
+
+func (c *HTTPClient) GetCollectionTemplates(siteID, collectionID string) ([]engine.CollectionTemplateItem, error) {
+	return nil, fmt.Errorf("collection operations not implemented in HTTPClient")
+}
+
+func (c *HTTPClient) CreateCollectionItem(siteID, collectionID, itemID string, templateID int, htmlContent string, position int, lastEditedBy string) (*engine.CollectionItemWithTemplate, error) {
+	return nil, fmt.Errorf("collection operations not implemented in HTTPClient")
+}
--- a/internal/content/mock.go
+++ b/internal/content/mock.go
@@ -174,3 +174,11 @@ func (m *MockClient) GetCollectionItems(siteID, collectionID string) ([]engine.C
 func (m *MockClient) CreateCollectionTemplate(siteID, collectionID, name, htmlTemplate string, isDefault bool) (*engine.CollectionTemplateItem, error) {
 	return nil, fmt.Errorf("collection operations not implemented in MockClient")
 }
+
+func (m *MockClient) GetCollectionTemplates(siteID, collectionID string) ([]engine.CollectionTemplateItem, error) {
+	return nil, fmt.Errorf("collection operations not implemented in MockClient")
+}
+
+func (m *MockClient) CreateCollectionItem(siteID, collectionID, itemID string, templateID int, htmlContent string, position int, lastEditedBy string) (*engine.CollectionItemWithTemplate, error) {
+	return nil, fmt.Errorf("collection operations not implemented in MockClient")
+}
--- a/internal/engine/database_client.go
+++ b/internal/engine/database_client.go
@@ -410,3 +410,109 @@ func (c *DatabaseClient) CreateCollectionTemplate(siteID, collectionID, name, ht
 		return nil, fmt.Errorf("unsupported database type: %s", c.database.GetDBType())
 	}
 }
+
+// GetCollectionTemplates retrieves all templates for a collection
+func (c *DatabaseClient) GetCollectionTemplates(siteID, collectionID string) ([]CollectionTemplateItem, error) {
+	switch c.database.GetDBType() {
+	case "sqlite3":
+		templates, err := c.database.GetSQLiteQueries().GetCollectionTemplates(context.Background(), sqlite.GetCollectionTemplatesParams{
+			CollectionID: collectionID,
+			SiteID:       siteID,
+		})
+		if err != nil {
+			return nil, err
+		}
+
+		result := make([]CollectionTemplateItem, len(templates))
+		for i, template := range templates {
+			result[i] = CollectionTemplateItem{
+				TemplateID:   int(template.TemplateID),
+				CollectionID: template.CollectionID,
+				SiteID:       template.SiteID,
+				Name:         template.Name,
+				HTMLTemplate: template.HtmlTemplate,
+				IsDefault:    template.IsDefault != 0, // SQLite uses INTEGER for boolean
+			}
+		}
+		return result, nil
+
+	case "postgresql":
+		templates, err := c.database.GetPostgreSQLQueries().GetCollectionTemplates(context.Background(), postgresql.GetCollectionTemplatesParams{
+			CollectionID: collectionID,
+			SiteID:       siteID,
+		})
+		if err != nil {
+			return nil, err
+		}
+
+		result := make([]CollectionTemplateItem, len(templates))
+		for i, template := range templates {
+			result[i] = CollectionTemplateItem{
+				TemplateID:   int(template.TemplateID),
+				CollectionID: template.CollectionID,
+				SiteID:       template.SiteID,
+				Name:         template.Name,
+				HTMLTemplate: template.HtmlTemplate,
+				IsDefault:    template.IsDefault, // PostgreSQL uses BOOLEAN
+			}
+		}
+		return result, nil
+
+	default:
+		return nil, fmt.Errorf("unsupported database type: %s", c.database.GetDBType())
+	}
+}
+
+// CreateCollectionItem creates a new collection item
+func (c *DatabaseClient) CreateCollectionItem(siteID, collectionID, itemID string, templateID int, htmlContent string, position int, lastEditedBy string) (*CollectionItemWithTemplate, error) {
+	switch c.database.GetDBType() {
+	case "sqlite3":
+		item, err := c.database.GetSQLiteQueries().CreateCollectionItem(context.Background(), sqlite.CreateCollectionItemParams{
+			ItemID:       itemID,
+			CollectionID: collectionID,
+			SiteID:       siteID,
+			TemplateID:   int64(templateID),
+			HtmlContent:  htmlContent,
+			Position:     int64(position),
+			LastEditedBy: lastEditedBy,
+		})
+		if err != nil {
+			return nil, err
+		}
+		return &CollectionItemWithTemplate{
+			ItemID:       item.ItemID,
+			CollectionID: item.CollectionID,
+			SiteID:       item.SiteID,
+			TemplateID:   int(item.TemplateID),
+			HTMLContent:  item.HtmlContent,
+			Position:     int(item.Position),
+			LastEditedBy: item.LastEditedBy,
+		}, nil
+
+	case "postgresql":
+		item, err := c.database.GetPostgreSQLQueries().CreateCollectionItem(context.Background(), postgresql.CreateCollectionItemParams{
+			ItemID:       itemID,
+			CollectionID: collectionID,
+			SiteID:       siteID,
+			TemplateID:   int32(templateID),
+			HtmlContent:  htmlContent,
+			Position:     int32(position),
+			LastEditedBy: lastEditedBy,
+		})
+		if err != nil {
+			return nil, err
+		}
+		return &CollectionItemWithTemplate{
+			ItemID:       item.ItemID,
+			CollectionID: item.CollectionID,
+			SiteID:       item.SiteID,
+			TemplateID:   int(item.TemplateID),
+			HTMLContent:  item.HtmlContent,
+			Position:     int(item.Position),
+			LastEditedBy: item.LastEditedBy,
+		}, nil
+
+	default:
+		return nil, fmt.Errorf("unsupported database type: %s", c.database.GetDBType())
+	}
+}
--- a/internal/engine/engine.go
+++ b/internal/engine/engine.go
@@ -409,28 +409,42 @@ func (e *ContentEngine) processCollection(collectionNode *html.Node, collectionI
 			return fmt.Errorf("failed to create collection %s: %w", collectionID, err)
 		}

-		// 3. Extract templates from existing children
-		err = e.extractAndStoreTemplates(collectionNode, collectionID, siteID)
+		// 3. Extract templates and store initial items from existing children
+		err = e.extractAndStoreTemplatesAndItems(collectionNode, collectionID, siteID)
 		if err != nil {
-			return fmt.Errorf("failed to extract templates for collection %s: %w", collectionID, err)
+			return fmt.Errorf("failed to extract templates and items for collection %s: %w", collectionID, err)
 		}

-		fmt.Printf("✅ Created new collection: %s with templates\n", collectionID)
+		fmt.Printf("✅ Created new collection: %s with templates and initial items\n", collectionID)
 	} else {
-		// 4. Existing collection: reconstruct items from database
-		err = e.reconstructCollectionItems(collectionNode, collectionID, siteID)
+		// 4. Database-first approach: Check if collection items already exist
+		existingItems, err := e.client.GetCollectionItems(siteID, collectionID)
 		if err != nil {
-			return fmt.Errorf("failed to reconstruct collection %s: %w", collectionID, err)
+			return fmt.Errorf("failed to check existing collection items: %w", err)
 		}

-		fmt.Printf("✅ Reconstructed collection: %s from database\n", collectionID)
+		if len(existingItems) == 0 {
+			// 5. Collection exists but no items - store original children as initial items
+			err = e.storeInitialCollectionItems(collectionNode, collectionID, siteID)
+			if err != nil {
+				return fmt.Errorf("failed to store initial collection items for %s: %w", collectionID, err)
+			}
+			fmt.Printf("✅ Stored initial items for existing collection: %s\n", collectionID)
+		} else {
+			// 6. Items exist: reconstruct from database (normal case)
+			err = e.reconstructCollectionItems(collectionNode, collectionID, siteID)
+			if err != nil {
+				return fmt.Errorf("failed to reconstruct collection %s: %w", collectionID, err)
+			}
+			fmt.Printf("✅ Reconstructed collection: %s from database (%d items)\n", collectionID, len(existingItems))
+		}
 	}

 	return nil
 }

-// extractAndStoreTemplates extracts template patterns from existing collection children
-func (e *ContentEngine) extractAndStoreTemplates(collectionNode *html.Node, collectionID, siteID string) error {
+// extractAndStoreTemplatesAndItems extracts templates and stores initial items from existing collection children
+func (e *ContentEngine) extractAndStoreTemplatesAndItems(collectionNode *html.Node, collectionID, siteID string) error {
 	// Find existing children elements to use as templates
 	var templateElements []*html.Node

@@ -451,20 +465,28 @@ func (e *ContentEngine) extractAndStoreTemplates(collectionNode *html.Node, coll
 		return nil
 	}

-	// Extract templates from existing children
+	// Extract templates from existing children and store them
+	var templateIDs []int
 	for i, templateElement := range templateElements {
 		templateHTML := e.extractOriginalTemplate(templateElement)
 		templateName := fmt.Sprintf("template-%d", i+1)
 		isDefault := (i == 0) // First template is default

-		_, err := e.client.CreateCollectionTemplate(siteID, collectionID, templateName, templateHTML, isDefault)
+		template, err := e.client.CreateCollectionTemplate(siteID, collectionID, templateName, templateHTML, isDefault)
 		if err != nil {
 			return fmt.Errorf("failed to create template %s: %w", templateName, err)
 		}

+		templateIDs = append(templateIDs, template.TemplateID)
 		fmt.Printf("✅ Created template '%s' for collection %s\n", templateName, collectionID)
 	}

+	// Store original children as initial collection items (database-first approach)
+	err := e.storeChildrenAsCollectionItems(collectionNode, collectionID, siteID, templateIDs)
+	if err != nil {
+		return fmt.Errorf("failed to store initial collection items: %w", err)
+	}
+
 	return nil
 }

@@ -476,6 +498,18 @@ func (e *ContentEngine) reconstructCollectionItems(collectionNode *html.Node, co
 		return fmt.Errorf("failed to get collection items: %w", err)
 	}

+	// Get templates for this collection
+	templates, err := e.client.GetCollectionTemplates(siteID, collectionID)
+	if err != nil {
+		return fmt.Errorf("failed to get collection templates: %w", err)
+	}
+
+	// Create a template map for quick lookup
+	templateMap := make(map[int]string)
+	for _, template := range templates {
+		templateMap[template.TemplateID] = template.HTMLTemplate
+	}
+
 	// Clear existing children (they will be replaced with database items)
 	for child := collectionNode.FirstChild; child != nil; {
 		next := child.NextSibling
@@ -485,32 +519,159 @@ func (e *ContentEngine) reconstructCollectionItems(collectionNode *html.Node, co

 	// Add items from database in position order
 	for _, item := range items {
-		// Parse the item HTML content
-		itemDoc, err := html.Parse(strings.NewReader(item.HTMLContent))
-		if err != nil {
-			fmt.Printf("⚠️ Failed to parse item HTML for %s: %v\n", item.ItemID, err)
+		// Get the template for this item
+		templateHTML, exists := templateMap[item.TemplateID]
+		if !exists {
+			fmt.Printf("⚠️ Template %d not found for item %s\n", item.TemplateID, item.ItemID)
 			continue
 		}

-		// Find the body element and extract its children
-		var bodyNode *html.Node
-		e.walkNodes(itemDoc, func(n *html.Node) {
+		// Parse the template HTML
+		templateDoc, err := html.Parse(strings.NewReader(templateHTML))
+		if err != nil {
+			fmt.Printf("⚠️ Failed to parse template HTML for %s: %v\n", item.ItemID, err)
+			continue
+		}
+
+		// Find the body element and extract the template structure
+		var templateBody *html.Node
+		e.walkNodes(templateDoc, func(n *html.Node) {
 			if n.Type == html.ElementNode && n.Data == "body" {
-				bodyNode = n
+				templateBody = n
 			}
 		})

-		if bodyNode != nil {
-			// Move all children from body to collection
-			for bodyChild := bodyNode.FirstChild; bodyChild != nil; {
-				next := bodyChild.NextSibling
-				bodyNode.RemoveChild(bodyChild)
-				collectionNode.AppendChild(bodyChild)
-				bodyChild = next
+		if templateBody != nil && templateBody.FirstChild != nil {
+			// Clone the template structure (first child of body)
+			templateNode := templateBody.FirstChild
+			clonedTemplate := e.cloneNode(templateNode)
+
+			// Replace the template's inner content with the stored item content
+			// Clear the cloned template's children
+			for child := clonedTemplate.FirstChild; child != nil; {
+				next := child.NextSibling
+				clonedTemplate.RemoveChild(child)
+				child = next
 			}
+
+			// Parse and add the item's content
+			itemDoc, err := html.Parse(strings.NewReader(item.HTMLContent))
+			if err != nil {
+				fmt.Printf("⚠️ Failed to parse item content for %s: %v\n", item.ItemID, err)
+				continue
+			}
+
+			var itemBody *html.Node
+			e.walkNodes(itemDoc, func(n *html.Node) {
+				if n.Type == html.ElementNode && n.Data == "body" {
+					itemBody = n
+				}
+			})
+
+			if itemBody != nil {
+				// Move all children from item body to cloned template
+				for itemChild := itemBody.FirstChild; itemChild != nil; {
+					next := itemChild.NextSibling
+					itemBody.RemoveChild(itemChild)
+					clonedTemplate.AppendChild(itemChild)
+					itemChild = next
+				}
+			}
+
+			// Add the reconstructed item to collection
+			collectionNode.AppendChild(clonedTemplate)
 		}
 	}

 	fmt.Printf("✅ Reconstructed %d items for collection %s\n", len(items), collectionID)
 	return nil
 }
+
+// cloneNode creates a deep copy of an HTML node
+func (e *ContentEngine) cloneNode(node *html.Node) *html.Node {
+	cloned := &html.Node{
+		Type:      node.Type,
+		Data:      node.Data,
+		DataAtom:  node.DataAtom,
+		Namespace: node.Namespace,
+	}
+
+	// Clone attributes
+	for _, attr := range node.Attr {
+		cloned.Attr = append(cloned.Attr, html.Attribute{
+			Namespace: attr.Namespace,
+			Key:       attr.Key,
+			Val:       attr.Val,
+		})
+	}
+
+	// Clone children recursively
+	for child := node.FirstChild; child != nil; child = child.NextSibling {
+		clonedChild := e.cloneNode(child)
+		cloned.AppendChild(clonedChild)
+	}
+
+	return cloned
+}
+
+// storeInitialCollectionItems stores original children as collection items (for existing collections)
+func (e *ContentEngine) storeInitialCollectionItems(collectionNode *html.Node, collectionID, siteID string) error {
+	// Get existing templates for this collection
+	templates, err := e.client.GetCollectionTemplates(siteID, collectionID)
+	if err != nil {
+		return fmt.Errorf("failed to get collection templates: %w", err)
+	}
+
+	if len(templates) == 0 {
+		fmt.Printf("⚠️ No templates found for collection %s, skipping initial items storage\n", collectionID)
+		return nil
+	}
+
+	// Use template IDs from existing templates
+	var templateIDs []int
+	for _, template := range templates {
+		templateIDs = append(templateIDs, template.TemplateID)
+	}
+
+	return e.storeChildrenAsCollectionItems(collectionNode, collectionID, siteID, templateIDs)
+}
+
+// storeChildrenAsCollectionItems stores HTML children as collection items in database
+func (e *ContentEngine) storeChildrenAsCollectionItems(collectionNode *html.Node, collectionID, siteID string, templateIDs []int) error {
+	// Find existing children elements to store as items
+	var childElements []*html.Node
+
+	// Walk through direct children of the collection
+	for child := collectionNode.FirstChild; child != nil; child = child.NextSibling {
+		if child.Type == html.ElementNode {
+			childElements = append(childElements, child)
+		}
+	}
+
+	if len(childElements) == 0 {
+		fmt.Printf("ℹ️ No children found to store as collection items for %s\n", collectionID)
+		return nil
+	}
+
+	// Store each child as a collection item (database-first pattern like .insertr)
+	for i, childElement := range childElements {
+		// Generate item ID (like content ID generation)
+		itemID := fmt.Sprintf("%s-initial-%d", collectionID, i+1)
+
+		// Extract HTML content from the child (reuse .insertr pattern)
+		htmlContent := e.extractHTMLContent(childElement)
+
+		// Use appropriate template ID (cycle through available templates)
+		templateID := templateIDs[i%len(templateIDs)]
+
+		// Store as collection item
+		_, err := e.client.CreateCollectionItem(siteID, collectionID, itemID, templateID, htmlContent, i+1, "system")
+		if err != nil {
+			return fmt.Errorf("failed to create collection item %s: %w", itemID, err)
+		}
+
+		fmt.Printf("✅ Stored initial collection item: %s (template %d)\n", itemID, templateID)
+	}
+
+	return nil
+}
--- a/internal/engine/types.go
+++ b/internal/engine/types.go
@@ -53,7 +53,9 @@ type ContentClient interface {
 	GetCollection(siteID, collectionID string) (*CollectionItem, error)
 	CreateCollection(siteID, collectionID, containerHTML, lastEditedBy string) (*CollectionItem, error)
 	GetCollectionItems(siteID, collectionID string) ([]CollectionItemWithTemplate, error)
+	GetCollectionTemplates(siteID, collectionID string) ([]CollectionTemplateItem, error)
 	CreateCollectionTemplate(siteID, collectionID, name, htmlTemplate string, isDefault bool) (*CollectionTemplateItem, error)
+	CreateCollectionItem(siteID, collectionID, itemID string, templateID int, htmlContent string, position int, lastEditedBy string) (*CollectionItemWithTemplate, error)
 }

 // ContentItem represents a piece of content from the database