Skip to content

Commit

Permalink
fix oppia#18211: Unexpected error on Exploration Editor page RTE comp…
Browse files Browse the repository at this point in the history
…onent modal when start time is greater than end time
  • Loading branch information
joanapeixinho committed Mar 25, 2024
1 parent 9a4370d commit 68533fe
Show file tree
Hide file tree
Showing 7 changed files with 231 additions and 111 deletions.
2 changes: 1 addition & 1 deletion core/domain/html_cleaner.py
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
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
27 changes: 4 additions & 23 deletions core/templates/services/rte-helper-modal.component.html
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
104 changes: 100 additions & 4 deletions core/templates/services/rte-helper-modal.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,10 @@ describe('RteHelperModalComponent', () => {
beforeEach(() => {
fixture = TestBed.createComponent(RteHelperModalComponent);
component = fixture.componentInstance;
component.attrsCustomizationArgsDict = {
heading: 'This value is not default.',
};
(component.componentId = 'video'),
(component.attrsCustomizationArgsDict = {
heading: 'This value is not default.',
});
component.customizationArgSpecs = customizationArgSpecs;
});

Expand All @@ -132,7 +133,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 +146,7 @@ describe('RteHelperModalComponent', () => {
});
}));
});

describe('when there are validation errors in any form control', function () {
var customizationArgSpecs = [
{
Expand Down Expand Up @@ -175,4 +180,95 @@ 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 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 68533fe

Please sign in to comment.