Skip to content

Commit

Permalink
refactor: remove ENABLE_TAXES_DISPLAY feature flag (#3354)
Browse files Browse the repository at this point in the history
* refactor: migrate ENABLE_TAXES_DISPLAY to posthog

* remove flag

* rename enable_taxes_display and display_taxes
  • Loading branch information
asadali145 authored Jan 7, 2025
1 parent 90ed18a commit 6482515
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 37 deletions.
11 changes: 4 additions & 7 deletions ecommerce/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,21 +157,18 @@ def calculate_tax(
return (0, "", item_price)


def display_taxes(request):
def is_tax_applicable(request):
"""
Returns a boolean to manage the taxes display.
Returns a boolean to indicate whether the user is tax applicable.
Args:
request(HttpRequest): Request object
Returns:
Boolean: True if flag and taxes are enabled for the specific country.
Boolean: True if taxes are enabled for the specific country.
"""
visitor_country = determine_visitor_country(request)
return (
settings.FEATURES.get("ENABLE_TAXES_DISPLAY", False)
and TaxRate.objects.filter(active=True, country_code=visitor_country).exists()
)
return TaxRate.objects.filter(active=True, country_code=visitor_country).exists()


def generate_cybersource_sa_signature(payload):
Expand Down
27 changes: 12 additions & 15 deletions ecommerce/api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
complete_order,
create_coupons,
create_unfulfilled_order,
display_taxes,
enroll_user_in_order_items,
fetch_and_serialize_unused_coupons,
generate_cybersource_sa_payload,
Expand All @@ -52,6 +51,7 @@
get_product_version_price_with_discount,
get_readable_id,
get_valid_coupon_versions,
is_tax_applicable,
latest_coupon_version,
latest_product_version,
make_receipt_url,
Expand Down Expand Up @@ -1866,7 +1866,6 @@ def test_tax_country_and_no_ip_tax(user):

@pytest.mark.parametrize(
(
"is_taxes_display_flag_enabled",
"is_force_profile_country_flag_enabled",
"user_profile_country",
"user_determined_country",
Expand All @@ -1876,18 +1875,17 @@ def test_tax_country_and_no_ip_tax(user):
"expected_taxes_display",
),
[
(True, True, "US", "US", "US", True, True, True),
(True, True, "US", "US", "PK", True, True, False),
(True, False, "US", "US", "US", True, True, True),
(True, False, "PK", "US", "US", True, True, True),
(True, False, "PK", "US", "PK", True, True, False),
(True, False, "US", "US", "US", True, False, False),
(True, False, "PK", "US", "US", True, False, False),
(True, False, "US", "US", "US", False, False, False),
(True, "US", "US", "US", True, True, True),
(True, "US", "US", "PK", True, True, False),
(False, "US", "US", "US", True, True, True),
(False, "PK", "US", "US", True, True, True),
(False, "PK", "US", "PK", True, True, False),
(False, "US", "US", "US", True, False, False),
(False, "PK", "US", "US", True, False, False),
(False, "US", "US", "US", False, False, False),
],
)
def test_display_taxes( # noqa: PLR0913
is_taxes_display_flag_enabled,
def test_is_tax_applicable( # noqa: PLR0913
is_force_profile_country_flag_enabled,
user_profile_country,
user_determined_country,
Expand All @@ -1898,12 +1896,11 @@ def test_display_taxes( # noqa: PLR0913
mocker,
):
"""
Tests that `display_taxes` returns the expected display status.
Tests that `is_tax_applicable` returns the expected display status.
"""
mocker.patch(
"ecommerce.api.determine_visitor_country", return_value=user_determined_country
)
settings.FEATURES["ENABLE_TAXES_DISPLAY"] = is_taxes_display_flag_enabled
settings.ECOMMERCE_FORCE_PROFILE_COUNTRY = is_force_profile_country_flag_enabled
user = UserFactory.create(legal_address__country=user_profile_country)
request = FakeRequest()
Expand All @@ -1912,4 +1909,4 @@ def test_display_taxes( # noqa: PLR0913
if tax_rate_created:
TaxRateFactory.create(country_code=tax_rate_country, active=tax_rate_enabled)

assert display_taxes(request) == expected_taxes_display
assert is_tax_applicable(request) == expected_taxes_display
2 changes: 1 addition & 1 deletion ecommerce/mail_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ def send_ecommerce_order_receipt(order, cyber_source_provided_email=None):
"company": purchaser.get("company"),
"vat_id": purchaser.get("vat_id"),
},
"enable_taxes_display": bool(order["tax_rate"]),
"is_tax_applicable": bool(order["tax_rate"]),
"support_email": settings.EMAIL_SUPPORT,
},
),
Expand Down
4 changes: 2 additions & 2 deletions ecommerce/mail_api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ def test_send_b2b_receipt_email_error(mocker):
)
def test_send_ecommerce_order_receipt(mocker, receipt_data, settings):
"""send_ecommerce_order_receipt should send a receipt email"""
settings.FEATURES["ENABLE_TAXES_DISPLAY"] = False
mocker.patch("ecommerce.api.is_tax_applicable", return_value=False)
patched_mail_api = mocker.patch("ecommerce.mail_api.api")
date = datetime.datetime(2010, 1, 1, 0, tzinfo=datetime.UTC)
user = UserFactory.create(
Expand Down Expand Up @@ -344,7 +344,7 @@ def test_send_ecommerce_order_receipt(mocker, receipt_data, settings):
"company": user.profile.company,
"vat_id": "AT12349876",
},
"enable_taxes_display": False,
"is_tax_applicable": False,
"support_email": settings.EMAIL_SUPPORT,
},
)
Expand Down
6 changes: 3 additions & 3 deletions mail/templates/product_order_receipt/body.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
<br />
Cambridge, MA 02139 USA
<br />
{% if enable_taxes_display %}
{% if is_tax_applicable %}
GSTIN: 9923USA29055OSB
<br />
{% endif %}
Expand Down Expand Up @@ -51,7 +51,7 @@ <h3 style="color: #000000; font-size: 16px; font-weight: 700; line-height: 18px;
{% for line in lines %}
<p>
<strong style="font-weight: 700;">Order Item:</strong> {{ line.content_title }}<br>
{% if enable_taxes_display %}
{% if is_tax_applicable %}
<strong style="font-weight: 700;">HSN:</strong> 9992<br>
{% endif %}
<strong style="font-weight: 700;">Product Number:</strong> {{ line.readable_id }}<br>
Expand All @@ -61,7 +61,7 @@ <h3 style="color: #000000; font-size: 16px; font-weight: 700; line-height: 18px;
<strong style="font-weight: 700;">Quantity:</strong> {{ line.quantity }}<br>
<strong style="font-weight: 700;">Unit Price:</strong> ${{ line.price }}<br>
<strong style="font-weight: 700;">Discount:</strong> {{ line.discount|format_discount }}<br>
{% if enable_taxes_display %}
{% if is_tax_applicable %}
<strong style="font-weight: 700;">Total Before Tax:</strong> ${{ line.total_before_tax|floatformat:2 }}<br>
<strong style="font-weight: 700;">Tax ({{ order.tax_rate|floatformat:"-2" }}%):</strong> ${{ line.tax_paid|floatformat:2 }}<br>
{% endif %}
Expand Down
4 changes: 2 additions & 2 deletions mitxpro/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ def get_js_settings(request: HttpRequest):
Returns:
dict: the settings object
"""
from ecommerce.api import display_taxes
from ecommerce.api import is_tax_applicable

return {
"gtmTrackingID": settings.GTM_TRACKING_ID,
Expand All @@ -602,7 +602,7 @@ def get_js_settings(request: HttpRequest):
"course_dropdown": settings.FEATURES.get("COURSE_DROPDOWN", False),
"webinars": settings.FEATURES.get("WEBINARS", False),
"enable_blog": settings.FEATURES.get("ENABLE_BLOG", False),
"enable_taxes_display": display_taxes(request),
"is_tax_applicable": is_tax_applicable(request),
"enable_enterprise": settings.FEATURES.get("ENABLE_ENTERPRISE", False),
"posthog_api_token": settings.POSTHOG_PROJECT_API_KEY,
"posthog_api_host": settings.POSTHOG_API_HOST,
Expand Down
7 changes: 4 additions & 3 deletions mitxpro/utils_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import pytest
from rest_framework import status

from ecommerce.api import is_tax_applicable
from ecommerce.models import Order
from mitxpro.test_utils import MockResponse
from mitxpro.utils import (
Expand Down Expand Up @@ -455,7 +456,7 @@ def test_request_get_with_timeout_retry(mocker):
assert result == mock_response


def test_get_js_settings(settings, rf, user):
def test_get_js_settings(settings, rf, user, mocker):
"""Test get_js_settings"""
settings.GA_TRACKING_ID = "fake"
settings.GTM_TRACKING_ID = "fake"
Expand All @@ -472,9 +473,9 @@ def test_get_js_settings(settings, rf, user):
settings.DIGITAL_CREDENTIALS_SUPPORTED_RUNS = "test_run1,test_run2"
settings.FEATURES["COURSE_DROPDOWN"] = False
settings.FEATURES["WEBINARS"] = False
settings.FEATURES["ENABLE_TAXES_DISPLAY"] = False
settings.FEATURES["ENABLE_BLOG"] = False
settings.FEATURES["ENABLE_ENTERPRISE"] = False
mocker.patch("ecommerce.api.is_tax_applicable", return_value=False)

request = rf.get("/")
request.user = user
Expand All @@ -494,7 +495,7 @@ def test_get_js_settings(settings, rf, user):
"digital_credentials_supported_runs": settings.DIGITAL_CREDENTIALS_SUPPORTED_RUNS,
"course_dropdown": settings.FEATURES.get("COURSE_DROPDOWN", False),
"webinars": settings.FEATURES.get("WEBINARS", False),
"enable_taxes_display": settings.FEATURES.get("ENABLE_TAXES_DISPLAY", False),
"is_tax_applicable": is_tax_applicable(request),
"enable_blog": settings.FEATURES.get("ENABLE_BLOG", False),
"enable_enterprise": settings.FEATURES.get("ENABLE_ENTERPRISE", False),
"posthog_api_token": settings.POSTHOG_PROJECT_API_KEY,
Expand Down
2 changes: 1 addition & 1 deletion static/js/components/forms/CheckoutForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ export class InnerCheckoutForm extends React.Component<InnerProps, InnerState> {
</span>
</div>
) : null}
{SETTINGS.enable_taxes_display ? (
{SETTINGS.is_tax_applicable ? (
<div>
<div className="bar" />
<div className="flex-row total-before-tax-row">
Expand Down
4 changes: 2 additions & 2 deletions static/js/components/forms/CheckoutForm_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ describe("CheckoutForm", () => {
});
[true, false].forEach((taxDisplayEnabled) => {
it(`handles basket tax details display as per feature flag`, async () => {
SETTINGS.enable_taxes_display = taxDisplayEnabled;
SETTINGS.is_tax_applicable = taxDisplayEnabled;

const inner = await renderForm();

Expand All @@ -136,7 +136,7 @@ describe("CheckoutForm", () => {
it(`Calculates and displayes the tax correctly ${
hasCoupon ? "with" : "without"
} a coupon`, async () => {
SETTINGS.enable_taxes_display = true;
SETTINGS.is_tax_applicable = true;
basket.tax_info.tax_rate = taxRate;

const inner = await renderForm({
Expand Down
2 changes: 1 addition & 1 deletion static/js/flow/declarations.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ declare type Settings = {
course_dropdown: boolean,
webinars: boolean,
enable_blog: boolean,
enable_taxes_display: boolean,
is_tax_applicable: boolean,
enable_enterprise: boolean,
posthog_api_token: ?string,
posthog_api_host: ?string
Expand Down

0 comments on commit 6482515

Please sign in to comment.