Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delta updates #5146

Merged
merged 36 commits into from
Oct 28, 2019
Merged

Delta updates #5146

merged 36 commits into from
Oct 28, 2019

Conversation

brandonkelly
Copy link
Member

@brandonkelly brandonkelly commented Oct 23, 2019

Will resolve #4064, #4149, and pave the way for a proper fix for #4642.

  • Elements should keep track of dirty field values.
  • Only update content for dirty fields
  • Track deltas for Edit Entry forms
  • Track deltas for Edit User forms
  • Track deltas for Edit Category forms
  • Track deltas for Element Editor HUDs
  • Matrix field support
    • Only process field if dirty
    • Merge delta blocks with existing blocks
    • Only save dirty blocks
    • Update sort order separately
  • Relational field support
  • Only update search indexes for dirty fields

- fieldAttributes (alternative to the attr block)
- labelAttributes
- Only submit entry fields that were changed in some way
- Elements keep track of dirty field values
- Skip updating content values for non-dirty fields
- Matrix & relational fields skip post-save element processing if their field values aren't dirty
@brandonkelly brandonkelly added this to the 3.4 milestone Oct 23, 2019
# Conflicts:
#	src/web/assets/cp/dist/js/Craft.js
#	src/web/assets/cp/dist/js/Craft.min.js
#	src/web/assets/cp/dist/js/Craft.min.js.map
If a block-style field that supports delta updates is nested in another block-style field that doesn't support delta updates, no delta input names should be registered
@Mosnar
Copy link
Contributor

Mosnar commented Oct 26, 2019

I just wanted to say that this is awesome and it's working great in my tests!

Gonna casually drop some ideas...

  • Could start tracking fields accessed on elements in templatecacheelements and invalidating template caches more precisely based on the fields used in them. I couldn't help myself and actually developed a rough implementation of this and it was magical, though there are some table growth implications to consider.
  • Could automatically set revision notes to include a list of updated fields if left unspecified
  • Could have a "hard-save" keyboard short-cut, such as CMDSHIFTS that would save all fields to appease the inevitable complaints of not re-saving all fields.
  • Could start updating the search index on a per-field-basis as a further optimization

@brandonkelly
Copy link
Member Author

@Mosnar Great ideas!

Could start tracking fields accessed on elements in templatecacheelements and invalidating template caches more precisely based on the fields used in them. I couldn't help myself and actually developed a rough implementation of this and it was magical, though there are some table growth implications to consider.

We’d also have to start tracking other element properties for this to work effectively. Otherwise let’s say you have a cached template that outputs an entry’s author, and then edit the entry to change the author (without modifying custom field values), the cache wouldn’t get cleared. We could track whether any element properties besides their custom fields were accessed, but at that point there’s not much to gain here.

That said, we’re probably going to swap out the entire template caching system with something else in Craft 4, so I’ll keep this in mind for that.

Could automatically set revision notes to include a list of updated fields if left unspecified

The plan to fix #4642 is going to involve keeping track of custom field changes, and we’ll likely find a way to show what’s changed within the UI, so this should be unnecessary.

Could have a "hard-save" keyboard short-cut, such as CMDSHIFTS that would save all fields to appease the inevitable complaints of not re-saving all fields.

Yeah it will probably be needed from time to time. I think I’ll make it possible from resave/* commands, and see if that is enough.

Could start updating the search index on a per-field-basis as a further optimization

Yeah we should definitely do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants