Repository navigation
Feat/clone sections - #265
Open
SepsiLaszlo wants to merge 12 commits into
Open
SepsiLaszlo wants to merge 12 commits into
SepsiLaszlo wants to merge 12 commits into
Conversation
In the editor's section blocks index (tree presentation), a nested block was rendered twice: once at the root level and once under its parent. Root cause: Maglev::Content::SectionContent#root_blocks selected blocks via the root? flag of their block *type definition* (which defaults to true when unspecified) instead of looking at the block's own parent_id. So whenever a block type is both root-eligible AND accepted as a child of another block (e.g. a navbar menu_item accepting nested menu_item blocks, a very common pattern for navbars with submenus), every nested block of that type also showed up in the root list. Besides the duplicated entry in the UI, this also generated duplicated DOM ids and mixed sortable scopes (root vs. parent), confusing drag & drop. Fix: select root blocks by parent_id (blank = root), mirroring how the front-end builds the block tree in Maglev::SectionComponent #build_block_tree. Notes: - Maglev::Section#root_blocks (definition level) is untouched: the root flag still drives which block types can be *added* at the root. - Bonus: #root_blocks no longer raises when a block's type has been removed from the theme (its definition is nil). Tests: - model spec: #root_blocks returns only parentless blocks and #child_blocks_of returns the direct children (navbar factory with nested menu items). - request spec: the navbar blocks index renders a nested menu item exactly once, nested under its parent's sortable scope (red before the fix, green after). - the theme factory navbar now uses blocks_presentation: 'tree' and menu_item accepts menu_item children (mirroring the dummy theme YAML), so the tree presentation is actually exercised in specs.
Editing a site scoped section (e.g. a navbar with a menu) always ended up showing the "someone else updated the content in the meantime" modal, even with a single editor. The very first save went through (both stores shared the same lock version at that point), and every subsequent save failed forever. Root cause: a site scoped section is listed in a layout store (its "slot", e.g. the header/main store) while its content lives in the global site scoped store (_site), which is the store the content services actually write to. The editor forms however submitted the lock version of the *slot* store (LockVersionConcern#source_lock_version, keyed off SectionContent#store_handle). Since site scoped writes only bump the _site store, the two versions diverge after the first save and every following save raises ActiveRecord::StaleObjectError. Fixes: 1. Forms now submit the section's own lock version (LockVersionConcern#source_lock_version): the sections content fetcher already stamps each section with the lock version of the store its content comes from — the _site store for site scoped sections. This is a no-op for regular sections (their stamped version is the one of their own store). 2. The :persisted turbo stream event now dispatches the fresh lock version of the store which was actually written (UpdateSectionService / UpdateSectionBlockService return it), so the auto-saving settings forms keep working on subsequent changes. Previously the (stale) slot store version was dispatched. 3. The preview now uses the same counter: Maglev::SectionComponent #lock_version reads the lock version stamped on the section attributes instead of the unrelated site/page *model* lock versions. As a result, the section:checkLockVersion mechanism only refreshes genuinely stale preview sections instead of always refreshing site scoped ones (the two counters could never match). Tests: - request specs: with the slot and _site stores holding different lock versions, the section and block forms of a site scoped section render the _site lock version, updating with it succeeds and dispatches the bumped version, and a stale submission is still rejected with a 409. - component spec: SectionComponent#lock_version uses the stamped attribute, including for a site scoped definition. - UpdateSectionService specs now assert the returned store (the passed store for regular sections, the site scoped one for site scoped sections).
Sorting the blocks of a section (drag & drop in the editor, or the sort endpoint) re-sorted the whole flat blocks list with Array#sort_by!, giving every block outside of the sorted scope the same key (-1). Two problems with that: - Array#sort_by! is not stable, so the blocks sharing the -1 key (the root blocks when sorting the children of a menu item, or all the children when sorting the root blocks) could be shuffled. Whether it actually happens depends on the platform's qsort_r, which is why the bug shows up on some machines only (macOS, glibc 2.37/2.38, ...). - even with a stable sort, all the non-sibling blocks were moved before the sorted siblings, so the stored order churned on every sort. Fix: only re-order the siblings (the blocks sharing the requested parent_id), in place, with a stable sort. The other blocks keep the exact position they had in the list. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
fix(editor): keep the other blocks in place when sorting tree section blocks
Sorting the blocks of a section (drag & drop in the editor, or the sort endpoint) re-sorted the whole flat blocks list with Array#sort_by!, giving every block outside of the sorted scope the same key (-1). Two problems with that: - Array#sort_by! is not stable, so the blocks sharing the -1 key (the root blocks when sorting the children of a menu item, or all the children when sorting the root blocks) could be shuffled. Whether it actually happens depends on the platform's qsort_r, which is why the bug shows up on some machines only (macOS, glibc 2.37/2.38, ...). - even with a stable sort, all the non-sibling blocks were moved before the sorted siblings, so the stored order churned on every sort. Fix: only re-order the siblings (the blocks sharing the requested parent_id), in place, with a stable sort. The other blocks keep the exact position they had in the list. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…locking Restore the three fixes and their regression coverage removed by 20ca650. The resulting tree matches 968a334 and retains all current upstream main code. Validation: full RSpec suite on SQLite (483 examples, 0 failures), RuboCop on all 15 changed Ruby files (no offenses), and git diff --check.
…r resizable A text setting with `html: true` rendered the Tiptap editor at the uikit default of 4 rows no matter what `nb_rows` said, because the editor setting component never passed it through. Pass it as `rows` so a section can ask for a taller editor, and add `resize-y` to `.ProseMirror` so the author can drag the box taller still. The built tailwind.css is regenerated with `rake app:maglev:tailwindcss:build`; it was stale and also picks up a few utilities already used by components. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Honour nb_rows on rich text settings and make the editor resizable
GetPageFullpath#build_fullpath joined the base url, the locale prefix and
the page path blindly, so a path stored with a leading slash
("/hello-world"), a base url ending with a slash or a static page path
declared as "/products" produced hrefs like "https://host//hello-world".
- GetPageFullpath.join strips leading/trailing slashes from every segment,
collapses inner "//" and always yields a single leading "/" when there
is no base url (the scheme's "//" is kept).
- The static page branch of TransformLinkConcern uses the same
normalisation (absolute urls are left untouched).
- PagePath#clean_value now strips every leading/trailing slash (it only
removed one) and collapses any run of slashes (it only collapsed 3+).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
fix: never emit a double slash in page fullpaths
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.