Skip to content

Added PreviewLinkAutopopulate plugin to auto-populate preview links #146

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

Merged
merged 5 commits into from
Aug 6, 2024

Conversation

stephen-cox
Copy link
Member

@stephen-cox stephen-cox requested review from finnlewis and ekes May 24, 2024 09:20
@ekes
Copy link
Member

ekes commented Jun 17, 2024

Is there something else that needs to be done other than checking out the branch. I've got it, and done a site install, and used demo, but I don't see the button, only one to copy the preview link.

Also is it intentionally only getting the next level of child pages, or should it be getting all child pages (using the Nested Storage)?

@stephen-cox
Copy link
Member Author

Also is it intentionally only getting the next level of child pages, or should it be getting all child pages (using the Nested Storage)?

I missed that subsite pages can also be parents along with overviews, I'll have to update to logic to handle this.

@finnlewis finnlewis marked this pull request as draft June 18, 2024 11:22
@finnlewis
Copy link
Member

@stephen-cox working on this today

@stephen-cox stephen-cox marked this pull request as ready for review July 31, 2024 15:39
@stephen-cox
Copy link
Member Author

@ekes I have updated this PR to use Entity Hierarchy to load the nodes in a subsite; which is frankly a pain to work with. Loading all nodes is not straightforward so I've added a test.

Would be nice to get this reviewed before Next Tuesday, if possible, as it's been hanging around for a while now.

Copy link
Member

@ekes ekes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the rest that works.


$storage = $this->getNestedSetStorage('localgov_subsites');
$node_key = $this->getNestedSetNodeKeyFactory()->fromEntity($node);
if ($ancestors = $storage->findAncestors($node_key)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could just be $storage->findRoot($node_key) which does much the same.

@ekes
Copy link
Member

ekes commented Aug 6, 2024

Loading all nodes is not straightforward so I've added a test.

It's just get root, get all ancestors of root, and load them? Seems straightforward to me. Pain I found is the way that NestedSet class has be abstracted so you don't get code completion (or I don't anyway).

@ekes ekes merged commit 86246d8 into 2.x Aug 6, 2024
8 checks passed
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.

3 participants