Stanford paid course registration #1118

jbau · 2013-09-25T00:40:18Z

With tests, some settings changes
(all should default to not breaking anything for edx)

Added styling for shopping cart User Experience

Styled shoppingcart list page
Styled navigation shopping cart button
Styled receipt page
Styled course about page for shopping cart courses

@dianakhuang and @talbs for the styling/small bit of front-end.

rake coverage at 100%, rake quality at 100%

jbau · 2013-09-25T00:55:11Z

To give a bit of background context, we are going to be running this for Stanford Med School's Continuing Medical Education (CME) program. The students must pay in order to enroll in courses and access content.

To set the prices, we'll just create "honor" CourseModes with minimum prices, and use that to create PaidCourseRegistration cart item.

jbau · 2013-09-25T00:56:54Z

lms/djangoapps/courseware/views.py

This might be the best place to start review? Basically the course_about page will have an "add-to-cart" button instead of a register button.

talbs · 2013-09-25T14:00:15Z

@jbau, thanks for the ping on this. I've chimed in on your front-end code, but would also love to see a local instance of this. Can you list out some simple steps to turn on the features needed and progress through the flows (in order to see all of the views you worked on)?

Thanks for the help and nice job getting this up and running.

dianakhuang · 2013-09-25T14:37:23Z

lms/djangoapps/courseware/views.py

I know that this isn't part of this pull request, but I think part_of_order is a confusing name. contained_in_order? order_contains?

jzoldak · 2013-09-25T18:34:08Z

lms/envs/test.py

This is also set further up in test.py, pls remove one reference.

Also it's set twice in dev.py, can you pls remove one from there too.

Duh on me. Thanks for noting.

On Sep 25, 2013, at 11:34 AM, Jay Zoldak notifications@github.com wrote:

In lms/envs/test.py:

@@ -165,7 +165,7 @@
###################### Payment ##############################3

Enable fake payment processing page

MITX_FEATURES['ENABLE_PAYMENT_FAKE'] = True

+MITX_FEATURES['ENABLE_SHOPPING_CART'] = True
This is also set further up in test.py, pls remove one reference.

—
Reply to this email directly or view it on GitHub.

jbau · 2013-09-26T02:57:23Z

@dianakhuang @talbs
I think @caesar2164 and I have address most (all?) of your comments. Mind taking another look see?

talbs · 2013-09-26T12:41:28Z

@jbau and @caesar2164, thanks for the discussion and extra revisions.

After previewing the UI you're adding for your needs locally, I noticed there were some visual inconsistencies (button styles for the newly added cart button, add to cart/added in cart buttons on course detail view) and more details (typography, alignment, information hierarchy) could be polished on the receipt page itself. These shouldn't stop this PR, as you folks are trying to get a first pass out and the single group using these now - so your design standards and timetable are most important to meet.

In the event that we on the east coast get involved with them in the future, I'd want to work further on some of those details. I hope you're okay with that.

👍

dianakhuang · 2013-09-26T14:34:29Z

common/djangoapps/student/views.py

I didn't catch this before, but I'm not sure it makes sense to push this through the views. Would it be possible to do the same sort of error handling in the models? Fat models, skinny views, etc.

I'm not quite sure I understand the point you're objecting to here. This particular line of code is calling another view (in shoppingcart).

Do you mean that it should be calling a model function instead?
I have it calling the shopping cart view for code reuse (otherwise it'd have to have all the cases of shoppingcart.views.add_course_to_cart). Also note that in the student app views functions wrap other views all the time (including this function getting called by try_change_enrollment).

Or do you mean that all the clauses in shoppingcart.views.add_course_to_cart should themselves be in the model? In that case I don't think the model function should be returning HttpResponse and its relatives, which doesn't seem like right layer to me.

Or do you mean that the model function PaidCourseRegistration.add_to_order should be raising various subtypes of InvalidCartItem, and shoppingcart.views.add_course_to_cart should be handling those by translating them to HttpResponse codes? I think I'm most amenable to that, though it then doesn't change anything about this particular call.

dianakhuang · 2013-09-26T14:44:52Z

I think other than that one comment, I'm good with this. I'd really like @chrisndodge to give the thumbs up as well, since he has an eye on maybe using it for our own purposes in the future.

chrisndodge · 2013-09-26T15:37:29Z

@dianakhuang thanks for the consideration. My interests are mainly in terms as to the applicability of this feature to other projects. I've been chatting with Jason and others out-of-band, but I'm fine with the current implementation and it's something we might want to build upon.

jbau · 2013-09-26T16:05:34Z

Thanks for your review, @talbs. We would absolutely welcome your further involvement.

Jason

On Sep 26, 2013, at 5:41 AM, Brian Talbot notifications@github.com wrote:

@jbau and @caesar2164, thanks for the discussion and extra revisions.

After previewing the UI you're adding for your needs locally, I noticed there were some visual inconsistencies (button styles for the newly added cart button, add to cart/added in cart buttons on course detail view) and more details (typography, alignment, information hierarchy) could be polished on the receipt page itself. These shouldn't stop this PR, as you folks are trying to get a first pass out and the single group using these now - so your design standards and timetable are most important to meet.

In the event that we on the east coast get involved with them in the future, I'd want to work further on some of those details. I hope you're okay with that.

—
Reply to this email directly or view it on GitHub.

ormsbee · 2013-09-26T17:08:56Z

common/djangoapps/course_modes/models.py

The brackets are unnecessary.

ormsbee · 2013-09-26T21:48:11Z

Other than the above comments, I'm good with this. Thank you.

@dave

With tests, some settings changes (all should default to not breaking anything for edx) Added styling for shopping cart User Experience - Styled shoppingcart list page - Styled navigation shopping cart button - Styled receipt page - Styled course about page for shopping cart courses Addressed HTML/SCSS issues Remove offending body class and unnecessary sass changes Addresses many review comments on stanford shopping cart * framework for generating order instructions on receipt page in shoppingcart.models * move user_cart_has_item into shoppingcart.models * move min_course_price_for_currency into course_modes.models * remove auto activation on purchase * 2-space indents in templates * etc revert indentation on navigation.html for ease of review pep8 pylint move logging/error handling from shoppingcart view to model Addressing @dave changes

jbau · 2013-09-27T14:31:10Z

tests are passing. going to merge this PR. Thanks everyone for the reviews. Any further comments can be address in follow on work.

Stanford paid course registration

miguelurtado · 2014-11-07T01:56:00Z

ADD DEMOX TO CART ($10) - in course about page

but

PRICE PER STUDENT: $0.00 in shoppingcart

It is one bug?

chrisndodge · 2014-11-07T02:27:23Z

@miguelurtado this is the original PR for the shoppingcart. A lot has changed since then. I'd recommend that you post to the edx-code mailing list, and perhaps we can help you over there rather than on a closed PR. Thanks.

…mckin-7806-clean-image.png-2 Bump verison of projects-edx-platform-extensions

We are planning on deprecating this method, and this piece of code has been commented out for a while anyway.

jbau reviewed Sep 25, 2013
View reviewed changes

dianakhuang reviewed Sep 25, 2013
View reviewed changes

jzoldak reviewed Sep 25, 2013
View reviewed changes

dianakhuang reviewed Sep 26, 2013
View reviewed changes

ormsbee reviewed Sep 26, 2013
View reviewed changes

common/djangoapps/course_modes/models.py Outdated

Copy link

Contributor

ormsbee Sep 26, 2013

Choose a reason for hiding this comment

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

The brackets are unnecessary.

jbau added a commit that referenced this pull request Sep 27, 2013

Merge pull request #1118 from edx/jbau/shoppingcart-stanford

c1d555b

Stanford paid course registration

jbau merged commit c1d555b into master Sep 27, 2013

pomegranited pushed a commit to open-craft/openedx-platform that referenced this pull request Aug 8, 2018

Merge pull request openedx#1118 from edx-solutions/kshitij/yonk-1031-…

5ec053b

…mckin-7806-clean-image.png-2 Bump verison of projects-edx-platform-extensions

baby636 mentioned this pull request Nov 5, 2022

[Snyk] Fix for 1 vulnerabilities baby636/edx-platform#33

Open

baby636 mentioned this pull request Dec 26, 2022

[Snyk] Fix for 1 vulnerabilities baby636/edx-platform#41

Open

iloveagent57 pushed a commit that referenced this pull request Feb 26, 2024

chore: Remove reference to paver update_db. (#1118)

9e8b7c8

We are planning on deprecating this method, and this piece of code has been commented out for a while anyway.

Stanford paid course registration #1118

Stanford paid course registration #1118

Uh oh!

Conversation

jbau commented Sep 25, 2013

Uh oh!

jbau commented Sep 25, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

talbs commented Sep 25, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Enable fake payment processing page

MITX_FEATURES['ENABLE_PAYMENT_FAKE'] = True

Uh oh!

jbau commented Sep 26, 2013

Uh oh!

talbs commented Sep 26, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dianakhuang commented Sep 26, 2013

Uh oh!

chrisndodge commented Sep 26, 2013

Uh oh!

jbau commented Sep 26, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ormsbee commented Sep 26, 2013

Uh oh!

jbau commented Sep 27, 2013

Uh oh!

miguelurtado commented Nov 7, 2014

Uh oh!

chrisndodge commented Nov 7, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants