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

Update E-Commerce testing instructions #1417

Merged
merged 1 commit into from Mar 2, 2017
Merged

Conversation

gsong
Copy link
Contributor

@gsong gsong commented Feb 16, 2017

Update the E-Commerce testing instructions to provide step-by-step detail.

Date Needed (optional)

There's no specific urgency around this.

Reviewers

Possible roles follow. The PR submitter checks the boxes after each reviewer finishes and gives 👍.

  • Subject matter expert: @clintonb
  • Doc team review (sanity check, copy edit, or dev edit?): @edx/doc

Testing

  • Ran ./run_tests.sh without warnings or errors

HTML Version (optional)

  • Build an RTD draft for your branch and add a link here

Post-review

  • Add a comment with the description of this change or link this PR to the next release notes task.
  • Squash commits

@clintonb
Copy link
Contributor

@gsong have a chat with @davec-edx. He is also looking at docs.

@gsong gsong force-pushed the george/test-ecommerce-update branch from ac4b0dc to 60dcc3e Compare February 16, 2017 21:09
@srpearce
Copy link
Contributor

@gsong I should be able to review this tomorrow.

@gsong
Copy link
Contributor Author

gsong commented Feb 16, 2017

@clintonb Will do. These are changes based on our working session to hook up LMS and E-Commerce while I was in town.

@davec-edx Take a look if this is of interest to you.

@srpearce Thanks!

@davec-edx
Copy link

LGTM - thanks

Copy link
Contributor

@srpearce srpearce left a comment

Choose a reason for hiding this comment

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

Light copy editing pass finished. Let me know if you have any questions.


#. Verify that the following settings in ``lms.auth.json`` are correct.
#. Login into the VM.
Copy link
Contributor

Choose a reason for hiding this comment

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

Our convention is "Sign in to"


.. code-block:: bash
#. Start LMS.
Copy link
Contributor

Choose a reason for hiding this comment

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

Start the LMS.


Configure E-Commerce
********************

You use the CAT to finish configuring the two courses in your LMS instance.
You use the Course Administration Tool ("**CAT**") to finish configuring the
Copy link
Contributor

@srpearce srpearce Feb 17, 2017

Choose a reason for hiding this comment

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

No bold or quotation marks

.. code-block:: bash
#. Navigate to the `Oauth2 Clients section`_ of `Django adminstration`_, at
http://localhost:8000/admin/oauth2/client/. Login using th staff account
designated in step 5.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since these links won't work for all readers (which could cause confusion), I wouldn't make them appear live. We don't do this elsewhere in the guide.

"of the Django administration console" (or "panel"—we're not very consistent about this)

Typo: th

Instead of referencing specific step numbers (which might change), say "the staff account that you have verified" or similar.

Make note of this token; it is required to run the acceptance tests.
#. Navigate to the `Edx\_Oauth2\_Provider Trusted clients section`_
of `Django administration`_, at
http://localhost:8000/admin/edx_oauth2_provider/trustedclient/.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment here re: links.

``course-v1:edX+DemoX+Demo_Course``.

#. Navigate to `E-Commerce Courses section`_ and add a new course. Leave
all fields at default, with the exception of the following.
Copy link
Contributor

Choose a reason for hiding this comment

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

Leave the default values in all fields


::

Course ID: The course key from LMS
Copy link
Contributor

Choose a reason for hiding this comment

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

the LMS

Course Type: Verified
Include Honor Seat: No

#. Navigate to `LMS Dashboard`_ and verify the course you added in
Copy link
Contributor

@srpearce srpearce Feb 17, 2017

Choose a reason for hiding this comment

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

Navigate to the learner dashboard

No link

E-Commerce now has a green "Upgrade to Verified" badge, which you can
click on.

#. In the CAT, add the second course present on your LMS instance to
Copy link
Contributor

Choose a reason for hiding this comment

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

Strike "present"


#. Navigate to `LMS Dashboard`_ and verify the course you added in
E-Commerce now has a green "Upgrade to Verified" badge, which you can
click on.
Copy link
Contributor

Choose a reason for hiding this comment

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

"click on" --> select

@gsong gsong force-pushed the george/test-ecommerce-update branch 2 times, most recently from 6b02f57 to 353a319 Compare February 23, 2017 20:32
@gsong
Copy link
Contributor Author

gsong commented Feb 23, 2017

@srpearce I believe I addressed all your edit suggestions.

@gsong gsong force-pushed the george/test-ecommerce-update branch from 353a319 to fe600ed Compare February 28, 2017 18:54
Copy link
Contributor

@srpearce srpearce left a comment

Choose a reason for hiding this comment

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

A couple of nits.

Course Type: Verified
Include Honor Seat: No

#. Navigate to the learner Dashboard (e.g. http://localhost:8000/dashboard) and
Copy link
Contributor

Choose a reason for hiding this comment

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

Decap "Dashboard"

Client ID: 'replace-me'
Client Secret: 'replace-me'
Client Type: Confidential
#. Navigate to the Oauth2 Clients section of Django administration console
Copy link
Contributor

Choose a reason for hiding this comment

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

of the Django (here and line 211)

@gsong gsong force-pushed the george/test-ecommerce-update branch from fe600ed to 234e788 Compare March 2, 2017 19:54
@gsong
Copy link
Contributor Author

gsong commented Mar 2, 2017

@srpearce Re:

Add a comment with the description of this change or link this PR to the next release notes task.

Do I need to do that?

@gsong gsong merged commit f4d9111 into master Mar 2, 2017
@gsong gsong deleted the george/test-ecommerce-update branch March 2, 2017 20:56
@srpearce
Copy link
Contributor

srpearce commented Mar 2, 2017

@gsong Do you want to announce this change in next week's release notes? (Are there any developers who will be particularly excited about it?)

@gsong
Copy link
Contributor Author

gsong commented Mar 2, 2017

@srpearce I think it's worthwhile if people ever want to get LMS and E-Commerce working together. It's certainly useful both for internal and open source developers.

@srpearce
Copy link
Contributor

srpearce commented Mar 7, 2017

@gsong Meant to get back to this on Thursday. It sounds like this should be covered in release notes, in that case. I'm not sure what the process is these days, so I'd recommend contacting @marcotuts.

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.

None yet

4 participants