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

Free shipping doesn't work as advertised #597

Closed
gldraphael opened this issue Sep 15, 2021 · 5 comments
Closed

Free shipping doesn't work as advertised #597

gldraphael opened this issue Sep 15, 2021 · 5 comments
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@gldraphael
Copy link

image

@Shabirmean Shabirmean added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Sep 15, 2021
@NimJay
Copy link
Collaborator

NimJay commented Sep 21, 2021

I think this "free shipping" promise is worth fixing.

BOGO Ads

BOGO = "Buy One Get One"
Somewhat related to this are these other "buy x, get (x+1)th free" false advertisements that show up at the bottom of product pages:
Screen Shot 2021-09-21 at 10 46 08 AM

There's 3 such ads in total (in AdService.js).
If I'm not mistake, these ads are supposed to mimic third-party (external) ads — we just happen to link them to Online Boutique products. So I don't think it's worth fixing. If anything, we could change the wording.

@NimJay NimJay added priority: p3 Desirable enhancement or fix. May not be included in next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Nov 19, 2021
@NimJay
Copy link
Collaborator

NimJay commented Apr 12, 2022

This is a good beginner issue. :)
You can see related code in a slightly similar issue that was fixed, https://github.com/GoogleCloudPlatform/microservices-demo/pull/694/files.
We could also add a unit test for this.
Please ignore my previous comment about the BOGO ads — the BOGO ads aren't worth fixing.

CC: @charlieyu1996

@NimJay
Copy link
Collaborator

NimJay commented Apr 13, 2022

After talking to @charlieyu1996 about this issue, we also thought of deleting the "free shipping" message altogether.
Reasons:

  1. Removing code is one of the most productive things we can do as developers. Less maintenance for the future.
  2. I don't think any demos or tutorials rely on its existence. I did a quick skim of about 5 Google tutorials (see Google-internal list) that lists some of these demos/tutorials.
  3. The "free shipping" message doesn't really add a lot of value to Online Boutique as a demo app.

I think this removal will involve:

@charlieyu1996
Copy link
Contributor

TLDR: I do agree with Nim here.

For context:
The issue is in shippingservice/quote.go, we call CreateQuoteFromCount , which is the legacy logic that uses the number of items from the cart to calculate the shipping fee. Since we changed the logic, the count integer is not useful anymore (We can deprecate it or change it to accept a total cart price).

Now, the issue is, the function GetQuote which calls CreateQuoteFromCount. And in GetQuote ,we don't have the cart price according to pb.GetQuoteRequest. From my understanding, in order to get a product price, we need to traverse through the cart and calculate the price for each product.

Removing the quote logic would be a sensible approach since it does not add value to the demo.

@charlieyu1996
Copy link
Contributor

PR merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

4 participants