From 5f494b8aa82b6a89b12da6d444f22a150bc885ec Mon Sep 17 00:00:00 2001 From: Joakim Date: Mon, 22 Sep 2025 21:50:18 +0200 Subject: [PATCH] Fix critical collection persistence bug where initial items disappeared after first enhancement by implementing database-first pattern with template-based reconstruction --- internal/content/client.go | 8 ++ internal/content/mock.go | 8 ++ internal/engine/database_client.go | 106 ++++++++++++++ internal/engine/engine.go | 215 +++++++++++++++++++++++++---- internal/engine/types.go | 2 + 5 files changed, 312 insertions(+), 27 deletions(-) diff --git a/internal/content/client.go b/internal/content/client.go index 141a30e..cd088f6 100644 --- 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") +} diff --git a/internal/content/mock.go b/internal/content/mock.go index ec65534..c4b41be 100644 --- 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") +} diff --git a/internal/engine/database_client.go b/internal/engine/database_client.go index 9ec87ac..6113c36 100644 --- 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()) + } +} diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 0617a84..0b359f9 100644 --- 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 +} diff --git a/internal/engine/types.go b/internal/engine/types.go index 1cc017c..9e2eae1 100644 --- 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