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

Added videoalpha to advanced components in component.py file. #8

Merged
merged 4 commits into from
Jun 2, 2013

Conversation

valera-rozuvan
Copy link
Contributor

Without this, Video Alpha will not appear under the Advanced Components section. It escapes me how this was missed in the original pull request https://github.com/edx/edx-platform-pre-clean/pull/1714 which was merged recently.

@vaxXxa and @auraz please review!

@ghost ghost assigned auraz May 31, 2013
@auraz
Copy link
Contributor

auraz commented May 31, 2013

Good to go.

@vasylnakvasiuk
Copy link
Contributor

+1

@Lyla-Fischer
Copy link

@chrisndodge @cahrens

@cahrens
Copy link

cahrens commented May 31, 2013

We just talked about this list of advanced components in an RCA. We have no tests that the list works properly (that the strings in the list are valid advanced xmodules). We've had 2 hotfixes around the list being improperly formatted. Therefore, the decision at the RCA was that the next person to touch this list needed to write some tests around it.

Hate to be the bearer of bad news, but I can't give a thumbs-up with tests (in particular, that "videoalpha" translates into the correct xmodule).

@chrisndodge
Copy link
Contributor

Is this PR blocking Stanford? My understanding was that video alpha was working but users had to manually add to the list of advanced modules first.

Or is my understand wrong?

If this isn't a blocker then lets wait for tests. If it really is a blocker, then maybe I can look into writing a test tonight.

Sent from my iPhone

On May 31, 2013, at 4:09 PM, Christina Roberts notifications@github.com wrote:

We just talked about this list of advanced components in an RCA. We have no tests that the list works properly (that the strings in the list are valid advanced xmodules). We've had 2 hotfixes around the list being improperly formatted. Therefore, the decision at the RCA was that the next person to touch this list needed to write some tests around it.

Hate to be the bearer of bad news, but I can't give a thumbs-up with tests (in particular, that "videoalpha" translates into the correct xmodule).


Reply to this email directly or view it on GitHub.

@cahrens
Copy link

cahrens commented May 31, 2013

The problem is that you can't add videoalpha via AdvancedSettings unless it is in this list. I don't know when it got removed... perhaps merge conflicts? This line of code has been responsible for 2 hotfixes (in 2 consecutive weeks).

@cdodge
Copy link

cdodge commented May 31, 2013

Ok, sounds like a blocker. I'll try to write test tonight as I presume the Ukraine is away for weekend.

Sent from my iPhone

On May 31, 2013, at 5:11 PM, "Christina Roberts" notifications@github.com wrote:

The problem is that you can't add videoalpha via AdvancedSettings unless it is in this list. I don't know when it got removed... perhaps merge conflicts? This line of code has been responsible for 2 hotfixes (in 2 consecutive weeks).


Reply to this email directly or view it on GitHub.

@jbau
Copy link

jbau commented May 31, 2013

Just FYI: It's important enough that we already cherry-picked this commit into edx-west/release and have it on our site. So far, it's been working (video is showing up, speeds are changing, captions are working), modulo some bugs re: changing speeds when video is playing, which we will file

@chrisndodge
Copy link
Contributor

@cahrens @jbau @Lyla-Fischer @valera-rozuvan @auraz @vaxXxa

As per Christina's request, added simple unit test to assert that the advanced modules appear in the edit unit page as expected. This probably should be a Lettuce test at some point, but we can do some basics as unit tests

self.assertIn('Video Alpha', resp.content)
self.assertIn('Word cloud', resp.content)
self.assertIn('Annotation', resp.content)
self.assertIn('Open Ended Response', resp.content)
Copy link

Choose a reason for hiding this comment

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

Can you add peer grading as well? (It is correct that Notes will not appear in the list.)

@cahrens
Copy link

cahrens commented Jun 2, 2013

A reasonable first test-- thanks, Chris. 👍 once you add a check for peer grading.

chrisndodge pushed a commit that referenced this pull request Jun 2, 2013
Added videoalpha to advanced components in component.py file.
@chrisndodge chrisndodge merged commit 7735e37 into master Jun 2, 2013
@auraz
Copy link
Contributor

auraz commented Jun 3, 2013

@chrisndodge thank you

@valera-rozuvan
Copy link
Contributor Author

@chrisndodge = ) You are great! Thanks.

martynovp referenced this pull request in miptliot/edx-platform Aug 19, 2015
Оценивание по вертикалям
mbifulco referenced this pull request in gymnasium/edx-platform Aug 27, 2015
shrlyhe pushed a commit that referenced this pull request Jun 5, 2017
# This is the 1st commit message:
ENT-385 change error msg for confirm email

# This is the commit message #2:

remove changed msgid from po files

# This is the commit message #3:

add name to AUTHORS file

# This is the commit message #4:

remove lastfailed file

# This is the commit message #5:

GradingPolicyChanged Signal Handler

https://openedx.atlassian.net/browse/EDUCATOR-393

# This is the commit message #6:

EDUCATOR-435 | Include enrollment status in in course and problem grade reports.

# This is the commit message #7:

Change visibility to access.

EDUCATOR-396

# This is the commit message #8:

LEARNER-923 Make Pattern Library support tabs transparent

# This is the commit message #9:

Conform to WCAG 2 AA contrast minimums for Google OAuth.

Also apply variables for FB/Linkedin OAuth2 rather than
leave color hashes hanging around.

# This is the commit message #10:

Updated auto_auth endpoint to always return JSON

Defaulting to a plaintext response makes no sense for an endpoint that is intended to be used by machines for testing. The endpoint now returns JSON only unless a redirect action is triggered.

# This is the commit message #1:

Fix the activation email support link in the dashboard sidebar

# This is the commit message #2:

Mask grades on progress page according to "Show Correctness" setting.

# This is the commit message #3:

More celery logging

# This is the commit message #4:

Eventing for grading policy change

# This is the commit message #5:

More specific grace_course logging

# This is the commit message #6:

Add course overrides of waffle flags.

# This is the commit message #7:

Mark test as flaky, and remove flaky from a fixed test.

EDUCATOR-511

# This is the commit message #8:

Fix for LEARNER-859: Updating styling on the course updates page to more clearly differentiate between multiple updates. Specifically:
- Updated styling on date in the top left corner
- Added horizontal line between updates
- Removed ability to toggle updates on and off
- Cleaned up code to always show all updates:
mumarkhan999 pushed a commit to mumarkhan999/edx-platform that referenced this pull request Mar 4, 2019
Added utils file and agent field in BaseTransformer
pomegranited referenced this pull request in edx-olive/edx-platform-old May 8, 2019
yoann-mroz referenced this pull request in weuplearning/edx-platform Nov 30, 2020
This custom image includes the elasticsearch-head plugin

Fixes #8
yasir1brahim pushed a commit to yasir1brahim/edx-platform that referenced this pull request Apr 19, 2021
Implemented custom APIs with custom response format

Approved-by: Oksana Slusarenko
lpm0073 added a commit to grid-synergy/edx-platform that referenced this pull request Apr 27, 2021
Sujeet1379 pushed a commit to chandrudev/edx-platform that referenced this pull request Nov 17, 2022
Valid courseware URLs currently include:
* /course/:courseId
* /course/:courseId/:sequenceId
* /course/:courseId/:sequenceId/:unitId

In this commit we add support for:
* /course/:courseId/:sectionId
* /course/:courseId/:sectionId/:unitId
* /course/:courseId/:unitId

All URL forms still redirect to:
  /course/:courseId/:sequenceId/:unitId

See ADR openedx#8 for more context.

All changes:
* refactor: allow courseBlocks factory to build multiple sections
* refactor: make CoursewareContainer tests less brittle & stateful
* feat: handle courseware paths more liberally
* refactor: reorder, rename, & comment redirection functions

TNL-7796
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.

8 participants