The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
Found a 5.x issue where a pager was added to the revisions list, it's regressed since but not sure where.
Now get out of memory or max_execution_time if an entity has a lot of revisions.
Core has the same problem looks like the diff form copied some of that code, see #2746033: NodeController::revisionOverview() does not have a pager, which results in unlimited queries.
Proposed resolution
Add a pager.
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | interdiff-2746069-18-19.txt | 2.78 KB | johnchque |
| #19 | needs_tests_no_pager-2746069-19.patch | 4.74 KB | johnchque |
| #18 | needs_tests_no_pager-2746069-18.patch | 4.63 KB | johnchque |
| #18 | interdiff-2746069-7-18.txt | 3.33 KB | johnchque |
| #7 | diff.add_revisions_pager.2746069_07.patch | 3.33 KB | rfay |
Comments
Comment #2
rfayComment #3
catchNot sure I'd bother making it configurable, views can (or should if it needs a bit more integration for diff) do that. Otherwise looks good though.
Comment #4
rfayThis patch adds the revision_pager_limit type to the schema. Thanks @damian.
@catch, since views bulk stuff is not currently revisions-aware, it seems like it might be a while before this is easy to do with a view, so with that concern, I'm inclined to leave the config in there. Thanks so much for the review.
Comment #5
rfayRevisions are currently in reverse order :(
Comment #6
rfayOK, now the revisions are in the correct order and the config gets properly set on a system where diff was already installed.
Comment #7
rfay- Remove the inline code to add default config
- Change to diff_update_8001() which is the logical choice
Comment #8
miro_dietikerI first opened an issue about the HEAD fails... #2757469: HEAD is broken
Comment #14
miro_dietikerI was about to commit this after fixing HEAD, but there is some fail. Also we also want to have a minimalistic test coverage for diff.
Comment #16
johnchqueLet's add tests for this. :)
Comment #18
johnchqueAdded tests, fixed current ones. :)
Comment #19
johnchqueMade some changes to avoid creating 50 revisions with no reason. :)
Comment #20
miro_dietikerThx, committing. :-)