Just from glancing at it, the code seems fine. It needs testing though (and see my 2 notes)

Don't really have time to test, but from quick look at the code patch LGTM as well.

@Bastien Montagne (mont29) thanks for taking a quick look, should I go ahead and commit now then or wait for better review?

@Zachman If I have an object with a child which (the child) is not in the collection, and I go in collection->delete hierarchy shouldn't the child be also deleted too? Right now it seems inconsistent with delete hierarchy in an object deleting its children.

How to reproduce:

1. Default scene, make cube child of lamp, move cube outside Collection; Collection -> delete hierarchy. (Cube is not gone)
2. Default scene, make cube child of lamp, move cube outside Collection; Lamp -> delete hierarchy. (Cube is gone)

@Dalai Felinto (dfelinto) tested, to me I think the current behavior could make sense. Collection delete hierarchy is to delete objects that are part of the collection, and object delete hierarchy is to delete an object parent-child hierarchy. Because the cube in your example is not part of the collection, it shouldn't be removed. I could see it the other way though. Maybe this behavior is fine for now since it's the same in master? Does @William Reynish (billreynish) have any thoughts?

That being said, I did just find an issue (crash) when deleting hierarchy on an object linked in two collections, so I'll go fix that.

@Brecht Van Lommel (brecht) (and @Andy Goralczyk (eyecandy)) may have something to say regarding the decision on whether delete hierarchy in a collection should also delete children objects that are not in the collection.

I'm not sure about the ideal behavior there to be honest. The fact that you can even create such situations is problematic.

My guess is that delete hierarchy should leave anything that is visually displayed outside the hierarchy unchanged. So objects in multiple collections or child objects should not be deleted if they are shown outside the selected hierarchy somewhere.

My 2 cents:
Delete Hierarchy on a collection: Don't delete child objects outside the collection.
Delete Hierarchy on an object: Delete everything that's parented to the object. Don't take collections into account at all, even though a child being in another collection might display somewhere else in the UI.

It would help to have a user-facing distinction between the two concepts of hierarchies here. Would it make sense, if only objects or only collections are selected, display "Delete Object Hierarchy" or "Delete Collection Hierarchy" accordingly?
And if both objects and collections are selected, either display "Delete Hierarchy", or don't provide the option at all.

I fixed the crash with deleting hierarchy objects in multiple collections. Right now it allows deleting objects hidden in closed hierarchies, but it would be easy to only allow deleting the visibly selected objects.

@Brecht Van Lommel (brecht) @Dalai Felinto (dfelinto) The behavior issues mentioned earlier only arise because I have removed the condition to only allow delete hierarchy in scenes view. I think because the outliner shows the object tree it makes sense to allow object delete hierarchy in View Layer mode. The current behavior matches that of master for deleting hierarchy on objects and collections. Because the deletion code itself has no behavioral changes I think this is good to go, but it could later be refined if needed.

@Brecht Van Lommel (brecht) thanks for reviewing; should I wait for @Dalai Felinto (dfelinto) to also review this before committing?

No, it's fine like this.