Skip to content

Commit

Permalink
Fix oppia#18211: Enhance Validation for Video Start and End Time in E…
Browse files Browse the repository at this point in the history 
…xploration Editor (oppia#20071)

* fix oppia#18211: Unexpected error on Exploration Editor page RTE

* Fix typo on create-activity-modal.component.spec.ts

* fix typo on rte-helper.service.spec.ts

* unsubscribe from customizationArgsFormSubscription in rte-helper-modal.controller.spec.ts

* Update initialization for test when customization args has a valid youtube video

* Fix indentation in beforeEach block

* Refactor customizationArgsDict assignment in RteHelperModalComponent

* Update save and cancel functions in RteHelperModalController

* Update test for when customization args has a valid youtube video

* Update save function rte-helper-modal.controller.ts

* Fix formating error in rte-helper-modal.controller.ts

* Update rte-helper-modal.controller.ts

* add test in rte-helper-modal.controller.spec.ts for when the text is empty in link form control

* Update rte-helper-modal.controller.ts
  • Loading branch information
@joanapeixinho @shivanandan17
joanapeixinho authored and shivanandan17 committed Apr 29, 2024
1 parent e20a6f9 commit be65bc6
Show file tree
Hide file tree
Showing 8 changed files with 270 additions and 118 deletions.
2 changes: 1 addition & 1 deletion core/domain/html_cleaner.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ def validate_rte_tags(

start_value = float(tag['start-with-value'].strip())
end_value = float(tag['end-with-value'].strip())
if start_value > end_value and start_value != 0.0 and end_value != 0.0:
if start_value >= end_value and start_value != 0.0 and end_value != 0.0:
raise utils.ValidationError(
'Start value should not be greater than End value in Video tag.'
)
Expand Down
3 changes: 3 additions & 0 deletions core/templates/components/ck-editor-helpers/ck-editor-4-widgets.initializer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export interface RteHelperService {
isInlineComponent: (string) => boolean;
openCustomizationModal: (
componentIsNewlyCreated,
componentId,
customizationArgSpecs,
attrsCustomizationArgsDict,
onSubmitCallback,
Expand Down Expand Up @@ -79,6 +80,7 @@ export class CkEditorInitializerService {
var tagName = 'oppia-noninteractive-ckeditor-' + componentDefn.id;
var customizationArgSpecs = componentDefn.customizationArgSpecs;
var isInline = rteHelperService.isInlineComponent(componentDefn.id);
var componentId = componentDefn.id;

// Inline components will be wrapped in a span, while block components
// will be wrapped in a div.
Expand Down Expand Up @@ -137,6 +139,7 @@ export class CkEditorInitializerService {

rteHelperService.openCustomizationModal(
componentIsNewlyCreated,
componentId,
customizationArgSpecs,
customizationArgs,
function (customizationArgsDict) {
Expand Down
2 changes: 1 addition & 1 deletion ...ates/pages/creator-dashboard-page/modal-templates/create-activity-modal.component.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('Create Activity Modal Component', () => {
fixture.destroy();
});

it('should evalute component properties after component is initialized', fakeAsync(() => {
it('should evaluate component properties after component is initialized', fakeAsync(() => {
const UserInfoObject = {
roles: ['USER_ROLE'],
is_moderator: false,
Expand Down
27 changes: 4 additions & 23 deletions core/templates/services/rte-helper-modal.component.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ <h3>Customize This Component</h3>
</div>
</div>
</form>
<div *ngIf="disableSaveButtonForLinkRte()" class="link-error-message">
It seems like clicking on this link will lead the user to a different URL
than the text specifies. Please change the text.
</div>
<div *ngIf="errorMessage" class="error-message">{{ errorMessage }}</div>
</div>

<div class="modal-footer">
Expand All @@ -37,29 +34,13 @@ <h3>Customize This Component</h3>
Cancel
</button>

<button *ngIf="!currentRteIsMathExpressionEditor && !currentRteIsLinkEditor"
type="button"
class="btn btn-success e2e-test-close-rich-text-component-editor oppia-save-btn"
(click)="save()"
[disabled]="!customizationArgsForm.valid">
Done
</button>

<button *ngIf="currentRteIsLinkEditor"
type="button"
<button type="button"
class="btn btn-success e2e-test-close-rich-text-component-editor oppia-save-btn"
(click)="save()"
[disabled]="!customizationArgsForm.valid || disableSaveButtonForLinkRte()">
[disabled]="!customizationArgsForm.valid || isErrorMessageNonempty()">
Done
</button>

<button *ngIf="currentRteIsMathExpressionEditor"
type="button"
class="btn btn-success e2e-test-close-rich-text-component-editor oppia-save-btn"
(click)="save()"
[disabled]="!customizationArgsForm.valid || disableSaveButtonForMathRte()">
Done
</button>
</div>
</div>
<style>
Expand All @@ -71,7 +52,7 @@ <h3>Customize This Component</h3>
.oppia-customization-arg-editor-inner {
margin-top: 15px;
}
.oppia-customization-arg-form-container .link-error-message {
.oppia-customization-arg-form-container .error-message {
color: #f00;
display: block;
font-size: 0.8em;
Expand Down
137 changes: 136 additions & 1 deletion core/templates/services/rte-helper-modal.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ describe('RteHelperModalComponent', () => {
beforeEach(() => {
fixture = TestBed.createComponent(RteHelperModalComponent);
component = fixture.componentInstance;
component.componentId = 'video';
component.attrsCustomizationArgsDict = {
heading: 'This value is not default.',
};
Expand All @@ -123,6 +124,8 @@ describe('RteHelperModalComponent', () => {
}));

it('should close modal when clicking on cancel button', fakeAsync(() => {
component.ngOnInit();
flush();
component.cancel();
expect(activeModal.dismiss).toHaveBeenCalledWith(false);
}));
Expand All @@ -132,7 +135,10 @@ describe('RteHelperModalComponent', () => {
spyOn(contextService, 'getEntityType').and.returnValue('exploration');
component.ngOnInit();
flush();
expect(component.disableSaveButtonForMathRte()).toBe(false);
component.onCustomizationArgsFormChange(
component.attrsCustomizationArgsDict.heading
);
expect(component.isErrorMessageNonempty()).toBe(false);
component.save();
flush();
expect(mockExternalRteSaveEventEmitter.emit).toHaveBeenCalled();
Expand All @@ -142,6 +148,7 @@ describe('RteHelperModalComponent', () => {
});
}));
});

describe('when there are validation errors in any form control', function () {
var customizationArgSpecs = [
{
Expand Down Expand Up @@ -175,4 +182,132 @@ describe('RteHelperModalComponent', () => {
flush();
}));
});

describe('when there are validation errors in link form control', function () {
var customizationArgSpecs = [
{
name: 'url',
default_value: 'oppia.org',
},
{
name: 'text',
default_value: 'oppia',
},
];

beforeEach(() => {
fixture = TestBed.createComponent(RteHelperModalComponent);
component = fixture.componentInstance;
(component.componentId = 'link'),
(component.attrsCustomizationArgsDict = {
url: 'oppia.org',
text: 'oppia',
});
component.customizationArgSpecs = customizationArgSpecs;
});

it('should disable save button and display error message', fakeAsync(() => {
component.ngOnInit();
flush();
component.customizationArgsForm.value[0] = 'oppia.org';
component.customizationArgsForm.value[1] = 'oppia.com';
component.onCustomizationArgsFormChange(
component.customizationArgsForm.value
);
expect(component.isErrorMessageNonempty()).toBe(true);
expect(component.errorMessage).toBe(
'It seems like clicking on this link will lead the user to a ' +
'different URL than the text specifies. Please change the text.'
);
flush();
}));
});

describe('when the text is empty in link form control', function () {
var customizationArgSpecs = [
{
name: 'url',
default_value: 'oppia.org',
},
{
name: 'text',
default_value: ' ',
},
];

beforeEach(() => {
fixture = TestBed.createComponent(RteHelperModalComponent);
component = fixture.componentInstance;
(component.componentId = 'link'),
(component.attrsCustomizationArgsDict = {
url: 'oppia.org',
text: ' ',
});
component.customizationArgSpecs = customizationArgSpecs;
});

it('should make the text equal to url when text is empty', fakeAsync(() => {
component.ngOnInit();
flush();
component.customizationArgsForm.value[0] = 'oppia.org';
component.customizationArgsForm.value[1] = '';
component.onCustomizationArgsFormChange(
component.customizationArgsForm.value
);
expect(component.isErrorMessageNonempty()).toBe(false);
expect(component.customizationArgsForm.value[1]).toBe('oppia.org');
flush();
}));
});

describe('when there are validation errors in video form control', function () {
var customizationArgSpecs = [
{
name: 'video_id',
default_value: 'https://www.youtube.com/watch?v=Ntcw0H0hwPU',
},
{
name: 'start',
default_value: 0,
},
{
name: 'end',
default_value: 0,
},
{
name: 'autoplay',
default_value: false,
},
];

beforeEach(() => {
fixture = TestBed.createComponent(RteHelperModalComponent);
component = fixture.componentInstance;
(component.componentId = 'video'),
(component.attrsCustomizationArgsDict = {
video_id: 'Ntcw0H0hwPU',
start: 0,
end: 0,
autoplay: false,
});
component.customizationArgSpecs = customizationArgSpecs;
});

it('should disable save button and display error message', fakeAsync(() => {
component.ngOnInit();
flush();

component.customizationArgsForm.value[1] = 10;
component.customizationArgsForm.value[2] = 0;
component.onCustomizationArgsFormChange(
component.customizationArgsForm.value
);
expect(component.isErrorMessageNonempty()).toBe(true);
expect(component.errorMessage).toBe(
'Please ensure that the start time of the video is earlier than ' +
'the end time.'
);
flush();
}));
});
});
Loading

0 comments on commit be65bc6

Please sign in to comment.