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

✨(backend) add offer and price fields to courseRun #2466

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JoaoGarcao
Copy link
Contributor

Add offer and price fields to courseRun displayed at admin view

related to #2445

1º task requested on issue #2451

@JoaoGarcao JoaoGarcao force-pushed the JoaoGarcao/issue/2451/model-add-price branch from d08ad9e to dde2d06 Compare July 29, 2024 09:21
@@ -623,6 +623,8 @@ def test_models_course_run_mark_dirty_any_field(self):
stub = CourseRunFactory(
sync_mode="manual",
catalog_visibility=CourseRunCatalogVisibility.COURSE_ONLY,
offer=CourseRunOffer.SUBSCRIPTION,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@JoaoGarcao You have to use the setting RICHIE_DEFAULT_COURSE_RUN_OFFER that you have added to to the settings.py

@igobranco
Copy link
Collaborator

@JoaoGarcao to fix the check-changelog CI check you have to add a line to the CHANGELOG.md file on the Unreleased Added section.

Copy link
Member

@jbpenrath jbpenrath left a comment

Choose a reason for hiding this comment

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

Do not forgot to add an entry into the changelog.

src/richie/apps/courses/models/course.py Outdated Show resolved Hide resolved
@@ -75,6 +75,8 @@ class Meta:
"state",
"enrollment_count",
"catalog_visibility",
"offer",
"price",
Copy link
Member

Choose a reason for hiding this comment

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

price field could be returned as a float not a string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made a change to this, but I'm not sure if it's in line with what you asked for.

src/richie/apps/courses/models/course.py Outdated Show resolved Hide resolved
Comment on lines 602 to 620
RICHIE_DEFAULT_COURSE_RUN_OFFER = values.Value(
"free",
environ_name="RICHIE_DEFAULT_COURSE_RUN_OFFER",
environ_prefix=None,
)

# Course run price currency value that would be shown on course detail page
RICHIE_DEFAULT_COURSE_RUN_PRICE_CURRENCY = values.Value(
"EUR",
environ_name="RICHIE_DEFAULT_COURSE_RUN_PRICE_CURRENCY",
environ_prefix=None,
)

# Course run price value that would be shown on course detail page
RICHIE_DEFAULT_COURSE_RUN_PRICE = values.Value(
0.0,
environ_name="RICHIE_DEFAULT_COURSE_RUN_PRICE",
environ_prefix=None,
)
Copy link
Member

Choose a reason for hiding this comment

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

RICHIE_DEFAULT_COURSE_RUN_OFFER and RICHIE_DEFAULT_COURSE_RUN_PRICE look not used. Furthermore, I wonder if this is really useful to be able to override to default values through a settings. WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree that the RICHIE_DEFAULT_COURSE_RUN_PRICE shouldn't be on a setting.
@JoaoGarcao the RICHIE_DEFAULT_COURSE_RUN_OFFER should be included only on the next PR that you are working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change done.

@JoaoGarcao JoaoGarcao force-pushed the JoaoGarcao/issue/2451/model-add-price branch 2 times, most recently from 84083cc to 78b87af Compare August 2, 2024 15:22
Add offer and price fields to courseRun displayed at admin view

related to openfun#2445
@JoaoGarcao JoaoGarcao force-pushed the JoaoGarcao/issue/2451/model-add-price branch from 78b87af to 8ee54a1 Compare August 6, 2024 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants