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

Fix part of #3704: make audio download URL safe; don't show flagging modal if all translations are flagged; use URL interpolation for modals. #3709

Merged
merged 1 commit into from
Aug 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Audio fixes: make audio download URL safe; don't show flagging modal …
…if all translations are flagged; use templateUrl for modals.
  • Loading branch information
seanlip committed Aug 5, 2017
commit 41459a6ca868df5e2c75cc8070021b16ea7a4a50
18 changes: 18 additions & 0 deletions app.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,21 @@ skip_files:
# Other folders to ignore
- tests/
- scripts/
- integrations/
- integrations_dev/
# Some third_party static scripts are directly imported, namely: jquery,
# jqueryui, angularjs, jqueryui-touch-punch, MathJax, code-mirror,
# ui-codemirror, d3js, midi-js, ui-map, guppy, skulpt, math-expressions.
# We exclude some of the scripts that are not directly imported in order to
# reduce deployed file count.
# TODO(sll): Find a more structured way of doing this.
- third_party/static/angular-audio-1.7.4
- third_party/static/angular-scroll-1.0.0
- third_party/static/angular-toastr-1.7.0
- third_party/static/font-awesome-4.4.0
- third_party/static/messageformat-0.3.1
- third_party/static/ng-img-crop-0.3.2
- third_party/static/nginfinitescroll-1.0.0
- third_party/static/perfectscrollbar-0.6.16
- third_party/static/textAngular-1.4.5
- third_party/static/widget
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ oppia.directive('audioFileUploader', [
templateUrl: UrlInterpolationService.getDirectiveTemplateUrl(
'/components/forms/audio_file_uploader_directive.html'),
link: function(scope, elt) {
var ALLOWED_AUDIO_FILE_TYPES = ['audio/mp3', 'audio/mpeg'];

var validateUploadedFile = function(file) {
if (!file) {
return 'No audio file was uploaded.';
Expand All @@ -37,7 +39,7 @@ oppia.directive('audioFileUploader', [
return 'This file is not recognized as an audio file.';
}

if (!file.type.match('audio.mp3')) {
if (ALLOWED_AUDIO_FILE_TYPES.indexOf(file.type) === -1) {
return 'Only the MP3 audio format is currently supported.';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ oppia.directive('audioTranslationsEditor', [
templateUrl: UrlInterpolationService.getDirectiveTemplateUrl(
'/components/forms/audio_translations_editor_directive.html'),
controller: [
'$scope', '$modal', 'stateContentService', 'editabilityService',
'$scope', '$modal', '$sce', 'stateContentService', 'editabilityService',
'LanguageUtilService', 'alertsService', 'explorationContextService',
'AssetsBackendApiService',
function(
$scope, $modal, stateContentService, editabilityService,
$scope, $modal, $sce, stateContentService, editabilityService,
LanguageUtilService, alertsService, explorationContextService,
AssetsBackendApiService) {
$scope.isEditable = editabilityService.isEditable;
Expand All @@ -49,8 +49,9 @@ oppia.directive('audioTranslationsEditor', [
LanguageUtilService.getAudioLanguageDescription);

$scope.getAudioTranslationFullUrl = function(filename) {
return AssetsBackendApiService.getAudioDownloadUrl(
explorationId, filename);
return $sce.trustAsResourceUrl(
AssetsBackendApiService.getAudioDownloadUrl(
explorationId, filename));
};

$scope.toggleNeedsUpdateAttribute = function(languageCode) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ <h4 class="oppia-audio-translations-header">Audio Translations</h4>
<div class="row">
<div class="col-lg-3 col-md-3 col-sm-3 audio-language-description">
<span class="needs-update-marker"
ng-attr-tooltip="<[audioTranslation.needsUpdate ? 'Audio might not match text. Delete and reupload the file, or click to unflag.' : 'Click to mark this audio translation as not matching text.']>"
ng-attr-tooltip="<[audioTranslation.needsUpdate ? 'Audio might not match text. Reupload the file, or click to unflag.' : 'Click to mark this audio translation as not matching text.']>"
ng-click="toggleNeedsUpdateAttribute(languageCode)">
<i ng-if="audioTranslation.needsUpdate" class="material-icons needs-update">&#xE002;</i>
<i ng-if="!audioTranslation.needsUpdate" class="material-icons is-current">&#xE876;</i>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,15 @@ oppia.factory('SubtitledHtmlObjectFactory', [
return this.getAudioLanguageCodes().length > 0;
};

SubtitledHtml.prototype.hasUnflaggedAudioTranslations = function() {
for (var languageCode in this._audioTranslations) {
if (!this._audioTranslations[languageCode].needsUpdate) {
return true;
}
}
return false;
};

SubtitledHtml.prototype.isFullyTranslated = function() {
var numLanguages = Object.keys(this._audioTranslations).length;
return (numLanguages === LanguageUtilService.getAudioLanguagesCount());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,19 @@ describe('SubtitledHtml object factory', function() {
expect(subtitledHtml.hasAudioTranslations()).toBe(false);
}));

it('should check existence of unflagged audio translations',
inject(function() {
expect(subtitledHtml.hasUnflaggedAudioTranslations()).toBe(true);
subtitledHtml.getAudioTranslation('en').needsUpdate = true;
expect(subtitledHtml.hasUnflaggedAudioTranslations()).toBe(true);
subtitledHtml.getAudioTranslation('hi').needsUpdate = true;
expect(subtitledHtml.hasUnflaggedAudioTranslations()).toBe(false);

subtitledHtml.deleteAudioTranslation('en');
subtitledHtml.deleteAudioTranslation('hi');
expect(subtitledHtml.hasUnflaggedAudioTranslations()).toBe(false);
}));

it('should check whether the text is fully translated', inject(function() {
expect(subtitledHtml.isFullyTranslated()).toBe(true);
subtitledHtml.deleteAudioTranslation('en');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ oppia.directive('stateContentEditor', [
$scope.onSaveContentButtonClicked = function() {
editorFirstTimeEventsService.registerFirstSaveContentEvent();
saveContent();
if (stateContentService.savedMemento.hasAudioTranslations()) {

var savedContent = stateContentService.savedMemento;
if (savedContent.hasUnflaggedAudioTranslations()) {
openMarkAllAudioAsNeedingUpdateModal();
}
$scope.getOnSaveContentFn()();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@
* @fileoverview Service for detecting spamming behavior from the learner.
*/

oppia.factory('FatigueDetectionService',
['$modal', function($modal) {
oppia.factory('FatigueDetectionService', [
'$modal', 'UrlInterpolationService',
function($modal, UrlInterpolationService) {
// 4 submissions in under 10 seconds triggers modal.
var SPAM_COUNT_THRESHOLD = 4;
var SPAM_WINDOW_MSEC = 10000;
Expand All @@ -30,7 +31,7 @@ oppia.factory('FatigueDetectionService',
isSubmittingTooFast: function() {
if (submissionTimesMsec.length >= SPAM_COUNT_THRESHOLD) {
var windowStartTime = submissionTimesMsec.shift();
var windowEndTime =
var windowEndTime =
submissionTimesMsec[submissionTimesMsec.length - 1];
if (windowEndTime - windowStartTime < SPAM_WINDOW_MSEC) {
return true;
Expand All @@ -40,7 +41,8 @@ oppia.factory('FatigueDetectionService',
},
displayTakeBreakMessage: function() {
$modal.open({
templateUrl: 'modals/takeBreak',
templateUrl: UrlInterpolationService.getDirectiveTemplateUrl(
'/pages/exploration_player/take_break_modal_directive.html'),
backdrop: 'static',
resolve: {},
controller: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ oppia.controller('LearnerLocalNav', [

$scope.showFlagExplorationModal = function() {
$modal.open({
templateUrl: 'modals/flagExploration',
templateUrl: UrlInterpolationService.getDirectiveTemplateUrl(
'/pages/exploration_player/flag_exploration_modal_directive.html'),
backdrop: true,
controller: [
'$scope', '$modalInstance', 'playerPositionService',
Expand Down Expand Up @@ -119,9 +120,9 @@ oppia.controller('LearnerLocalNav', [
]
}).result.then(function(result) {
var flagExplorationUrl = UrlInterpolationService.interpolateUrl(
FLAG_EXPLORATION_URL_TEMPLATE, {
exploration_id: $scope.explorationId
}
FLAG_EXPLORATION_URL_TEMPLATE, {
exploration_id: $scope.explorationId
}
);
var report = (
'[' + result.state + '] (' + result.report_type + ') ' +
Expand All @@ -132,7 +133,9 @@ oppia.controller('LearnerLocalNav', [
alertsService.addWarning(error);
});
$modal.open({
templateUrl: 'modals/explorationSuccessfullyFlagged',
templateUrl: UrlInterpolationService.getDirectiveTemplateUrl(
'/pages/exploration_player/' +
'exploration_successfully_flagged_modal_directive.html'),
backdrop: true,
controller: [
'$scope', '$modalInstance',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@

oppia.controller('LearnerViewInfo', [
'$scope', '$modal', '$http', '$log', 'explorationContextService',
'EXPLORATION_SUMMARY_DATA_URL_TEMPLATE',
'EXPLORATION_SUMMARY_DATA_URL_TEMPLATE', 'UrlInterpolationService',
function($scope, $modal, $http, $log, explorationContextService,
EXPLORATION_SUMMARY_DATA_URL_TEMPLATE) {
EXPLORATION_SUMMARY_DATA_URL_TEMPLATE, UrlInterpolationService) {
var explorationId = explorationContextService.getExplorationId();
var expInfo = null;

Expand All @@ -46,7 +46,8 @@ oppia.controller('LearnerViewInfo', [
var openInformationCardModal = function() {
$modal.open({
animation: true,
templateUrl: 'modal/informationCard',
templateUrl: UrlInterpolationService.getDirectiveTemplateUrl(
'/pages/exploration_player/information_card_modal_directive.html'),
windowClass: 'oppia-modal-information-card',
resolve: {
expInfo: function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,7 @@ <h3>Suggest a Change</h3>
}
</style>

{% include 'pages/exploration_player/exploration_successfully_flagged_modal.html' %}
{% include 'pages/exploration_player/feedback_popup_directive.html' %}
{% include 'pages/exploration_player/flag_exploration_modal.html' %}
{% include 'pages/exploration_player/information_card_modal.html' %}
{% include 'pages/exploration_player/take_break_modal.html' %}

{% endblock %}

{% block footer %}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<div class="modal-header">
<h3><['I18N_PLAYER_REPORT_SUCCESS_MODAL_HEADER' | translate]></h3>
</div>

<div class="modal-body">
<p>
<['I18N_PLAYER_REPORT_SUCCESS_MODAL_BODY' | translate]>
</p>
</div>

<div class="modal-footer">
<button class="btn btn-default" ng-click="close()"><['I18N_PLAYER_REPORT_SUCCESS_MODAL_CLOSE' | translate]></button>
</div>

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<div class="modal-header">
<h3>
<span><['I18N_PLAYER_REPORT_MODAL_HEADER' | translate]></span>
</h3>
</div>
<div class="modal-body oppia-long-text">
<h4>
<span><b><['I18N_PLAYER_REPORT_MODAL_BODY_HEADER' | translate]></b></span>
</h4>
<form>
<div class="radio">
<label>
<input type="radio" value="ad" ng-model="flag" ng-change="showFlagMessageTextarea(flag)">
<['I18N_PLAYER_REPORT_MODAL_BODY_AD' | translate]>
</label>
</div>
<div class="radio">
<label>
<input type="radio" value="poor_experience" ng-model="flag" ng-change="showFlagMessageTextarea(flag)">
<['I18N_PLAYER_REPORT_MODAL_BODY_POOR_EXPERIENCE' | translate]>
</label>
</div>
<div class="radio">
<label>
<input type="radio" value="other" ng-model="flag" ng-change="showFlagMessageTextarea(flag)">
<['I18N_PLAYER_REPORT_MODAL_BODY_OTHER' | translate]>
</label>
<p ng-show="flagMessageTextareaIsShown">
<['I18N_PLAYER_REPORT_MODAL_BODY_ADDITIONAL_DETAILS' | translate]>
<textarea rows="3" cols="50" ng-model="flagMessage" class="form-control" focus-on="flagMessageTextarea">
</textarea>
</p>
</div>
</form>
</div>
<div class="modal-footer">
<button class="btn btn-default" ng-click="cancel()">
<['I18N_PLAYER_REPORT_MODAL_FOOTER_CANCEL' | translate]>
</button>
<button class="btn btn-success" ng-click="submitReport()" ng-disabled="!flagMessage">
<['I18N_PLAYER_REPORT_MODAL_FOOTER_SUBMIT' | translate]>
</button>
</div>
Loading