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

[4.x]: Structure parents with no children show expand caret #11253

Closed
leevigraham opened this issue May 18, 2022 · 9 comments
Closed

[4.x]: Structure parents with no children show expand caret #11253

leevigraham opened this issue May 18, 2022 · 9 comments

Comments

@leevigraham
Copy link
Contributor

What happened?

Description

Screen.Recording.2022-05-18.at.10.39.26.am.mov

Steps to reproduce

  1. Create a structure
  2. Add a parent and child entry
  3. Delete child and clear trash

Expected behavior

Parent entry should have no caret

Actual behavior

Parent entry has caret

Craft CMS version

4.0.2

PHP version

No response

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

@brandonkelly
Copy link
Member

Not able to reproduce this. How did you delete the descendants?

@leevigraham
Copy link
Contributor Author

I just tried this again and it works as expected 🤷‍♂️

  1. Opened the tree
  2. Selected multiple children
  3. In the drop down select delete

image

I'll check the structure in the DB and see if I can see anything

@leevigraham
Copy link
Contributor Author

Okay… weird.

  • Element #16322 (Destinations) has a lft 60 and rgt 33 indicating it has a child
  • Element #16517 is the child but is invalid

Screen Shot 2022-05-19 at 10 21 40 am

Screen Shot 2022-05-19 at 10 21 31 am

Rebuilding the structure shows an unpublished draft:

image

but there are no drafts in the admin:

image

but there are drafts in the DB:

Screen Shot 2022-05-19 at 10 25 28 am

@leevigraham
Copy link
Contributor Author

I've deleted the page and recreated it which fixes the issue because a new element is created and the structure tree is cleaned up.

Probably a super edge case… could have also happened while I was working on a module so I'm going to close this.

@mmikkel
Copy link
Contributor

mmikkel commented Jan 30, 2023

This is an edge case for sure, and this may or may not be what caused the original issue here (I debated creating a new issue or not), but I do think there is a bug here related to how Craft determines if structure entries should display the child toggles or not.

One thing that definitely triggers very similar behavior is if an author creates a new child entry that is never saved (i.e. the draft is never converted from an unpublished draft to a provisional draft).

Being unsaved, the draft will be excluded from displaying in the entry index, but the parent structure entry will still display a "show/hide children" toggle for it, as the unsaved draft still counts as a descendant. In cases where a structure entry only has unpublished drafts as children, the end result is a child toggle that does nothing, exactly like in the screencast above.

The thing that makes this a edge case is that (AFAIK) there's not actually a way in the UI to create an unsaved draft that is also a child of another entry, as the action of adding a parent entry to a draft via the UI will "save" the draft (i.e. it's converted to a provisional draft), just like any other content edit.

But, there's at least one other way to create an unpublished draft that is a child of another entry: using the /new endpoint and the query parameter parentId. I.e. if a child entry is created using a URL like this:

https://example.com/admin/entries/someStructure/new?parentId=464

TL;DR: Unsaved drafts should probably be excluded in the getDescendants() query that determines whether or not structure entries should display their show/hide children toggle.

(An interesting aside: as far as I can tell, the only place unpublished drafts actually do display, is in the "Choose parent" element selector modal. That could be intentional, or perhaps a separate issue - my guess is that they shouldn't be visible there either?).

@brandonkelly
Copy link
Member

brandonkelly commented Feb 3, 2023

I believe we are showing the toggle if the entry’s lft and rgt values are more than 1 digit apart, which indicates that there are nested entries within it. (Thus avoiding an extra DB query per entry to check if it actually has descendants that should be shown on the page.)

The thing that makes this a edge case is that (AFAIK) there's not actually a way in the UI to create an unsaved draft that is also a child of another entry, as the action of adding a parent entry to a draft via the UI will "save" the draft (i.e. it's converted to a provisional draft), just like any other content edit.

Not quite – when you create a new unsaved draft, it will immediately be placed within the structure. And even provisional drafts will have a place within the structure if they have been assigned a different parent than the canonical entry.

@mmikkel
Copy link
Contributor

mmikkel commented Feb 3, 2023

I believe we are showing the toggle if the entry’s lft and rgt values are more than 1 digit apart, which indicates that there are nested entries within it. (Thus avoiding an extra DB query per entry to check if it actually has descendants that should be shown on the page.)

Hm... I'm slightly confused now, but it looks to me like the toggles are shown if the individual structure element has descendants, which is deduced from a per-element getDescendants() query here?

And I don't think the issue is that the unsaved draft isn't placed within the structure – the problem is that the element index doesn't display unsaved drafts, whilst the getDescendants() query determining if the toggles display or not, doesn't exclude unsaved drafts. Which will manifest in the described behavior, in cases where a structure element only has unsaved draft descendants.

Possibly that getDescendants() query should use the .savedDraftsOnly() param? Seems to solve the issue on my end.

brandonkelly added a commit that referenced this issue Feb 3, 2023
@brandonkelly
Copy link
Member

Doh… I forgot we switched to actually checking for descendants at some point. 🤦🏻

Yep, .savedDraftsOnly() does do the trick! Just added that for the next Craft 3 and 4 releases.

@brandonkelly
Copy link
Member

Craft 3.7.64 and 4.3.7 have been released with that fix.

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

No branches or pull requests

3 participants