fix(17): WR-01 keep a menu entry's subtree and parent in its menu

- The admin refuses a parent from another menu than the one the form
  names (errors.parent_menu) instead of moving the entry to the parent's
  menu without a word; BeforeSave records the named menu
  (MenuEntry.RequestedMenuID).
- An entry moved to another menu moves its descendants in the same
  transaction, so the public tree no longer drops them.

README.md

@@ -17,7 +17,7 @@ It requires no other plugin. Content types (blog posts, categories, pages) come
 ## Features
 
 - **Tables.** `golem15_menu_menus` (id, name, code unique, is_active default true, css_class, timestamps) and `golem15_menu_menu_entries` (id, menu_id, parent_id, nest_left, nest_right, nest_depth, sort_order, type, title, url, reference, code, icon, css_class, target, is_published default true, timestamps) keep the PHP columns. One additive nullable column, `url_pattern`, holds the per-entry URL pattern override of a registry entry. Three gormigrate steps create them; AutoMigrate is not used.
-- **Save rules.** `models.MenuEntry.BeforeSave` ports the PHP `beforeSave`: the nest columns become 0 when unset, an entry without `sort_order` goes after the menu's last entry, an entry with a parent always takes the parent's menu, a missing title is stored empty and a missing published flag is true. The nest columns are kept for the PHP schema; the tree is `parent_id` plus `sort_order`.
+- **Save rules.** `models.MenuEntry.BeforeSave` ports the PHP `beforeSave`: the nest columns become 0 when unset, an entry without `sort_order` goes after the menu's last entry, an entry with a parent always takes the parent's menu (`models.MenuEntry.RequestedMenuID` then reports the other menu the save named, if any), a missing title is stored empty and a missing published flag is true. The nest columns are kept for the PHP schema; the tree is `parent_id` plus `sort_order`.
 - **Entry types.** The built-ins `url` (Link, `models.TypeURL`) and `divider` (Divider, `models.TypeDivider`), then every registry type with its owning plugin's label. The reference dropdown depends on the type through `signpost.ReferenceOptions`; built-ins take no reference. The admin save refuses an unknown type, a reference the type does not offer, a URL outside the scheme allowlist and a target other than `_self`, `_blank` or empty.
 - **URL allowlist.** `classes.AllowedURL` accepts an empty URL, a path starting with `/`, a fragment starting with `#`, and `http:`, `https:`, `mailto:` and `tel:` URLs. Anything else (`javascript:`, `data:`, a URL with surrounding whitespace or control characters) is a 422 `errors.url_scheme` on save and a `null` URL in the API.
 - **Admin.** A top-level Menus item (icon `list`, order 510) with the side items Menus and Menu entries, both behind the permission `golem15.menu.manage_menu` (developer and publisher roles). Menus: name, code (preset from name), css_class, is_active. Menu entries: a list of every entry with a menu filter and the toolbar buttons reorder, create and delete; a form with menu, parent, type, title, url, reference, url_pattern, code, icon, css_class, target and is_published; a tree reorder page scoped to one menu (`config_reorder.yaml`, partition `menu_id`), which refuses a payload naming an entry of another menu with 409.
@@ -156,6 +156,7 @@ cors:
 | `menu.BucketPublicAPI` | `menu-public-api`, the throttle bucket of the public API. |
 | `models.Menu` | A menu on `golem15_menu_menus`; rules require a name and a unique code. |
 | `models.MenuEntry` | An entry on `golem15_menu_menu_entries` with the PHP save rules (`BeforeSave`) and the menu filter scope. |
+| `models.MenuEntry.RequestedMenuID` | The menu the last save named when `BeforeSave` replaced it with the parent's menu; 0 otherwise. |
 | `models.TypeURL`, `models.TypeDivider` | The built-in entry types `url` and `divider`. |
 | `models.Builtin` | Reports whether a type is a built-in. |
 | `models.Text` | The value of an optional text column, `""` when null. |
@@ -187,7 +188,7 @@ cors:
 - Menu entries list: PHP listed the top-level entries of one menu chosen in a toolbar select, with a Children tab and partial in the form. Here the list shows every entry, a menu filter picks the menu, and the reorder page edits the tree per menu; the Children tab is not ported.
 - Entry form: the menu field is a relation field (also the reorder partition); `url_pattern` (per-entry pattern override) and `target` (`_self`, `_blank`) are new fields. The labels "Menuses" and "Is_published" read Menus and Published.
 - Type keys: PHP `link` is `url`; PHP `blog_categories` is the registry type `all-journal-categories` when a blog plugin registers it; `page` is not supported.
-- Saves: an entry with a parent always takes the parent's menu (PHP only when the menu was empty); an unknown type, a parent cycle, a disallowed URL or target and an entry without a menu are refused with 422; a menu code another menu uses is a 422.
+- Saves: an entry with a parent always takes the parent's menu (PHP only when the menu was empty); the admin form refuses with 422 a parent from another menu than the one the form names (`errors.parent_menu`), and an entry moved to another menu takes its whole subtree along, so no descendant is left with a parent in another menu. An unknown type, a parent cycle, a disallowed URL or target and an entry without a menu are refused with 422; a menu code another menu uses is a 422.
 - API: the public JSON API replaces the PHP component and filters unpublished entries.
 - Timestamps are `TIMESTAMPTZ`.
 

controllers/menuentries.go

@@ -24,6 +24,7 @@ const (
 	errTypeUnknown  = "golem15.menu::lang.errors.type_unknown"
 	errMenuRequired = "golem15.menu::lang.errors.menu_required"
 	errParentCycle  = "golem15.menu::lang.errors.parent_cycle"
+	errParentMenu   = "golem15.menu::lang.errors.parent_menu"
 	labelTypeURL    = "golem15.menu::lang.models.menuentry.type_url"
 	labelTypeDivide = "golem15.menu::lang.models.menuentry.type_divider"
 )
@@ -183,11 +184,33 @@ func (menuEntriesAdminController) FormAfterCreate(ctx context.Context, model any
 }
 
 // FormAfterUpdate also refuses a parent that is the entry itself or one of
-// its descendants.
+// its descendants, then moves the entry's descendants into its menu, so a
+// subtree follows its root to another menu instead of staying behind with
+// parents in a menu it is not in.
 func (menuEntriesAdminController) FormAfterUpdate(ctx context.Context, model any) error {
-	return checkPlacement(ctx, model)
+	if err := checkPlacement(ctx, model); err != nil {
+		return err
+	}
+	entry, ok := model.(*models.MenuEntry)
+	if !ok || entry == nil || entry.ID == 0 {
+		return nil
+	}
+	tx, ok := cabana.TxFromContext(ctx)
+	if !ok {
+		return errors.New("golem15.menu: the entry check needs the save's transaction")
+	}
+	return tx.Session(&gorm.Session{NewDB: true}).WithContext(ctx).Exec(`WITH RECURSIVE sub(id) AS (
+	SELECT id FROM golem15_menu_menu_entries WHERE parent_id = ? AND id <> ?
+	UNION
+	SELECT e.id FROM golem15_menu_menu_entries e JOIN sub ON e.parent_id = sub.id
+)
+UPDATE golem15_menu_menu_entries SET menu_id = ? WHERE id IN (SELECT id FROM sub) AND id <> ? AND menu_id <> ?`,
+		entry.ID, entry.ID, entry.MenuID, entry.ID, entry.MenuID).Error
 }
 
+// checkPlacement refuses an entry in no menu, a parent from another menu
+// than the one the entry names (BeforeSave would otherwise move the entry to
+// the parent's menu without a word) and a parent cycle.
 func checkPlacement(ctx context.Context, model any) error {
 	entry, ok := model.(*models.MenuEntry)
 	if !ok || entry == nil {
@@ -196,6 +219,9 @@ func checkPlacement(ctx context.Context, model any) error {
 	if entry.MenuID == 0 {
 		return &cabana.ValidationError{Details: map[string]any{"menu": []string{errMenuRequired}}}
 	}
+	if entry.RequestedMenuID() != 0 {
+		return &cabana.ValidationError{Details: map[string]any{"parent": []string{errParentMenu}}}
+	}
 	if entry.ParentID == nil {
 		return nil
 	}

lang/en/lang.yaml

@@ -19,7 +19,7 @@ models:
     label: Menu entry
     label_plural: Menu entries
     menu: Menu
-    menu_comment: Entries with a parent belong to the parent's menu.
+    menu_comment: Entries with a parent belong to the parent's menu; leave it empty to take the parent's. Moving an entry moves its sub-entries with it.
     parent: Parent
     type: Type
     type_url: Link
@@ -45,3 +45,4 @@ errors:
   type_unknown: Choose one of the listed types.
   menu_required: Choose a menu or a parent entry.
   parent_cycle: An entry cannot be placed under itself or one of its own entries.
+  parent_menu: The parent entry belongs to another menu. Choose a parent from the selected menu, or change the menu.

lang/pl/lang.yaml

@@ -19,7 +19,7 @@ models:
     label: Pozycja menu
     label_plural: Pozycje menu
     menu: Menu
-    menu_comment: Pozycje z pozycją nadrzędną należą do jej menu.
+    menu_comment: Pozycje z pozycją nadrzędną należą do jej menu; zostaw puste, aby przejąć menu pozycji nadrzędnej. Przeniesienie pozycji przenosi też jej podpozycje.
     parent: Pozycja nadrzędna
     type: Typ
     type_url: Link
@@ -45,3 +45,4 @@ errors:
   type_unknown: Wybierz jeden z typów z listy.
   menu_required: Wybierz menu albo pozycję nadrzędną.
   parent_cycle: Pozycji nie można umieścić pod nią samą ani pod jej własnymi pozycjami.
+  parent_menu: Pozycja nadrzędna należy do innego menu. Wybierz pozycję nadrzędną z wybranego menu albo zmień menu.

menu_smoke_test.go

@@ -378,6 +378,25 @@ func TestMenuAdminSmoke(t *testing.T) {
 		t.Fatalf("reorder with a foreign entry: %d %s", rec.Code, rec.Body.String())
 	}
 
+	// WR-01: a parent from another menu than the one the entry names is
+	// refused, and an entry moved to another menu takes its subtree along.
+	e.expect422(t, token, http.MethodPut, fmt.Sprintf("%s/%d", entriesPath, divider), map[string]any{"menu": mainID, "parent": foreign}, "parent", "golem15.menu::lang.errors.parent_menu")
+	if dv := e.entry(t, divider); dv.MenuID != mainID || dv.ParentID != nil {
+		t.Fatalf("a refused move changed the divider: %+v", dv)
+	}
+	rec = e.call(t, http.MethodPut, fmt.Sprintf("%s/%d", entriesPath, home), token, map[string]any{"menu": footerID})
+	if rec.Code != http.StatusOK {
+		t.Fatalf("move Home to the footer: %d %s", rec.Code, rec.Body.String())
+	}
+	for _, id := range []uint{home, child, page} {
+		if got := e.entry(t, id); got.MenuID != footerID {
+			t.Fatalf("entry %d stayed in menu %d after its root moved to %d", id, got.MenuID, footerID)
+		}
+	}
+	if dv := e.entry(t, divider); dv.MenuID != mainID {
+		t.Fatalf("the divider moved with Home: %+v", dv)
+	}
+
 	// Without the permission the screens answer 403.
 	other := e.admin(t)
 	for _, path := range []string{menusPath, entriesPath} {

models/menu_entry.go

@@ -48,6 +48,10 @@ type MenuEntry struct {
 	IsPublished *bool      `gorm:"column:is_published"`
 	CreatedAt   *time.Time `gorm:"column:created_at"`
 	UpdatedAt   *time.Time `gorm:"column:updated_at"`
+
+	// requestedMenuID is the menu a save asked for when BeforeSave replaced
+	// it with the parent's menu (see RequestedMenuID).
+	requestedMenuID uint
 }
 
 func (MenuEntry) TableName() string { return "golem15_menu_menu_entries" }
@@ -71,7 +75,10 @@ func (MenuEntry) Rules() map[string]string {
 // an entry with a parent takes the parent's menu, and an entry without a
 // sort_order goes after the menu's last one. A missing title is stored empty
 // (dividers have none) and a missing published flag is the PHP default true.
+// When the entry named another menu than its parent's, RequestedMenuID
+// reports the menu it named; the admin refuses such a save.
 func (e *MenuEntry) BeforeSave(tx *gorm.DB) error {
+	e.requestedMenuID = 0
 	zero := func(p **int) {
 		if *p == nil {
 			v := 0
@@ -96,6 +103,9 @@ func (e *MenuEntry) BeforeSave(tx *gorm.DB) error {
 			return err
 		}
 		if len(menuIDs) > 0 && menuIDs[0] != 0 {
+			if e.MenuID != 0 && e.MenuID != menuIDs[0] {
+				e.requestedMenuID = e.MenuID
+			}
 			e.MenuID = menuIDs[0]
 		}
 	}
@@ -113,6 +123,11 @@ func (e *MenuEntry) BeforeSave(tx *gorm.DB) error {
 	return nil
 }
 
+// RequestedMenuID is the menu the last save of e named when BeforeSave
+// replaced it with the parent's menu: the parent belongs to another menu.
+// It is 0 when the entry named no menu or its parent's.
+func (e *MenuEntry) RequestedMenuID() uint { return e.requestedMenuID }
+
 // Text is the value of an optional text column, "" when null.
 func Text(s *string) string {
 	if s == nil {