Skip to content

Further theme support for ECOMMERCE_* settings #11966

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.

Sign up for GitHub

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

Closed
e-kolpakov wants to merge 2 commits into from
Closed

Further theme support for ECOMMERCE_* settings #11966

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b53a12b
Enabling Otto checkout for themed/microsite sites
e-kolpakov Mar 28, 2016
150ced3
Fixing ecommerce_api+client to use themed/microsite config
e-kolpakov Mar 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion lms/djangoapps/commerce/tests/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,14 @@ def test_is_enabled(self):

@patch('openedx.core.djangoapps.theming.helpers.is_request_in_themed_site')
def test_is_enabled_for_microsites(self, is_microsite):
"""Verify that is_enabled() returns False if used for a microsite."""
"""Verify that is_enabled() returns True when ecomm checkout is enabled for microsite """
is_microsite.return_value = True
is_enabled = EcommerceService().is_enabled(self.user)
self.assertTrue(is_enabled)

config = CommerceConfiguration.current()
config.checkout_on_ecommerce_service = False
config.save()
is_not_enabled = EcommerceService().is_enabled(self.user)
self.assertFalse(is_not_enabled)

Expand Down
3 changes: 1 addition & 2 deletions lms/djangoapps/commerce/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ def __init__(self):

def is_enabled(self, user):
""" Check if the user is activated, if the service is enabled and that the site is not a microsite. """
return (user.is_active and self.config.checkout_on_ecommerce_service and not
helpers.is_request_in_themed_site())
return user.is_active and self.config.checkout_on_ecommerce_service

@mattdrayer mattdrayer Mar 28, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@e-kolpakov -- I think if we were to merge this line in, all microsites would begin using the Otto checkout process, correct? This seems like something that we should avoid at the present time because we are not ready to enable Otto for the microsites running on edx.org. I discussed with @mjfrey and @douglashall, and we are thinking that this should be a separate PR to master. Alternatively, we could add a site_id to the commerce configuration model and that would allow for multiple microsites to control their own setups. Thoughts?

@e-kolpakov e-kolpakov Mar 28, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattdrayer Right, I missed that commerce config is per-instance as well, not per-microsite. I believe the best course of actions would be to push for model-backed site configuration, but if it's not something we could achieve within the time bounds we have (i.e. before the WL-analytics launch), adding site_id would be an appropriate solution.

Alternatively, this setting can live in microsite config as a separate flag ("USE_OTTO") or as an implicit switch (i.e. if ECOMMERCE_PUBLIC_URL_ROOT is set).

@mattdrayer mattdrayer Mar 28, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have model-backed config for Sites waiting in the wings (WL-327 / #11806) -- we will be picking it up as soon as WL-325 / #11885 clears its gates.

@e-kolpakov e-kolpakov Mar 28, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattdrayer ok, than I believe it is safe to close this PR (I'll do that) and just wait for the SiteConfiguration PR to land and implement it in a proper way.


def payment_page_url(self):
""" Return the URL for the checkout page.
Expand Down
11 changes: 8 additions & 3 deletions openedx/core/djangoapps/commerce/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from edx_rest_api_client.client import EdxRestApiClient
from eventtracking import tracker

from openedx.core.djangoapps.theming.helpers import get_value

ECOMMERCE_DATE_FORMAT = "%Y-%m-%dT%H:%M:%SZ"


Expand All @@ -23,14 +25,17 @@ def is_commerce_service_configured():
Return a Boolean indicating whether or not configuration is present to use
the external commerce service.
"""
return bool(settings.ECOMMERCE_API_URL and settings.ECOMMERCE_API_SIGNING_KEY)
return bool(
get_value('ECOMMERCE_API_URL', settings.ECOMMERCE_API_URL) and
get_value('ECOMMERCE_API_SIGNING_KEY', settings.ECOMMERCE_API_SIGNING_KEY)
)


def ecommerce_api_client(user):
""" Returns an E-Commerce API client setup with authentication for the specified user. """
return EdxRestApiClient(
settings.ECOMMERCE_API_URL,
settings.ECOMMERCE_API_SIGNING_KEY,
get_value('ECOMMERCE_API_URL', settings.ECOMMERCE_API_URL),
get_value('ECOMMERCE_API_SIGNING_KEY', settings.ECOMMERCE_API_SIGNING_KEY),
user.username,
user.profile.name if hasattr(user, 'profile') else None,
user.email,
Expand Down