Skip to content
This repository has been archived by the owner on Dec 11, 2022. It is now read-only.

Provide service name in getAllowedMethods function #23

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

micwallace
Copy link

Carrier name is currently provided as the value for every service in the getAllowedMethods function. This patch corrects this so that the service name is returned instead.

Carrier name is currently provided as the value for every service in the getAllowedMethods function. This patch corrects this so that the service name is returned instead.
@chnorton
Copy link
Member

Looks good @micwallace! Just had one request before I merge it in - could you have it fall back to the getConfigData value in case getCode returns null?

@micwallace
Copy link
Author

Hi @chnorton! I think it would be better to exclude the method if it doesn't exist in the config any more. Or possibly show the service code instead. Which one do you think?

@chnorton
Copy link
Member

@micwallace it shouldn't be very common for a method to still be in the allowed list, but not have any config. So in this case just showing the service code probably makes sense.

Fallback to method code when method name isn't found in the config.
@micwallace
Copy link
Author

Hi @chnorton,

I've made the requested modification.

I've also come across another problem which I think should be corrected. In AustraliaPost.php there is the following code:


    /**
     * Simplifies creating a new shipping method.
     *
     * @param string $code
     * @param string $title
     * @param string $price
     * @return Method
     */
    protected function createMethod($code, $title, $price)
    {
        // format the method code
        $code = strtolower($code);
        $code = str_replace("parcel", "", $code);
        $code = str_replace("packaging", "", $code);
        $code = str_replace("signature_on_delivery", "sod", $code);
        $code = str_replace("extra_cover", "exc", $code);
        $code = str_replace("sms_track_advice", "std", $code);
        $code = str_replace("_", "", $code);

        $method = $this->_rateMethodFactory->create();
        $method->setCarrier($this->_code);
        $method->setCarrierTitle($this->getConfigData('title'));
        $method->setMethod($code);
        $method->setMethodTitle($title);
        $method->setPrice($this->getFinalPriceWithHandlingFee($price));
        $method->setCost($this->getFinalPriceWithHandlingFee($price));

        return $method;
    }

This is problematic since the transformations performed on the method code means that it no longer maps correctly to a valid Australia post service code. What was the logic behind this? Is there a dependency on this format?

The Fontis Australia extension for Magento 1 does not make those transformations. It would be helpful to add an option to keep the standard service code when other service options aren't used.

We use the official Aus. Post code to map Magento methods to shipping methods in our ERP system. I think other people would also require an exact method code to match which external systems.

I am happy to create a pull request to create that option if you think other people would benefit.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants