Skip to content

add check transcript ajax for edx_video_id - #17938

Merged
muhammad-ammar merged 4 commits into
transcripts-phase-2from
ammar/add-check-transcript-ajax-on-basic-tab
May 3, 2018
Merged

add check transcript ajax for edx_video_id#17938
muhammad-ammar merged 4 commits into
transcripts-phase-2from
ammar/add-check-transcript-ajax-on-basic-tab

Conversation

@muhammad-ammar

Copy link
Copy Markdown
Contributor

No description provided.

@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar
muhammad-ammar force-pushed the ammar/EDUCATOR-1759-delete-transcript-from-advance-tab branch from 6e1784f to 6784ee7 Compare April 12, 2018 19:54
@muhammad-ammar
muhammad-ammar force-pushed the ammar/add-check-transcript-ajax-on-basic-tab branch from 39411c3 to eec9b74 Compare April 13, 2018 06:56
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar
muhammad-ammar force-pushed the ammar/add-check-transcript-ajax-on-basic-tab branch from eec9b74 to de95b17 Compare April 14, 2018 08:58
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar
muhammad-ammar force-pushed the ammar/add-check-transcript-ajax-on-basic-tab branch from de95b17 to 7eaef32 Compare April 15, 2018 05:39
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

1 similar comment
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar
muhammad-ammar force-pushed the ammar/add-check-transcript-ajax-on-basic-tab branch 2 times, most recently from 788ee72 to fe05b18 Compare April 16, 2018 13:32
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

1 similar comment
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar
muhammad-ammar force-pushed the ammar/add-check-transcript-ajax-on-basic-tab branch 2 times, most recently from 6e3680c to c21461d Compare April 17, 2018 13:49
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

1 similar comment
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar

Copy link
Copy Markdown
Contributor Author

jenkins run all

@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

1 similar comment
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar
muhammad-ammar force-pushed the ammar/add-check-transcript-ajax-on-basic-tab branch from 0bbf6c4 to bc6b589 Compare April 19, 2018 13:29
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

1 similar comment
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar
muhammad-ammar force-pushed the ammar/add-check-transcript-ajax-on-basic-tab branch from bc6b589 to 6fc318b Compare April 19, 2018 14:40
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar

Copy link
Copy Markdown
Contributor Author

jenkins run js

@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar
muhammad-ammar force-pushed the ammar/add-check-transcript-ajax-on-basic-tab branch from 6fc318b to 0e16708 Compare April 20, 2018 08:00
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar
muhammad-ammar force-pushed the ammar/add-check-transcript-ajax-on-basic-tab branch from e5528b7 to f6aea07 Compare April 23, 2018 13:55
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar
muhammad-ammar force-pushed the ammar/EDUCATOR-1759-delete-transcript-from-advance-tab branch from 82d0968 to d5758bd Compare April 24, 2018 11:50
@muhammad-ammar
muhammad-ammar force-pushed the ammar/add-check-transcript-ajax-on-basic-tab branch from f6aea07 to d7b5b6b Compare April 24, 2018 11:55
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar
muhammad-ammar changed the base branch from ammar/EDUCATOR-1759-delete-transcript-from-advance-tab to transcripts-phase-2 April 25, 2018 13:28
@muhammad-ammar

Copy link
Copy Markdown
Contributor Author

jenkins run all

@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar
muhammad-ammar changed the base branch from transcripts-phase-2 to ammar/remove-video-transcript-enabled-flag April 26, 2018 03:59
@muhammad-ammar
muhammad-ammar force-pushed the ammar/add-check-transcript-ajax-on-basic-tab branch from d7b5b6b to 88c2a08 Compare April 26, 2018 04:01
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar

Copy link
Copy Markdown
Contributor Author

@mushtaqak This is waiting for your review

@muhammad-ammar
muhammad-ammar force-pushed the ammar/add-check-transcript-ajax-on-basic-tab branch from 88c2a08 to 388649b Compare April 27, 2018 06:06
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar
muhammad-ammar force-pushed the ammar/remove-video-transcript-enabled-flag branch from 6a8ea29 to aaaafee Compare April 30, 2018 10:59
@muhammad-ammar
muhammad-ammar changed the base branch from ammar/remove-video-transcript-enabled-flag to transcripts-phase-2 April 30, 2018 12:35
@muhammad-ammar
muhammad-ammar force-pushed the ammar/add-check-transcript-ajax-on-basic-tab branch from 388649b to 35611e7 Compare April 30, 2018 12:52
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@mushtaqak mushtaqak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Miner comments, rest is looking 👍


try:
get_transcript_from_val(edx_video_id=item.edx_video_id, lang=u'en')
edx_video_id = videos.get('edx_video_id')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's clean this.


Metadata.VideoID = Metadata.String.extend({
// Delay between check_transcript requests
requestDelay: 300,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

300 should be a constant

spyOn(editor, 'getLocator').and.returnValue(component_locator);

// reset
Backbone.trigger.calls.reset();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this needed ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Events(Backbone.trigger) were triggered when Editor view is rendered above. We are clearing the those calls so that we can test in a clean state.

MessageManager: MessageManager
});

waitForEvent()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is this doing exactly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This waits for transcripts:basicTabFieldChanged event which was triggered when VideoList is rendered.

# whenever edx_video_id changes on frontend. Thats why we
# are changing type to `VideoID` so that a specific
# Backbonjs view can handle it.
editable_fields['edx_video_id']['type'] = 'VideoID'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice

if (response.responseJSON !== undefined) {
errorMessage = response.responseJSON.status;
} else {
errorMessage = gettext('Error: Connection with server failed.');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we can remove this else block since errorMessage is declared with gettext('Error: Connection with server failed.');

return this.$el.closest('[data-locator]').data('locator');
},

handleFieldChanged: function() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we may need docstring for this method or a comment at-least

@muhammad-ammar

Copy link
Copy Markdown
Contributor Author

@mushtaqak Addressed feedback.

@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@muhammad-ammar
muhammad-ammar force-pushed the ammar/add-check-transcript-ajax-on-basic-tab branch from 4ce3382 to c02955d Compare May 2, 2018 13:24
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

…ad-edx-video-id

basic tab attach uploads with edx_video_id
@muhammad-ammar
muhammad-ammar merged commit 583bacf into transcripts-phase-2 May 3, 2018
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests.

@UmanShahzad
UmanShahzad deleted the ammar/add-check-transcript-ajax-on-basic-tab branch May 9, 2018 20:42
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