Skip to content
This repository was archived by the owner on May 6, 2024. It is now read-only.

SE-2298 Makes the ecommerce OSCAR_DEFAULT_CURRENCY configurable. - #5757

Merged
pomegranited merged 1 commit into
openedx-unsupported:masterfrom
open-craft:samuel/configure-default-currency
Jul 22, 2020
Merged

SE-2298 Makes the ecommerce OSCAR_DEFAULT_CURRENCY configurable.#5757
pomegranited merged 1 commit into
openedx-unsupported:masterfrom
open-craft:samuel/configure-default-currency

Conversation

@samuelallan72

@samuelallan72 samuelallan72 commented Apr 22, 2020

Copy link
Copy Markdown

(cherry picked from commit 436092e)

This allows overriding the default currency in ecommerce during deployment. Useful for instances where the desired currency isn't USD. Originally introduced in open-craft#119

Jira tickets: OSPR-4409

Dependencies: For this to successfully configure ecommerce, ecommerce requires the changes in https://github.com/edx/ecommerce/pull/2916

Sandbox URL:

Merge deadline: None

Test instructions:

  1. Set ECOMMERCE_OSCAR_DEFAULT_CURRENCY and deploy a new appserver using this branch and the ecommerce branch from https://github.com/edx/ecommerce/pull/2916.
  2. Ensure that the ecommerce settings.OSCAR_DEFAULT_CURRENCY matches the setting.

Reviewers:

Settings:

SANDBOX_ENABLE_ECOMMERCE: yes
ECOMMERCE_OSCAR_DEFAULT_CURRENCY: "GBP"
ECOMMERCE_VERSION: samuel/override-default-currency
ECOMMERCE_REPOS:
  - PROTOCOL: "{{ COMMON_GIT_PROTOCOL }}"
    DOMAIN: "{{ COMMON_GIT_MIRROR }}"
    PATH: "open-craft"
    REPO: "ecommerce.git"
    VERSION: "{{ ECOMMERCE_VERSION }}"
    DESTINATION: "{{ ecommerce_code_dir }}"
    SSH_KEY: "{{ ECOMMERCE_GIT_IDENTITY }}"

@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @swalladge! I've created OSPR-4409 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

Feel free to add as much of the following information to the ticket:

  • supporting documentation
  • edx-code email threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will still be done via the GitHub pull request interface. As a reminder, our process documentation is here.

@natabene

Copy link
Copy Markdown
Contributor

@swalladge Thank you for your contribution. Please let me know once it is ready for our review.

@openedx-webhooks openedx-webhooks added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed needs triage labels Apr 22, 2020
@pomegranited

Copy link
Copy Markdown
Contributor

👍

  • I tested this using the PR instructions on the sandbox created for this issue which was built with the following configuration variables:

    SANDBOX_ENABLE_ECOMMERCE: yes
    ECOMMERCE_OSCAR_DEFAULT_CURRENCY: "GBP"
    ECOMMERCE_VERSION: samuel/override-default-currency
    ECOMMERCE_REPOS:
      - PROTOCOL: "{{ COMMON_GIT_PROTOCOL }}"
        DOMAIN: "{{ COMMON_GIT_MIRROR }}"
        PATH: "open-craft"
        REPO: "ecommerce.git"
        VERSION: "{{ ECOMMERCE_VERSION }}"
        DESTINATION: "{{ ecommerce_code_dir }}"
        SSH_KEY: "{{ ECOMMERCE_GIT_IDENTITY }}"
  • I read through the code, and checked it against original Ironwood SE-1617, SE-2298 Makes the ecommerce OSCAR_DEFAULT_CURRENCY configurable. open-craft/configuration#119, which is deployed to a client and working.

  • I checked for accessibility issues N/A

  • Includes documentation N/A

  • I made sure any change in configuration variables is reflected in the corresponding
    client's configuration-secure repository.
    N/A

@samuelallan72

Copy link
Copy Markdown
Author

@natabene, this is ready for edX review. :)

@natabene

Copy link
Copy Markdown
Contributor

@swalladge Thank you for letting me know. @fredsmith I assume you will want to wait until edx/ecommerce#2916 merges?

@openedx-webhooks openedx-webhooks added awaiting prioritization and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Apr 24, 2020
@pomegranited

Copy link
Copy Markdown
Contributor

@feanil Could my review here count as a CC review? We can get another OpenCraft internal review if needed too, just let me know. CC @nedbat

@feanil

feanil commented Jul 21, 2020

Copy link
Copy Markdown
Contributor

@pomegranited I think your review here can count as a CC review so feel free to merge.

@pomegranited
pomegranited merged commit 623b7ee into openedx-unsupported:master Jul 22, 2020
@openedx-webhooks

Copy link
Copy Markdown

@swalladge 🎉 Your pull request was merged!

Please take a moment to answer a two question survey so we can improve your experience in the future.

@pomegranited
pomegranited deleted the samuel/configure-default-currency branch July 22, 2020 06:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants