diff --git a/README.md b/README.md index 4cbae82..2d423c5 100644 --- a/README.md +++ b/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`. diff --git a/controllers/menuentries.go b/controllers/menuentries.go index 301eb10..74eb363 100644 --- a/controllers/menuentries.go +++ b/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 } diff --git a/lang/en/lang.yaml b/lang/en/lang.yaml index e11301f..771d4ef 100644 --- a/lang/en/lang.yaml +++ b/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. diff --git a/lang/pl/lang.yaml b/lang/pl/lang.yaml index a9ce5cf..35b8f56 100644 --- a/lang/pl/lang.yaml +++ b/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. diff --git a/menu_smoke_test.go b/menu_smoke_test.go index 3876b9b..af9e286 100644 --- a/menu_smoke_test.go +++ b/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} { diff --git a/models/menu_entry.go b/models/menu_entry.go index 4312f83..fdeebb9 100644 --- a/models/menu_entry.go +++ b/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 {