Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getDestCity() method returns nothing in collectRates() method. #3789

Closed
Selk opened this issue Mar 15, 2016 · 42 comments
Closed

getDestCity() method returns nothing in collectRates() method. #3789

Selk opened this issue Mar 15, 2016 · 42 comments
Assignees
Labels
bug report Component: Checkout Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development

Comments

@Selk
Copy link

Selk commented Mar 15, 2016

As the title says, the getDestCity() method returns absolutely nothing if I use it inside collectRates() method.

I tried like this :

$city = $request->getDestCity(); $street = $request->getDestStreet(); $postCode = $request->getDestPostcode();`

But it didn't worked ($city and $street are empty, not $postCode).

I think it might be a bug because it worked perfectly fine on magento 1.9.

@rgoncharuk rgoncharuk added the CS label Mar 17, 2016
@Selk
Copy link
Author

Selk commented Mar 22, 2016

Just a little adding :

I just entered the checkout page and filled the information.

In collectRates method (Magento 1.9.4) (with user city as Paris) :
$rate->setMethodTitle($request->getDestCity()); => Paris

In collectRates method (Magento 2.0.1) (with user city as Paris) :
$rate->setMethodTitle($request->getDestCity()); => nothing

@matthew-muscat
Copy link
Contributor

Seeing the same issue on our end here too - getDestCity is null and never contains a value in the carrier request.

An ajax request during checkout calls "rest/default/V1/guest-carts/KEY/estimate-shipping-methods
", however the city or street details are not passed over in the request payload

{address: {country_id: "AU", region_id: "526", region: "Victoria", postcode: "3016"}}

@matthew-muscat
Copy link
Contributor

It should be noted that street level details are also not available.

This data can be useful in the carrier request - an example of it's usage could be for logic that may need to handle PO Boxes differently.

@wsagen
Copy link

wsagen commented Apr 6, 2016

The checkout shipping method rates request is passing through the getEstimatedRates function which limits the fields available in the request.
The full address needs to be available for use cases like table rates that are configured down to city/town/suburb level, PO box address logic (as mentioned above), logic to calculate nearest dispatch point for rate calculation.
I don't believe the cart estimate logic should be re-used at checkout.

@oshmyheliuk
Copy link
Contributor

I tried to reproduce this issue, but everything works correct.
On the checkout page inside method collectRates() $request object contains all info that was set in
Magento\Quote\Model\Quote\Address::requestShippingRates.
Request contain city, street and you can get them by appropriate method.

Please give additional info or detailed steps for reproducing this issue.

@wsagen
Copy link

wsagen commented Apr 23, 2016

If you go to any enabled shipping method e.g. DHL or UPS, and look at the request object passed to collectRates when in the checkout on Guest Checkout - getDestCity(), getDestStreet() return null - they are not populated on the request object.
I've tested on v2.0.4 release of community.

@alena-marchenko
Copy link

Hi,

we've created MAGETWO-52308 internal ticket regarding this issue

@alena-marchenko alena-marchenko added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report labels Apr 26, 2016
@cpartica
Copy link
Contributor

cpartica commented Apr 26, 2016

I think this is the expected behavior, even more on a cart guest estimation

When doing an estimation there's not city just zip code, region and country
However when you go to checkout, final page that already has your info, only then you could get your destination details such as city and street

sample from 2.0.4
Checkout
url : http://magento.git/magento2-2.0.4/checkout/#payment
ajax request: rest/default/V1/guest-carts/434cd658bffb9fd9ee0e9fd0b703d303/shipping-information

screen shot 2016-04-26 at 5 04 20 pm

Estimate:
url : http://magento.git/magento2-2.0.4/checkout/cart/
ajax request: rest/default/V1/guest-carts/434cd658bffb9fd9ee0e9fd0b703d303/estimate-shipping-methods

screen shot 2016-04-26 at 5 14 17 pm

When exactly do you expect to get that info? at what stage in the process
It seems to me like for the guest checkout, only at the last step before pay you could do that..
we need more details in order to fix this bug

@matthew-muscat
Copy link
Contributor

Hi @cpartica,

Cart estimation is understandable, as there is no city input field, however this issue is occurring during checkout too (during the shipping step), preventing us from providing live quoting in a shipping module.

In our case we're using getDestCity() in our carrier "collectRates" method to present shipping methods based on a live quote.

The estimate-shipping-methods call is made once the address details have been filled in, however the request does not include all inputs, such as postcode, city and street address. This results in the Carrier RateRequest object not containing these details.

Our live quoting integration requires the city and postcode details, however as we are unable to get this data, we're unable to create a shipping method via a live quote

It should be noted that street level details are also not available. While not used in our specific case, this data can be useful for some implementations - an example of it's usage could be for logic that may need to handle PO Boxes differently.

@wsakaren
Copy link

wsakaren commented May 9, 2016

Needs to be fixed ASAP, is affecting rollout of Magento 2 to clients.

@pboisvert
Copy link

@ilol Illia--as shipping PM owner please investigate this ticket

@webtonic
Copy link

webtonic commented May 9, 2016

Hi has this bug been fixed yet?

@maksek maksek assigned cpartica and unassigned alena-marchenko May 9, 2016
@maksek
Copy link
Contributor

maksek commented May 9, 2016

@webtonic , @wsakaren - at the very moment we are fixing it.

@webtonic
Copy link

webtonic commented May 9, 2016

Thanks! Appreciated!

@maksek maksek removed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development question labels May 9, 2016
@cpartica
Copy link
Contributor

cpartica commented May 9, 2016

Where exactly do you plug-in to our checkout flow to get the live quotes?
I think the problem is that the checkout flow doesn't have the selected customer address in the main request, as all is controlled by javascript, by making rest calls with selected info, and updating dynamically the page, producing a final place order request that only at that point you'll have that info

So if you're extending a block or anything that' son the main request for http://yourdomain/checkout you will never have access to that info

To get live quoting for shipping you have to use knockout js models that subscribe themselves or extend current ones
There are certain events that you can use (js is splited into model views and actions)
Magento_Checkout/js/model/shipping-rate-processor/customer-address.js
Magento_Checkout/js/model/shipping-rate-service.js
Magento_Checkout/js/action/select-shipping-address.js
Magento_Checkout/js/view/shipping.js

Not sure if you're using this aproach to get your quotes, but don't rely that the main request will have anyting, because all things in checkout are client-side now

We have to modify @method string getDestCity() to @method string|null getDestCity() because this situation is possible

Hope this answer helps

@wsagen
Copy link

wsagen commented May 10, 2016

A custom shipping carrier shouldn't require any block or front end modification at all, assuming it intends to only return shipping rates.

If we ignore any custom shipping methods for a second and just look at core Magento 2 - consider the DHL carrier that is built in to Magento. In the Carrier.php in module-dhl:
->setDestStreet($this->string->substr(str_replace("\n", '', $request->getDestStreet()), 0, 35))
->setDestStreetLine2($request->getDestStreetLine2())
->setDestCity($request->getDestCity())

  • In both cart and checkout the city and street will always be null as it is not set on the request object
  • This same request is passed to collectRates function of every carrier that is active in Magento.
  • We expect city and street to be null when shipping rates are requested from the cart, but in the checkout these fields are available for both guest and logged in customers as the shipping rates request is triggered via Ajax after the shipping address form has been filled out for guests, or an address has been selected for logged in customers

As your built in DHL carrier is requiring these fields in the request, I would say this is a bug with core code at this point.

In addition, any custom shipping rate carriers would fully expect the full shipping address fields to be available on the shipping rate request as they have been filled out by the customer and are often crucial to accurate shipping rates. These were available in Magento 1, so at this point it's a step backwards.

Guest checkout: the fix required appears to be in
module-checkout/view/frontend/web/model/js/shipping-rate-processor/new-address.js
-> add the additional address fields to the payload
-> you are re-using the service URLs that are used in the cart to estimate rates which doesn't seem correct to me. This will eventually call estimateByAddress function in ShippingMethodManagement which only expects and adds region, country and zip code to the request. I think you should have a separate serviceURL at this point that will call a function that expects the full address payload and adds to the request.

Logged in customer checkout: the fix required appears to be in
module-checkout/view/frontend/web/model/js/shipping-rate-processor/customer-address.js
-> again you are re-using the service URLs from the cart, which will call estimateByAddressId function in ShippingMethodManagement which only adds the region, zip code and country to the request. I think you should use a separate service URL to call a function that adds the full address to the request. You have the shipping address via the Id so all of the shipping address fields are available.

@cpartica
Copy link
Contributor

That's a better info and thank you for that, very valuable
We are investigating how can we improve this by rewriting some interfaces, so you can have a better easier access to full shipping data

@wsakaren
Copy link

You need to fix it ASAP. Big issue.

@slavvka
Copy link
Member

slavvka commented May 14, 2016

@Selk, the fix has been delivered to mainline. Please see d10867f, d04a439.

@mikeweis mikeweis closed this as completed Aug 5, 2016
@rodrigowebjump
Copy link
Member

Hi guys.

Which version of EE has this fix?

Regards

@wsakaren
Copy link

Magento checkout is broken for pretty much everywhere apart from basic US needs because of this. Is anyone using M2 outside the US? If so they must have flat rate shipping switched on....

@pratikmage
Copy link

i try in magento 2.1.2 check Magento\Quote\Model\Quote\Address\RateRequest $request and get $request->getDestCity();
it's return null for every shipping method i check magento default Tablerate shipping metho it's not work.

@joni-jones
Copy link
Contributor

joni-jones commented Dec 27, 2016

@pratikmage, this fix available only for develop branch and is not available for Magento 2.0.x and 2.1.x.

Backport will be published in Magento 2.1.5 version.

@shahankitb997
Copy link

shahankitb997 commented Dec 28, 2016

HI @joni-jones Can you please provide the solution for now? What needs to be change? So for now we can put the fix. As our website is Live & facing this issue. We can't wait for version release.

Hope u understand.

http://magento.stackexchange.com/questions/151288/magento-2-dhl-api-hong-kong-address-issue-due-to-quote-session

@pratikmage
Copy link

HI @joni-jones pls check this module i fix it using this module.
https://github.com/shopgo-magento2/checkout-city

@joni-jones
Copy link
Contributor

@shahankitb997, you can compare \Magento\Quote\Model\ShippingMethodManagement and \Magento\Quote\Model\GuestCart\GuestShippingMethodManagement on develop with your 2.1.x version and try to apply changes. These both services now use estimateByExtendedAddress instead estimateByAddress.

@joni-jones
Copy link
Contributor

@pratikmage, this module similar to our fix, but uses deprecated \Magento\Quote\Model\ShippingMethodManagement::getEstimatedRates method.

@shahankitb997
Copy link

Thanks @pratikmage

@jchesko
Copy link

jchesko commented Feb 10, 2017

@mikeweis - Any idea when MAGETWO-55117 will make it into a release? We keep having to manually patch after each upgrade.

@joni-jones
Copy link
Contributor

joni-jones commented Feb 10, 2017

@jchesko, this fix should have been available in Magento 2.1.5 release but moved to 2.1.8.

@Zach2212
Copy link

Hi. Is there a temporary fix for this?

@pratikmage
Copy link

HI @Zach2212 pls check this module i fix it using this module.
https://github.com/shopgo-magento2/checkout-city

@horatiubrinza
Copy link

I've created a plugin that fixes this issue, by overriding Magento\Quote\Model\ShippingMethodManagement::estimateByAddressId(). You can have a look at the code here: http://pastebin.com/3GcesL2H

@edudeleon
Copy link

Hello @joni-jones, do you know if the fix for this issue was released in Magento 2.1.8?
I don't see it in the release notes: http://devdocs.magento.com/guides/v2.1/release-notes/ReleaseNotes2.1.8CE.html

@joni-jones
Copy link
Contributor

Hi, @edudeleon, unfortunately, the fix moved again to 2.1.10. I notified our managers that fix should be delivered ASAP.

@maderlock
Copy link

This is actually in the release notes for 2.1.10, so perhaps it's finally happened!

@magento-engcom-team magento-engcom-team added the Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed label Nov 8, 2017
@samanaveen
Copy link

Hi team,
We are using M2.2.2. Still, we are facing this issue. Please provide me a temporary fix.

checkout issue

@samanaveen
Copy link

I have modified below file. now it is working fine.
vendor/magento/module-checkout/view/frontend/web/js/model/shipping-save-processor/default.js

var payload;

            var street = [];
            var fname = $('[name="firstname"]').val();
            var lname = $('[name="lastname"]').val();
            var street0 = $('[name="street[0]"]').val();
            var street1 = $('[name="street[0]"]').val();
            var city = $('[name="city"]').val();
            var telephone = $('[name="telephone"]').val();
            
            street.push(street0);
            street.push(street1);
            var shipngadss = quote.shippingAddress();

            shipngadss['firstname'] = fname;
            shipngadss['lastname'] = lname;
            shipngadss['street'] = street;
            shipngadss['city'] = city;
            shipngadss['telephone'] = telephone;

            console.log('magestore-m');

            if (!quote.billingAddress() || BillingAddressState.sameAsShipping() == true) {
                selectBillingAddressAction(shipngadss);
            }

        payload = {
            addressInformation: {
                'shipping_address': shipngadss,
                'billing_address': quote.billingAddress(),
                'shipping_method_code': quote.shippingMethod()['method_code'],
                'shipping_carrier_code': quote.shippingMethod()['carrier_code']
            }
        };

        payloadExtender(payload);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Component: Checkout Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Development

No branches or pull requests