Merge Persistent templates into Customer and Checkout

[barbazul]

Since Mage_Persistent appeared, a bunch of templates from Mage_Customer and Mage_Checkout were never used again as they were overriden.

I think Magento 2 is a good time to clean this up and get rid of Mage_Persistent templates that override Mage_Customer's and Mage_Checkout's

[brendanfalkowski]

I agree with this suggestion. Either relocate the templates from Mage_Persistent, or erase the deprecated files in Mage_Customer and Mage_Checkout. This can be done at any time (i.e. Magento CE 1.7.1).

[barbazul]

I'd go with moving them to Mage_Customer and Mage_Checkout as it's more intuitive that way.

It makes more logic to go looking i.e. for the registration form inside Mage_Customer than inside some module that's actually an "appendage" to the existing customer account/session functionality from older versions

[matneves]

Totally agree. Why did they included the Mage_Pessistent, by the way?

[barbazul]

@matneves I used to think that way but it sorts of make sense to have it in a separate module as it "touches" functionallity in both Mage_Checkout and Mage_Customer modules so it cannot be fully merged into either.

I guess the whole module could be divided in two and then merge each part where it belongs, but I don't think it's worth the trouble.

However I still stand firmly against the overriden templates that only bring confusion to developers that are unaware of them.

I'll try to submit some code to see if this issue gets more attention

[brendanfalkowski]

@barbazul So basically deleting the previously deprecated templates. I'm on board with that.

I've seen plenty of "developers" ask why their changes in checkout and customer don't update the frontend. Logically they should, but they're not using template path hints to check what layout XML is actually rendering. This isn't a huge problem, but it's easily avoided by clearing out those deprecated templates.

[barbazul]

What do you think? should we delete the old templates or should we move the changes in the new templates back to the old ones?

I think the changes in the new templates can be simply replaced with a generic container called "additional" and have the layout in persistent use that container to drop the "remember me" boxes. The same pattern is used in other modules, after all

[brendanfalkowski]

I hate the generic "additional" container pattern. It's never clear what the purpose of that container is or which modules make use of it. By deliberately not being explicit you effectively have to read the entire codebase to see what might impact it. In practice, an end-user will flick some switches in the admin and something suddenly appears which breaks the layout because you had no idea you needed to test/accomodate for it.

If you're going to explicitly place components, then do it in a traceable way by adding a named block to its parent — not by creating a false parent.

At this point I think deleting the non-Persistent templates is better. We've already migrated customizations from each module in Persistent once. There's no point migrating those some customizations back into the original modules. That's a big waste of everyone's time.

[barbazul]

Meh. I'm not so convinced by your argument of having to trace where a particular container is used throughout the codebase when you have tools like Alan Storm's Layout viewer, Commerce Bug, Magneto debug, etc.

But I agree deleting the old templates is the easiest and fastest route to take.

I guess the only thing that keeps bugging me is that Magento2 is supposed to be a "reset button" in certain aspects and part of that reset would be to get those templates back where they belong (customer and checkout).

But again, there is no reason not to commit the deleted files now and maybe refactor them back to the appropiate modules later

[brendanfalkowski]

On topic:
Agreed, start by removing the confusing old templates. If that is approved. Then relocate them to the forms proper location (leaving the persistent sub-templates inside persistent). That makes sense to me.

Off topic:
Tracing is not the issue. Once you know a component exists it's trivial trace to its origin using the tools you mentioned. The problem is having core features that aren't explicitly declared in the layout. Until they're activated by an obscure admin setting or by the right product configuration circumstances it's impossible for a developer to know what a generic container might contain.

This is passable for extensions to drop-in their functionality, but Magento itself should be clearer. If you need to completely restructure the product view then the frustrating ambiguity of generic containers becomes more apparent. It's not a pattern we should strive for when more understandable alternatives exist without significantly more work.

[barbazul]

Totally agreed. Will submit the deleted templates as soon as I get my hands
on a computer
El 27/10/2012 15:41, "Brendan Falkowski" notifications@github.com
escribió:

On topic:
Agreed, start by removing the confusing old templates. If that is
approved. Then relocate them to the forms proper location (leaving the
persistent sub-templates inside persistent). That makes sense to me.

Off topic:
Tracing is not the issue. Once you know a component exists it's trivial
trace to its origin using the tools you mentioned. The problem is having
core features that aren't explicitly declared in the layout. Until they're
activated by an obscure admin setting or by the right product configuration
circumstances it's impossible for a developer to know what a generic
container might contain.

This is passable for extensions to drop-in their functionality, but
Magento itself should be clearer. If you need to completely restructure the
product view then the frustrating ambiguity of generic containers becomes
more apparent. It's not a pattern we should strive for when more
understandable alternatives exist without significantly more work.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/70#issuecomment-9838383.

[magento-team]

@barbazul, @brendanf

Thank you for noting the issue with the templates duplication and performing the actual implementation. We appreciate your work, it made us gather together and think about a good solution.

Unfortunately, we cannot accept your solution and the pull request. The reason is that, if we implement it, then Magento would lose modularity. I.e., removing Mage_Persistent from the application, after the pull request is accepted, would lead to the exceptions, because appropriate templates won't be found by Checkout and Customer modules. With the solution from pull request, Customer and Checkout module start depending on Persistent module. However, it should be vice versa, because persistent shopping cart is an additional feature for Customer and Checkout modules.

So we took our developers to think about a good way to deal with the templates duplication, and we stuck to the generic containers usage, as was proposed above. It will be implemented soon.

The pull request will be closed, this issue will remain open until the implementation is done.

Thank you for the idea and work on this issue.

[barbazul]

@mage2-team Awesome. I totally agree with your argument on modules dependency. I already sort of felt that way but failed to put it into words.

I am really pleased to know that you're working so hard in cutting the remaining ties between modules. I think the whole "Magento Framework" has great potential to build all kind of applications. I can see Magento suite of applications (not only ecommerce) in the not so far away future

[magento-team]

Closing the thread due to no activity.