Skip to content

Conversation

@DevanR
Copy link

@DevanR DevanR commented Feb 17, 2019

This PR contains bug fixes to the course home page with regards to courses that are marked as "public".

Specifically:

  1. Enroll warning is shown only if user is registered and not enrolled
  2. Enroll button is shown only if self-enrollment is allowed
  3. Enroll message is shown only if self-enrolment is allowed

Dependencies: None

Sandbox URL: TBD - sandbox is being provisioned.

Merge deadline: ASAP

Testing instructions:

  1. Set up edx-devstack
  2. Clone "opencraft/edx-platform/devan/show-enroll-links-on-public-courses-only-if-self-enrollment-allowed" to edx-platform
  3. Run "make lms-shell"
  4. Run
    pytest openedx/features/course_experience/tests/view/test_course_home.py::TestCourseHomePageAccess

Reviewers

  • Pooja Kulkarni (pkulkark)
  • edX reviewer[s] TBD

@openedx-webhooks
Copy link

Thanks for the pull request, @DevanR! I've created OSPR-3087 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.

@openedx-webhooks openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Feb 17, 2019
url = course_home_url(self.course)
response = self.client.get(url)
self.assertContains(response, TEST_COURSE_HOME_MESSAGE_UNENROLLED)

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

I was looking into this now and I'm not sure about the changes to be made to the test you referred me. Do I have to make changes in the ddt input for the function? I tried to keep my tests separate, but that function seems to test many different features.

@edx-status-bot
Copy link

Your PR has finished running tests. The following contexts failed:

  • jenkins/quality
  • jenkins/bokchoy

@DevanR DevanR closed this Feb 19, 2019
@DevanR DevanR deleted the opencraft/edx-platform/devan/show-enroll-links-on-public-courses-only-if-self-enrollment-allowed branch February 19, 2019 09:48
@openedx-webhooks
Copy link

@DevanR Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

@natabene
Copy link
Contributor

@DevanR Thank you for your attempt to contribute.

@clemente
Copy link
Contributor

clemente commented Apr 2, 2019

For reference: this PR was superseded by https://github.com/edx/edx-platform/pull/19837, which was already merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U rejected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants