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

BC-3475 - Add alternative text to file content element #4394

Merged
merged 17 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ describe(`card create (api)`, () => {
type: 'file',
content: {
caption: '',
alternativeText: '',
},
},
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,23 @@ import { EntityManager } from '@mikro-orm/mongodb';
import { HttpStatus, INestApplication } from '@nestjs/common';
import { Test, TestingModule } from '@nestjs/testing';
import { sanitizeRichText } from '@shared/controller';
import { BoardExternalReferenceType, InputFormat, RichTextElementNode } from '@shared/domain';
import {
BoardExternalReferenceType,
ContentElementType,
FileElementNode,
InputFormat,
RichTextElementNode,
} from '@shared/domain';
import {
TestApiClient,
UserAndAccountTestFactory,
cardNodeFactory,
cleanupCollections,
columnBoardNodeFactory,
columnNodeFactory,
courseFactory,
fileElementNodeFactory,
richTextElementNodeFactory,
TestApiClient,
UserAndAccountTestFactory,
} from '@shared/testing';
import { ServerTestModule } from '@src/modules/server/server.module';

Expand Down Expand Up @@ -52,51 +59,93 @@ describe(`content element update content (api)`, () => {

const column = columnNodeFactory.buildWithId({ parent: columnBoardNode });
const parentCard = cardNodeFactory.buildWithId({ parent: column });
const element = richTextElementNodeFactory.buildWithId({ parent: parentCard });

await em.persistAndFlush([teacherAccount, teacherUser, parentCard, column, columnBoardNode, element]);
const richTextelement = richTextElementNodeFactory.buildWithId({ parent: parentCard });
const fileElement = fileElementNodeFactory.buildWithId({ parent: parentCard });

await em.persistAndFlush([
teacherAccount,
teacherUser,
parentCard,
column,
columnBoardNode,
richTextelement,
fileElement,
]);
em.clear();

const loggedInClient = await testApiClient.login(teacherAccount);

return { loggedInClient, element };
return { loggedInClient, richTextelement, fileElement };
};

it('should return status 204', async () => {
const { loggedInClient, element } = await setup();
const { loggedInClient, richTextelement } = await setup();

const response = await loggedInClient.patch(`${element.id}/content`, {
data: { content: { text: 'hello world', inputFormat: InputFormat.RICH_TEXT_CK5 }, type: 'richText' },
const response = await loggedInClient.patch(`${richTextelement.id}/content`, {
data: {
content: { text: 'hello world', inputFormat: InputFormat.RICH_TEXT_CK5 },
type: ContentElementType.RICH_TEXT,
},
});

expect(response.statusCode).toEqual(204);
});

it('should actually change content of the element', async () => {
const { loggedInClient, element } = await setup();
const { loggedInClient, richTextelement } = await setup();

await loggedInClient.patch(`${element.id}/content`, {
data: { content: { text: 'hello world', inputFormat: InputFormat.RICH_TEXT_CK5 }, type: 'richText' },
await loggedInClient.patch(`${richTextelement.id}/content`, {
data: {
content: { text: 'hello world', inputFormat: InputFormat.RICH_TEXT_CK5 },
type: ContentElementType.RICH_TEXT,
},
});
const result = await em.findOneOrFail(RichTextElementNode, element.id);
const result = await em.findOneOrFail(RichTextElementNode, richTextelement.id);

expect(result.text).toEqual('hello world');
});

it('should sanitize rich text before changing content of the element', async () => {
const { loggedInClient, element } = await setup();
const { loggedInClient, richTextelement } = await setup();

const text = '<iframe>rich text 1</iframe> some more text';

const sanitizedText = sanitizeRichText(text, InputFormat.RICH_TEXT_CK5);

await loggedInClient.patch(`${element.id}/content`, {
data: { content: { text, inputFormat: InputFormat.RICH_TEXT_CK5 }, type: 'richText' },
await loggedInClient.patch(`${richTextelement.id}/content`, {
data: { content: { text, inputFormat: InputFormat.RICH_TEXT_CK5 }, type: ContentElementType.RICH_TEXT },
});
const result = await em.findOneOrFail(RichTextElementNode, element.id);
const result = await em.findOneOrFail(RichTextElementNode, richTextelement.id);

expect(result.text).toEqual(sanitizedText);
});

it('should sanitize caption parameter before changing caption of the element', async () => {
const { loggedInClient, fileElement } = await setup();

const caption = '<iframe>rich text 1</iframe> some more text';

await loggedInClient.patch(`${fileElement.id}/content`, {
data: { content: { caption, alternativeText: '' }, type: ContentElementType.FILE },
});

const result = await em.findOneOrFail(FileElementNode, fileElement.id);

expect(result.caption).toEqual('rich text 1 some more text');
});

it('should sanitize alternativeText parameter before changing caption of the element', async () => {
const { loggedInClient, fileElement } = await setup();

const alternativeText = '<iframe>rich text 1</iframe> some more text';

await loggedInClient.patch(`${fileElement.id}/content`, {
data: { content: { caption: '', alternativeText }, type: ContentElementType.FILE },
});
const result = await em.findOneOrFail(FileElementNode, fileElement.id);

expect(result.alternativeText).toEqual('rich text 1 some more text');
});
});

describe('with invalid user', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@ import { ContentElementType } from '@shared/domain';
import { TimestampsResponse } from '../timestamps.response';

export class FileElementContent {
constructor({ caption }: FileElementContent) {
constructor({ caption, alternativeText }: FileElementContent) {
this.caption = caption;
this.alternativeText = alternativeText;
}

@ApiProperty()
caption: string;

@ApiProperty()
alternativeText: string;
}

export class FileElementResponse {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ export class FileContentBody {
@IsString()
@ApiProperty({})
caption!: string;

@IsString()
@ApiProperty({})
alternativeText!: string;
}

export class FileElementContentBody extends ElementContentBody {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export class FileElementResponseMapper implements BaseResponseMapper {
id: element.id,
timestamps: new TimestampsResponse({ lastUpdatedAt: element.updatedAt, createdAt: element.createdAt }),
type: ContentElementType.FILE,
content: new FileElementContent({ caption: element.caption }),
content: new FileElementContent({ caption: element.caption, alternativeText: element.alternativeText }),
});

return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export class BoardDoBuilderImpl implements BoardDoBuilder {
const element = new FileElement({
id: boardNode.id,
caption: boardNode.caption,
alternativeText: boardNode.alternativeText,
children: [],
createdAt: boardNode.createdAt,
updatedAt: boardNode.updatedAt,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ describe(RecursiveSaveVisitor.name, () => {
id: fileElement.id,
type: BoardNodeType.FILE_ELEMENT,
caption: fileElement.caption,
alternativeText: fileElement.alternativeText,
};
expect(visitor.createOrUpdateBoardNode).toHaveBeenCalledWith(expect.objectContaining(expectedNode));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export class RecursiveSaveVisitor implements BoardCompositeVisitor {
const boardNode = new FileElementNode({
id: fileElement.id,
caption: fileElement.caption,
alternativeText: fileElement.alternativeText,
parent: parentData?.boardNode,
position: parentData?.position,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
Column,
ColumnBoard,
FileElement,
InputFormat,
RichTextElement,
SubmissionContainerElement,
SubmissionItem,
Expand Down Expand Up @@ -35,7 +36,13 @@ export class ContentElementUpdateVisitor implements BoardCompositeVisitor {

visitFileElement(fileElement: FileElement): void {
if (this.content instanceof FileContentBody) {
fileElement.caption = this.content.caption;
fileElement.caption = this.content.caption
? sanitizeRichText(this.content.caption, InputFormat.PLAIN_TEXT)
: fileElement.caption;

fileElement.alternativeText = this.content.alternativeText
? sanitizeRichText(this.content.alternativeText, InputFormat.PLAIN_TEXT)
: fileElement.alternativeText;
} else {
this.throwNotHandled(fileElement);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ describe(ContentElementService.name, () => {

const content = new FileContentBody();
content.caption = 'this has been updated';
content.alternativeText = 'this altText has been updated';
const card = cardFactory.build();
boardDoRepo.findParentOfId.mockResolvedValue(card);

Expand All @@ -216,6 +217,7 @@ describe(ContentElementService.name, () => {
await service.update(fileElement, content);

expect(fileElement.caption).toEqual(content.caption);
expect(fileElement.alternativeText).toEqual(content.alternativeText);
});

it('should persist the element', async () => {
Expand Down
2 changes: 1 addition & 1 deletion apps/server/src/modules/board/uc/element.uc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ describe(ElementUc.name, () => {
const setup = () => {
const user = userFactory.build();
const fileElement = fileElementFactory.build();
const content = { caption: 'this has been updated' };
const content = { caption: 'this has been updated', alternativeText: 'this altText has been updated' };

const elementSpy = elementService.findById.mockResolvedValue(fileElement);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export class ContentElementFactory {
const element = new FileElement({
id: new ObjectId().toHexString(),
caption: '',
alternativeText: '',
children: [],
createdAt: new Date(),
updatedAt: new Date(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@ describe(FileElement.name, () => {
});
});

describe('update alternative text', () => {
it('should be able to update alternative text', () => {
const fileElement = fileElementFactory.build();
const text = 'this is the titanic movie from 1997 in Blue-Ray Quality';
fileElement.alternativeText = text;

expect(fileElement.alternativeText).toEqual(text);
});
});

describe('accept', () => {
it('should call the right visitor method', () => {
const visitor = createMock<BoardCompositeVisitor>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ export class FileElement extends BoardComposite<FileElementProps> {
this.props.caption = value;
}

get alternativeText(): string {
return this.props.alternativeText;
}

set alternativeText(value: string) {
this.props.alternativeText = value;
}

isAllowedAsChild(): boolean {
return false;
}
Expand All @@ -25,4 +33,5 @@ export class FileElement extends BoardComposite<FileElementProps> {

export interface FileElementProps extends BoardCompositeProps {
caption: string;
alternativeText: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe(FileElementNode.name, () => {

describe('useDoBuilder()', () => {
const setup = () => {
const element = new FileElementNode({ caption: 'Test' });
const element = new FileElementNode({ caption: 'Test', alternativeText: 'altTest' });
const builder: DeepMocked<BoardDoBuilder> = createMock<BoardDoBuilder>();
const elementDo = fileElementFactory.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@ export class FileElementNode extends BoardNode {
@Property()
caption: string;

@Property()
alternativeText: string;

constructor(props: FileElementNodeProps) {
super(props);
this.type = BoardNodeType.FILE_ELEMENT;
this.caption = props.caption;
this.alternativeText = props.alternativeText;
}

useDoBuilder(builder: BoardDoBuilder): AnyBoardDo {
Expand All @@ -23,4 +27,5 @@ export class FileElementNode extends BoardNode {

export interface FileElementNodeProps extends BoardNodeProps {
caption: string;
alternativeText: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export const fileElementNodeFactory = BaseFactory.define<FileElementNode, FileEl
({ sequence }) => {
return {
caption: `caption #${sequence}`,
alternativeText: `alternativeText #${sequence}`,
};
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const fileElementFactory = BaseFactory.define<FileElement, FileElementPro
id: new ObjectId().toHexString(),
children: [],
caption: `<p>caption #${sequence}</p>`,
alternativeText: `alternativeText #${sequence}`,
createdAt: new Date(),
updatedAt: new Date(),
};
Expand Down
Loading