Suppress processing of part when displaying

Topics: Customizing Orchard, Writing modules
Developer
Nov 1, 2011 at 10:17 AM

Could somebody please tell me what is the best way (even if more complex) to suppress a part of a content item completely when building the display? I understand there is Placement.info to hide shapes, but what if the underlying Driver makes something computationally intensive what we want to completely avoid (under some conditions)?

Am I missing something and Placement.info can also suppress even the invocation of the Driver? (It seems to me that it does not.) Or is there any other way? The content part in question is not under my influence.

Thanks in advance!

Nov 1, 2011 at 10:34 AM

If you look at many of the existing drivers in Orchard's built-in modules you'll see they use "factory methods". That is, no processing is done immediately in the driver; instead a closure function is passed into the ShapeResult, and any significant processing is done in that delegate. The closure will only get executed if placement finds a match.

I'm not sure how well this is explained in the docs - it's something I figured out on my own after writing a few modules. There are probably lots of community modules that do things improperly and will therefore perform work whether displayed or not (I know some of my early code was guilty of this!)

There are situations where the delegate pattern is hard to stick to - something I found troublesome was if you iterate over a set of items and emit a delegate for each one - the variable when the delegate finally executes is scoped to the last value of your enumeration, rather than correctly taking each value. But it's not a very common situation, and I found ways to work around it.

Developer
Nov 1, 2011 at 11:21 AM

Thank you, that's very interesting. I've never though of that, although this is a very clever solution I'll use in my modules from now on.

I'll elaborate my problem a bit further, since unfortunately it seems that this approach is not suitable in this special case. What I'd like to do is to hide the contained items of a container on specific occasions. It looks that containers go through their own special controller (Orchard.Core.Containers.Controllers.ItemController) action. There the list of contained items is made before the display of the other parts of the content type is built, so actually there's no driver. I guess then there is not possible to suppress this list?

Nov 1, 2011 at 11:58 AM

You're right, the special case of the container controller is fairly inflexible.

There are of course a number of ways to work around this situation. One would be to use an override template when your conditions are met, and suppress the list displaying in the template (but of course the list would still be getting processed).

Another option is to use Mechanics which gives you far more flexibility to customise the behaviour and rendering of child relationships.

What are the exact conditions under which you need to suppress the list?

Developer
Nov 1, 2011 at 3:10 PM

I've already looked at template override, but the problem is just as you say: in the background the list is there anyway, just not shown. Now one could say that it won't make a real difference unless we have dozens of complicated items on one page, which is true, but still, this doesn't look like a nice solution (and of course we're always struggling to find the most beautiful solution, aren't we :-)?).

I'll look into Mechanics, thanks.

Basically the condition would be a bool in another part, nothing fancy: a tick box would be displayed in the editor whether to display the list or not.

To further specify the task, since it's not a secret project, I wanted to write a simple hierarchical page module, where the users would have the option to very easily add subpages. I know there are a few modules doing this, but I aimed for something really simple. Now if the list would be displayed from a driver, the module would be in the gallery since yesterday :-).

Coordinator
Nov 1, 2011 at 3:16 PM
A handler is a good place to intercept things and modify the structure of a content item before it gets to do anything harmful.
You should be able to add and remove parts from there.

Sent from my TI-99/4A

From: Piedone
Sent: 11/1/2011 8:11 AM
To: Bertrand Le Roy
Subject: Re: Suppress processing of part when displaying [orchard:277865]

From: Piedone

I've already looked at template override, but the problem is just as you say: in the background the list is there anyway, just not shown. Now one could say that it won't make a real difference unless we have dozens of complicated items on one page, which is true, but still, this doesn't look like a nice solution (and of course we're always struggling to find the most beautiful solution, aren't we :-)?).

I'll look into Mechanics, thanks.

Basically the condition would be a bool in another part, nothing fancy: a tick box would be displayed in the editor whether to display the list or not.

To further specify the task, since it's not a secret project, I wanted to write a simple hierarchical page module, where the users would have the option to very easily add subpages. I know there are a few modules doing this, but I aimed for something really simple. Now if the list would be displayed from a driver, the module would be in the gallery since yesterday :-).

Nov 1, 2011 at 3:22 PM

The easiest approach might be to inherit the controller (and OrchardSuppressDependency the old one) - then override the relevant action and modify the code slightly.

Hierarchical pages can certainly be done with Mechanics, and although the admin UI is a bit clunky right now it'd be possible to raise a "Create Sub Page" link - I can help out with this if you wanted to go that route. With Mechanics you'd have a number of advantages in how you can control the rendering and placement of the sub page list, and by default no processing will be performed if the list is hidden.

Nov 1, 2011 at 3:25 PM
BertrandLeRoy wrote:
A handler is a good place to intercept things and modify the structure of a content item before it gets to do anything harmful.
You should be able to add and remove parts from there.

The problem there is that the shape still gets created, you can just prune it afterwards. So the work building the shape still happens (which is what Piedone is trying to avoid).

On the other hand, if you're talking about actually removing the ContainerPart before it gets rendered, that would simply throw an exception in the ItemController of Orchard.Containers!

Coordinator
Nov 1, 2011 at 7:00 PM

Maybe. I didn't try it. But overtaking the controller seems very heavy-handed and intrusive. There must be a better way.

Nov 2, 2011 at 9:23 AM

The best way would be fixing Containers so they render as normal items, with a driver to render the list; that way it can be switched on and off with placement. Maybe this could be part of the Alias/Routable work, since container routing will need looking at with all of that anyway?

Developer
Nov 2, 2011 at 9:49 AM
Edited Nov 2, 2011 at 9:52 AM

Thanks both of you for your feedbacks.

I couldn't think of anything better than overriding the controller, despite this being intrusive.

I understand a controller is necessary to handle the pager parameters, but I guess despite this the list could be rendered from a driver, maybe lazy-loading the contained items into the part?

Nov 2, 2011 at 10:21 AM

If the list comes from a driver then lazy loading is handled already by the factory method I initially described. There should be a way to handle the pager parameters without relying on the controller.

Hmm ... even then, since your conditions are logic-based they still can't be achieved thru Placement (although, I've been doing my own experiment with making Placement extensible so you can create your own Match predicates...)

Developer
Nov 2, 2011 at 6:15 PM

Today I tried to go the other way around and set PageSite in ContainerPartRecord to 0 in the handler. This won't prevent the list from displaying as I thought: if ContentQuery.Slice() is called with 0, 0, then all the records (not none of them) are returned. Sadly, this isn't a solution either, although this would also solve the problem of not being able to set placement programatically. Maybe the latter isn't true, maybe BuildDisplayContext's FindPlacement method (used in Handlers's OnGetDisplayShape<>() methods) is something useful for this?

Developer
Nov 3, 2011 at 1:17 AM
Edited Nov 3, 2011 at 3:45 PM

Please take a look at the this fork. I've refactored containers, so now the list is rendered from the driver. There's an option whether or not the list should render (just like it's with comments). I've also added a new property to ContainablePart, Position. With this, items can have an arbitrary position.

What do you think? Could this modification or parts (or the approach) become part of the core? (If yes, of course I would set up the necessary administration - issue/issues - too.)

Developer
Nov 10, 2011 at 3:09 PM
Edited Nov 10, 2011 at 3:09 PM

Could somebody (perhaps from The Team) please look at the fork? I'm willing to describe the mechanism and outline the differences.

Coordinator
Nov 10, 2011 at 4:51 PM

I am willing to take a look at it as it could also fix an active bug.

Coordinator
Nov 10, 2011 at 5:07 PM

Believe it or not, I like it ! A lot !

Few remarks:
- The driver should not check for the DisplayType to return automatically. You need to set it in the Placement file, so that people can decide to change it. Testing on the ItemsShown is ok though
- I don't understand the purpose of  ItemsShown ? Why would I check false ?
- "Position" is misleading, as you could have multiple values with the same one, they are not managed. Maybe Weight would be better, which would also invert the OrderBy
- In the migration, fusion the updates in one, and update the Create() to return the same value as the update, after applying the modifications to it to. Do you understand the goal ?

I have not tried it live though. Can you check if with those changes the List can be added on the home page ? 

Developer
Nov 10, 2011 at 5:23 PM

Thanks a lot for taking time and I'm glad you liked it.

"The driver should not check for the DisplayType to return automatically. You need to set it in the Placement file, so that people can decide to change it. Testing on the ItemsShown is ok though"
Right, I'll fix this, fully understandable.

"I don't understand the purpose of  ItemsShown ? Why would I check false ?"
The intention was that sometimes it would be necessary to maintain a container-containable relation, although not listing containables automatically. Formerly I found such functionality very handy with other systems: on a page we want to link to the contained pages by hand (in some very special list, or in the text), but still, these pages would be contained by their parent. A good example is here at CodePlex the documentation page: one creates pages "under" the docs, but the links to these pages can be inserted at arbitrary locations.

""Position" is misleading, as you could have multiple values with the same one, they are not managed. Maybe Weight would be better, which would also invert the OrderBy"
Yeah, sure, I've seen this is the notion also used with Taxonomies, so this could be named with the same convention.

"In the migration, fusion the updates in one, and update the Create() to return the same value as the update, after applying the modifications to it to. Do you understand the goal ?"
That's something I planned to do, but haven't done yet. I also like to keep migrations uncluttered. However I'm a little confused about returning Create() the same as the Update. Shouldn't there be one update to make the necessary changes?

"Can you check if with those changes the List can be added on the home page ? "
I'll check it.

Developer
Nov 10, 2011 at 8:31 PM

I pushed several changesets with the above modifications.

I checked, making a list to the home page works. Are there any reasons it shouldn't?

Coordinator
Nov 10, 2011 at 8:45 PM
Edited Nov 10, 2011 at 8:45 PM

It wasn't before.

Developer
Nov 10, 2011 at 8:53 PM

And what's desired? :-)

Coordinator
Nov 10, 2011 at 9:10 PM

Well, if you promote a list to be the home page, it should work. It didn't before.

Developer
Nov 11, 2011 at 12:16 PM

Cool, actually I didn't know there was such a bug.

Nov 25, 2011 at 11:28 AM
Edited Nov 25, 2011 at 11:30 AM

Just wanted to make sure you saw http://orchard.codeplex.com/discussions/280815 regarding the Pager shape. It seems like it's no longer there at all?

How I think things could be further changed to provide maximum flexibility to control the rendering, is to actually add a view called Parts_Container_Contained.cshtml (rather than currently how it's a list shape but aliased as Parts_Container_Contained for placement). This would simply contain:

@Display(Model.List)

@Display(Model.Pager)

So now the driver would create that part view, and pass both Pager and List shapes into it; then anyone can easily override that view and rearrange things however they want.

Another approach might be to actually provide two separate driver results - one for the list, one for the pager. But I think that decreases flexibility because you couldn't do what alexmarreiros is after, i.e. have pager both top and bottom (a reasonably common design).

Finally - the driver is emitting these shapes:

 

                ContentShape("Parts_Container_Contained_Summary",
                             () => shapeHelper.Parts_Container_Contained_Summary(ContentPart: part)),
                ContentShape("Parts_Container_Contained_SummaryAdmin",
                             () => shapeHelper.Parts_Container_Contained_SummaryAdmin(ContentPart: part))

 

But, there is no Parts_Container_Contained_Summary at all, and Parts_Container_Contained_SummaryAdmin only exists in Orchard.Lists; so if anyone tried to display either of these shapes you'd get a "key not present in the dictionary" error thrown by the shape table. Having Lists enabled would fix the admin but not the normal summary.

Finally finally - Placement.info in Containers doesn't actually display the Parts_Container_Contained shape, so by default you'll get no list. Again you need the Lists module enabled which does have the Placement entry. The question really is, what is the intended separation between Orchard.Core.Containers and Orchard.Lists? Perhaps the driver should just be in Orchard.Lists in the first place?

Developer
Nov 27, 2011 at 7:14 PM
Edited Nov 27, 2011 at 11:26 PM

Thanks, very valid points.

The pager being not there is a big mistake from me. It looks that I commented out the line where the pager shape was built for some reason then I forgot about it. The first solution you mentioned would be indeed the best approach in my opinion too, I'll fix it tomorrow.

About the misery with Lists: not as an excuse, but that's the original implementation (sincerely I haven't known where these shapes should come from until now you mentioned Lists) I was hesitant to change. I think the reference to Parts_Container_Contained_Summary should be deleted for sure (but maybe a core developer could describe why it's there).

SummayAdmin should be handled somehow else (clearly not in Containers; maybe there's a way to inject the shape without the driver having to return the corresponding ContentShape).

I'm hesitant whether the driver should be in Containers or in List...

Developer
Nov 27, 2011 at 8:51 PM
Edited Nov 28, 2011 at 6:12 PM

I've made the pager fix today in this new fork. I'll think about the other issues too.

Developer
Nov 28, 2011 at 6:50 PM
Edited Nov 28, 2011 at 6:52 PM

I think that the driver should stay in Containers. It's supposed to be the ContainerPart's driver, usable without Lists, so not having a driver would be a mistake IMHO. However to decouple Containers from Lists, there is a need for another driver, so I've made one that suppresses the original one but inherits from it. I really don't like the casting of shapeHelper to object, though.

Regarding the placement issue with Parts_Container_Contained: the placement for this shape was already in Container's Placement.info. Am I missing something?

Please take a look at the changes, I would be glad to hear feedbacks.

Nov 28, 2011 at 7:04 PM

I'm not totally sure what the point of the Lists module is, and there's so little in it, personally I'd say just move it all to Core.Containers (and its Migration could just be in a recipe). But you should get a dev feedback before doing anything so drastic :)

The inherited driver sounds odd; of course it'll work that way, but it doesn't sound too neat. I'm wondering why does the driver need to inherit, can't you just have a second driver that spits out an extra shape (you realise you can have multiple drivers for the same part?) I was going to have a look at the code but I can't access the fork you linked earlier.

Developer
Nov 28, 2011 at 7:22 PM

I hope a core developer will tell something about Lists.

No, I haven't known that there can be multiple drivers (although I should have recognized this, my bad...), I've fixed that.

I don't know what's the problem with the fork (you tried the new fork linked three messages up?), it's up and running for me, must be a temporal Codeplex issue.

Nov 28, 2011 at 7:31 PM

Ok, had the wrong fork link from further back.

The more I look at Orchard.Lists, the more I wonder if it needs to exist at all. Especially since even more of what it currently does will disappear away into Alias / Autoroute / etc.

Coordinator
Nov 28, 2011 at 7:54 PM

Let me be very clear on this ;)

Orchard.Lists is an ugly thing. We built it because back then we had pressure *cough*management*cough* to build a custom list feature of sorts asap. What we wanted to do from the start was Projector, but we needed something fast. So we built that half-assed feature. So is it still necessary? Hell, no. I would gladly kill it if people weren't relying on it.

Developer
Nov 28, 2011 at 8:13 PM

I guess then what's currently in the fork OK since there is no more coupling with Lists. Diving into Lists would be pointless, so there is nothing more to do, everybody can go home, wonderful :-). I'm greatly appreciating Projector!

Developer
Dec 13, 2011 at 8:19 PM

I am on the 1.x branch and I need the Pager.

@Piedone, your fix in repo (here: http://orchard.codeplex.com/SourceControl/network/Forks/Piedone/Containers), how far away is it from being a pull request?

Developer
Dec 13, 2011 at 8:48 PM

It's done, I've notified Sebastien about the issue and the fix. I guess he'll pull when he gets there.

Developer
Dec 13, 2011 at 8:50 PM

Fantastic!! Thanks Piedone for the push. I will also bug people to get it thru!! :)

Coordinator
Dec 13, 2011 at 8:51 PM

long way to ... get there :/ don't bug me, I am aware of it