Enhancement/attributes localization

[MatteoPiovanelli-Laser]

This implements the feature presented in #110

[MatteoPiovanelli-Laser]

In the latest changes:

• I replaced the Tuple<int, int> with the struct AttributeIdPair
• I created an MVC filter that loads a script into the index view of AttributesAdminController. The script adds the little culture string I was previously adding to the DisplayText property of the ContentItems being listed.

[bleroy]

Thanks for the changes. I don't understand why you need to do that through a filter and script. Isn't there a template that you could alternate or override instead?

[MatteoPiovanelli-Laser]

My reasoning in favour of the filter injecting the script is a matter of flexibility.

(This is all based on what I know about using alternates, which is admittedly not a lot) Alternates as I understand them seem to be more rigid. With alternates:

• The Nwazet.Attributes feature uses the Index.cshtml template as the view for AttributesAdminController's Index.
• If I activate Nwazet.AttributeLocalizationExtension , I somehow (using IShapeTableProvider?) "enable" an alternate. This alternate specifically is pretty much identical to the original Index.cshtml, with the exception of the "(culture)" notation after each attribute's DisplayText in the list.
• If another feature wants to change something else in that view, now it has to do its own alternate, accounting for the fact that Nwazet.AttributeLocalizationExtension may or may not be enabled. Suppose for example a SubscriberAttributes made-up feature adds a BooleanField to the Attributes ContentType that tells me whether that attribute can only be changed by customers belonging to a "Subscribed" role. Suppose that feature wants to show that in the view by writing "(subscribers only)" after the DisplayText. I would have a new alternate, but then this would not have the "(culture)" text, because that is from a separate feature.

WIth the filter, I can just say: "if this ViewModel is there, add this script". This is not perfect of course. I can see that if someone implemented an alternate, the script may fail to do what it's supposed to. But unless I make some significant change, I could stack MVC filters.

Controller overrides, I think, suffer from pretty much the same flexibilty limit that I see in alternates.

Did I make sense?

(Github tells me there is "1 review requesting changes by reviewers with write access." but I cannot see it)

[bleroy]

The person who wants to create an alternate has the choice of creating one or the other, or even add their own based on their own criteria. That's the ultimate flexibility. With alternates, you can add an alternate in exactly the same circumstances where you add your script currently. It seems like you don't like alternates and as a consequence came up with your own feature. Well, alternates are the way it's done in Orchard.

[MatteoPiovanelli-Laser]

I changed that thing. To have the list with the added "(culture)" notation, I override the route to "AttributesAdmin/index" so it goes to "LocalizationAttributesAdmin/Index". THat calls the view where I made that change.

I was not able to simply use a IShapeTableProvider to replace the template here.

[bleroy]

I think if you changed the original controller so it uses a shape as the view model, that should be easy to override without taking over the route. I'm not sure what you tried in a IShapeTableProvider that didn't work. What shape did you add alternates to?

[MatteoPiovanelli-Laser]

My attempt with IShapeTableProvider failed because I was trying to somehow override the View used by the controller. I figured, and Sebastien just confirmed it, that I cannot do it that way.

I haven't played with alternates much, to be honest. Could give a couple snippets to clarify what you mean here? WHat I am thinking is to have the Index method end on

return new ShapeResult(_someServiceProbablyAttributesAdmin.GetIndexShape(vm));

The default implementation there returns the shape it's been using so far. THe implementation from the feature in this branch adds the "(culture)" notation.

[bleroy]

Well, this view started as something super simple, just iterating over a bunch of content items and rendering some links and buttons with the data in there. At this point, it should be clear that we've hit a case where this simplicity is no longer affordable since we have a clear extensibility scenario. What I'd do is that I'd refactor the original view a bit so that it creates custom shapes for each of the attributes, and then you could add alternates to that.

I wrote some blog posts a long time ago on that stuff, but there's also documentation on it now: https://weblogs.asp.net/bleroy/creating-shapes-on-the-fly https://weblogs.asp.net/bleroy/orchard-shapeshifting

[MatteoPiovanelli-Laser]

I think the latest changes are in the right direction, else I completely misunderstood your last few comments.

I changed the Index view so it shows a shape for the list of attributes. The list of attributes, then, for each attribute calls a specialized shape. Using an IShapeTableProvider, the localization feature adds and alternate to this latter specialized shape.

[bleroy]

Yes, that's exactly it :) I love how much red there is in this change ;) Thanks for bearing with me, I hope you agree this is much better than what we started with.

[bleroy]

Thanks a lot. I really appreciate the many contributions you've made recently. I hope my requests for changes are not too annoying :D

[MatteoPiovanelli-Laser]

Not at all. I am learning quite a bit while doing this, and that is ever my goal.