area17 / twill

Twill is an open source CMS toolkit for Laravel that helps developers rapidly create a custom admin console that is intuitive, powerful and flexible. Chat with us on Discord at https://discord.gg/cnWk7EFv8R.
https://twillcms.com
Apache License 2.0
3.72k stars 568 forks source link

Fix: nested repeaters #2490

Open Tofandel opened 6 months ago

Tofandel commented 6 months ago

Fixes #1060

This allows Repeaters and InlineRepeaters to be nested to infinity (or until memory is exhausted at least, since they are memory hungry)

ifox commented 6 months ago

Hi @Tofandel, thank you so much for working on this!

The only thing I'm wondering about is whether or not this change may have a breaking impact on existing revisions.

Tofandel commented 6 months ago

I am currently blocking on the fact that block editors inside nested repeaters don't play well they seem to be saving as an array inside of blocks[blockName][] instead of block[]

As for breaking change, given that previously nested repeaters had no way to work (only nested blocks where working) I don't think there would be one, I'm not familiar with how revisions are created is it directly from the frontend field list and not the dirty attributes of the model?

Tofandel commented 6 months ago

Example image

I'm trying to see what's causing this because the backend cannot handle it

If you have any idea before I spend 2 hours debugging, it's welcome (in vue they are saved completely normally, but only when it comes to clicking on save then the blocks are sent like this)

Okay I think I've found the cause, this is how the are normally saved, they are not supposed to be keyed

image

Tofandel commented 6 months ago

It would be nice to have access to the vue devtools, but it seems that even in dev mode it's disabled Nvm it's just the usual bug of my browser not displaying the tab until I reopen it

Tofandel commented 6 months ago

Okay I completely rechanged the approach, I got everything working perfectly finally, schema is clean and a bit less memory hungry than before

I also tested old revisions and switching backend while editing frontend since the structure is mostly the same and it's all backwards compatible, I made sure of it

Tofandel commented 6 months ago

Good to go now repeaters are only stored like

parts
blocks-parts-1|lessons
blocks-lessons-1|content

Instead of the original approach of

parts
parts|blocks-parts-1|lessons
parts|blocks-parts-1|lessons|blocks-lessons-1|content

Or the current not working approach in 3.x

parts
blocks-parts-1-lessons
blocks-parts-1-blocks-lessons-1-content

This is the same as how blocks are stored flattened and it seems to work just fine so I just thought let's reuse that

Tofandel commented 6 months ago

I think I'm pretty much done on the best way to approach this by causing the least amount of breaking change

Now the schema hasn't even changed except for the nested repeaters outside the block editor as mentionned in the previous comment which was something that was not working before so it can hardly be called a breaking change and only a fix

As for testing, I'm personnaly testing on a module with 4 levels of inline repeaters with blocks and nested blocks and browsers inside those repeaters and repeaters inside those blocks and I'm happy to report that everything is working perfectly image

antonioribeiro commented 5 months ago

This is great @Tofandel , thank you!