Move saving logic outside the View part

[stalniy]

It's necessary that saving logic should be in model or controller, not in block like this

class Mage_Checkout_Block_Onepage_Shipping_Method_Available extends Mage_Checkout_Block_Onepage_Abstract
{
................................................
    public function getShippingRates()
    {

        if (empty($this->_rates)) {
            $this->getAddress()->collectShippingRates()->save();
..........................................................

Because it's not clear (hidden) logic. Blocks should be used only for View part of MVC.

[colinmollenhour]

Definitely agree here. I and another person I know have both been bothered with tracking down this mysterious save occurring unexpectedly.

[magento-team]

@stalniy
Thank you for raising the issue.
Brief investigation revealed, that there are quite a few places in the checkout implementation, which contain business logic in blocks. So, it does not make much sense to fix this particular place and leave similar occurrences intact. At the same time, moving out all checkout business logic from blocks seems to be a pretty complex task.

The issue has been converted into the ticket in the internal tracking system. However, the improvement seems to be completely developer-oriented without clear business value, which significantly lowers its priority. Providing a pull request with appropriate changes along with corresponding tests may help to expedite the changes.

Leaving the ticket open, upon any activity in the internal ticket, corresponding update will be posted to the GitHub.

[magento-team]

Closing the ticket for now, it will be reopened upon an activity on the corresponding internal ticket.