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

Communication: Allow users to paste images from the clipboard #9637

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,8 @@ export class MarkdownEditorMonacoComponent implements AfterContentInit, AfterVie
if (action instanceof FullscreenAction) {
// We include the full element if the initial height is set to 'external' so the editor is resized to fill the screen.
action.element = this.isInitialHeightExternal() ? this.fullElement.nativeElement : this.wrapper.nativeElement;
} else if (this.enableFileUpload && action instanceof AttachmentAction) {
action.setUploadCallback(this.embedFiles.bind(this));
}
this.monacoEditor.registerAction(action);
});
Expand Down Expand Up @@ -488,6 +490,9 @@ export class MarkdownEditorMonacoComponent implements AfterContentInit, AfterVie
* @param inputElement The input element that contains the files. If provided, the input element will be reset.
*/
embedFiles(files: File[], inputElement?: HTMLInputElement): void {
if (!this.enableFileUpload) {
return;
}
files.forEach((file) => {
(this.useCommunicationForFileUpload()
? this.fileUploaderService.uploadMarkdownFileInCurrentMetisConversation(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,12 @@ export class MonacoTextEditorAdapter implements TextEditor {
this.editor.revealRangeInCenter(this.toMonacoRange(range));
}

addPasteListener(callback: (insertedText: string) => void | Promise<void>): Disposable {
return this.editor.onDidPaste((pasteEvent) => {
callback(this.getTextAtRange(this.fromMonacoRange(pasteEvent.range)));
});
}
pzdr7 marked this conversation as resolved.
Show resolved Hide resolved

private toMonacoPosition(position: TextEditorPosition): monaco.IPosition {
return new monaco.Position(position.getLineNumber(), position.getColumn());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,11 @@ export interface TextEditor {
* @return A disposable that can be used to remove the completer from the editor.
*/
addCompleter<ItemType>(completer: TextEditorCompleter<ItemType>): Disposable;

/**
* Adds a listener to the editor that is triggered after the user pastes something.
* @param callback The callback to execute after the user has pasted something.
* @return A disposable that can be used to remove the listener from the editor.
*/
addPasteListener(callback: (insertedText: string) => void | Promise<void>): Disposable;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { faImage } from '@fortawesome/free-solid-svg-icons';
import { TextEditorAction } from 'app/shared/monaco-editor/model/actions/text-editor-action.model';
import { TextEditor } from 'app/shared/monaco-editor/model/actions/adapter/text-editor.interface';
import { sanitizeStringForMarkdownEditor } from 'app/shared/util/markdown.util';
import { Disposable } from 'app/shared/monaco-editor/model/actions/monaco-editor.util';
import { TranslateService } from '@ngx-translate/core';

interface AttachmentArguments {
text: string;
Expand All @@ -14,10 +16,52 @@ interface AttachmentArguments {
export class AttachmentAction extends TextEditorAction {
static readonly ID = 'attachment.action';
static readonly DEFAULT_INSERT_TEXT = '![](https://)';

private disposablePasteListener?: Disposable;
private uploadCallback?: (files: File[]) => void;

constructor() {
super(AttachmentAction.ID, 'artemisApp.multipleChoiceQuestion.editor.imageUpload', faImage, undefined);
}

/**
* Sets the callback to be called when files are pasted into the editor. The callback will be reset to undefined when the action is disposed.
* @param callback The callback to call when files are pasted into the editor.
*/
setUploadCallback(callback?: (files: File[]) => void) {
this.uploadCallback = callback;
}

register(editor: TextEditor, translateService: TranslateService) {
super.register(editor, translateService);
this.disposablePasteListener = editor.addPasteListener(async (insertedText: string) => {
// We do not read from the clipboard if the user pasted text. This prevents an unnecessary prompt on Firefox.
if (!this.uploadCallback || insertedText) {
return;
}
const clipboardItems = await navigator.clipboard.read();
const files: File[] = [];
for (const clipboardItem of clipboardItems) {
for (const type of clipboardItem.types) {
if (type.startsWith('image/')) {
// Map image type to extension.
const extension = type.replace('image/', '');
const blob = await clipboardItem.getType(type);
files.push(new File([blob], `image.${extension}`, { type }));
break;
}
}
}
this.uploadCallback(files);
});
pzdr7 marked this conversation as resolved.
Show resolved Hide resolved
}

dispose() {
super.dispose();
this.disposablePasteListener?.dispose();
this.uploadCallback = undefined;
}

/**
* Executes the action in the current editor with the given arguments (url and text).
* @param args The text and url of the attachment to insert. If one or both are not provided, the default text will be inserted.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,21 @@ describe('MarkdownEditorMonacoComponent', () => {
expect(alertSpy).toHaveBeenCalledOnce();
}));

it('should set the upload callback on the attachment actions', () => {
const attachmentAction = new AttachmentAction();
const setUploadCallbackSpy = jest.spyOn(attachmentAction, 'setUploadCallback');
const embedFilesStub = jest.spyOn(comp, 'embedFiles').mockImplementation();
comp.defaultActions = [attachmentAction];
comp.enableFileUpload = true;
fixture.detectChanges();
expect(setUploadCallbackSpy).toHaveBeenCalledOnce();
// Check if the correct function is passed to the action.
const argument = setUploadCallbackSpy.mock.calls[0][0];
expect(argument).toBeDefined();
argument!([]);
expect(embedFilesStub).toHaveBeenCalledExactlyOnceWith([]);
});

it('should embed image and .pdf files', fakeAsync(() => {
const urlAction = new UrlAction();
const urlStub = jest.spyOn(urlAction, 'executeInCurrentEditor').mockImplementation();
Expand Down Expand Up @@ -179,6 +194,29 @@ describe('MarkdownEditorMonacoComponent', () => {
expect(urlStub).toHaveBeenCalledExactlyOnceWith({ url: fileInformation[1].url, text: fileInformation[1].file.name });
}));

it('should not embed files if file upload is disabled', () => {
const urlAction = new UrlAction();
const urlStub = jest.spyOn(urlAction, 'executeInCurrentEditor').mockImplementation();
const attachmentAction = new AttachmentAction();
const attachmentStub = jest.spyOn(attachmentAction, 'executeInCurrentEditor').mockImplementation();
const files = [new File([''], 'test.png'), new File([''], 'test.pdf')];
comp.defaultActions = [urlAction, attachmentAction];
comp.enableFileUpload = false;
fixture.detectChanges();
comp.embedFiles(files);
expect(urlStub).not.toHaveBeenCalled();
expect(attachmentStub).not.toHaveBeenCalled();
});

it('should execute the action when clicked', () => {
const action = new UrlAction();
const executeInCurrentEditorStub = jest.spyOn(action, 'executeInCurrentEditor').mockImplementation();
comp.defaultActions = [action];
fixture.detectChanges();
comp.handleActionClick(new MouseEvent('click'), action);
expect(executeInCurrentEditorStub).toHaveBeenCalledOnce();
});

it('should open the color selector', () => {
fixture.detectChanges();
const openColorSelectorSpy = jest.spyOn(comp.colorSelector, 'openColorSelector');
Expand Down
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There actually weren't any tests for this service, so I added some

Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { provideHttpClient } from '@angular/common/http';
import { TestBed } from '@angular/core/testing';
import { FileUploaderService } from 'app/shared/http/file-uploader.service';
import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing';
import { MAX_FILE_SIZE, MAX_FILE_SIZE_COMMUNICATION } from '../../../../../../main/webapp/app/shared/constants/input.constants';

describe('FileUploaderService', () => {
let service: FileUploaderService;
let httpMock: HttpTestingController;

beforeEach(() => {
TestBed.configureTestingModule({
imports: [],
providers: [provideHttpClient(), provideHttpClientTesting()],
});

service = TestBed.inject(FileUploaderService);
httpMock = TestBed.inject(HttpTestingController);
});

it('should upload a regular file for the markdown editor', async () => {
const file = new File([''], 'test.pdf', { type: 'application/pdf' });
const expectedResponse = { path: 'some-path' };
const promise = service.uploadMarkdownFile(file);

const request = httpMock.expectOne({ method: 'POST', url: '/api/markdown-file-upload' });
request.flush(expectedResponse);

httpMock.verify();
await expect(promise).resolves.toEqual(expectedResponse);
});

it('should upload a regular file for communication', async () => {
const file = new File([''], 'test.pdf', { type: 'application/pdf' });
const expectedResponse = { path: 'some-path' };
const promise = service.uploadMarkdownFileInCurrentMetisConversation(file, 1, 2);

const request = httpMock.expectOne({ method: 'POST', url: '/api/files/courses/1/conversations/2' });
request.flush(expectedResponse);

httpMock.verify();
await expect(promise).resolves.toEqual(expectedResponse);
});

it('should reject if the course for communication is not specified', async () => {
const file = new File([''], 'test.pdf', { type: 'application/pdf' });
await expect(service.uploadMarkdownFileInCurrentMetisConversation(file, undefined, 2)).rejects.toThrow(Error);
});

it('should reject if the conversation for communication is not specified', async () => {
const file = new File([''], 'test.pdf', { type: 'application/pdf' });
await expect(service.uploadMarkdownFileInCurrentMetisConversation(file, 1, undefined)).rejects.toThrow(Error);
});

it('should reject files with unsupported extensions', async () => {
const file = new File([''], 'test.docx', { type: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' });
await expect(service.uploadMarkdownFile(file)).rejects.toThrow(Error);
});

it('should reject files that are too large (general)', async () => {
const largeFile = new File([''], 'test.pdf', { type: 'application/pdf' });
// Overwrite the size property to be larger than the maximum allowed size
Object.defineProperty(largeFile, 'size', { value: MAX_FILE_SIZE + 1 });
await expect(service.uploadMarkdownFile(largeFile)).rejects.toThrow(Error);
});

it('should reject files that are too large (communication)', async () => {
const largeFile = new File([''], 'test.pdf', { type: 'application/pdf' });
// Overwrite the size property to be larger than the maximum allowed size
Object.defineProperty(largeFile, 'size', { value: MAX_FILE_SIZE_COMMUNICATION + 1 });
await expect(service.uploadMarkdownFileInCurrentMetisConversation(largeFile, 1, 2)).rejects.toThrow(Error);
});
pzdr7 marked this conversation as resolved.
Show resolved Hide resolved
});
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ describe('PostingsMarkdownEditor', () => {
setSelection: jest.fn(),
revealRange: jest.fn(),
addCompleter: jest.fn(),
addPasteListener: jest.fn(),
};

const mockPositionStrategy = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,30 @@ import { AttachmentAction } from 'app/shared/monaco-editor/model/actions/attachm
import { OrderedListAction } from 'app/shared/monaco-editor/model/actions/ordered-list.action';
import { UnorderedListAction } from 'app/shared/monaco-editor/model/actions/unordered-list.action';
import * as monaco from 'monaco-editor';
import { MockClipboardItem } from '../../../helpers/mocks/service/mock-clipboard-item';

describe('MonacoEditorActionIntegration', () => {
let fixture: ComponentFixture<MonacoEditorComponent>;
let comp: MonacoEditorComponent;

beforeEach(() => {
TestBed.configureTestingModule({
beforeEach(async () => {
await TestBed.configureTestingModule({
imports: [ArtemisTestModule, MonacoEditorComponent],
})
.compileComponents()
.then(() => {
fixture = TestBed.createComponent(MonacoEditorComponent);
comp = fixture.componentInstance;
global.ResizeObserver = jest.fn().mockImplementation((callback: ResizeObserverCallback) => {
return new MockResizeObserver(callback);
});
fixture.detectChanges();
});
}).compileComponents();

fixture = TestBed.createComponent(MonacoEditorComponent);
comp = fixture.componentInstance;
global.ResizeObserver = jest.fn().mockImplementation((callback: ResizeObserverCallback) => {
return new MockResizeObserver(callback);
});

Object.assign(navigator, {
clipboard: {
read: jest.fn(),
},
});

fixture.detectChanges();
});

afterEach(() => {
Expand Down Expand Up @@ -66,6 +72,46 @@ describe('MonacoEditorActionIntegration', () => {
expect(comp.getText()).toBe(defaultText);
});

it('should not access the clipboard if no upload callback is specified', async () => {
const clipboardReadSpy = jest.spyOn(navigator.clipboard, 'read');
const addPasteListenerSpy = jest.spyOn(comp['textEditorAdapter'], 'addPasteListener');
const action = new AttachmentAction();
comp.registerAction(action);
// The addPasteListenerSpy should have received a function that does not result in the clipboard being read when called.
expect(addPasteListenerSpy).toHaveBeenCalled();
const pasteListener = addPasteListenerSpy.mock.calls[0][0];
expect(pasteListener).toBeDefined();
await pasteListener('');
expect(clipboardReadSpy).not.toHaveBeenCalled();
});

it('should process files from the clipboard', async () => {
const imageBlob = new Blob([]);
const imageClipboardItem: MockClipboardItem = {
types: ['image/png'],
getType: jest.fn().mockResolvedValue(imageBlob),
};

const nonImageBlob = new Blob(['Sample text content']);
const textClipboardItem: MockClipboardItem = {
types: ['text/plain'],
getType: jest.fn().mockResolvedValue(nonImageBlob),
};

// Mock the clipboard read function to return the created ClipboardItems
const clipboardReadSpy = jest.spyOn(navigator.clipboard, 'read').mockResolvedValue([imageClipboardItem, textClipboardItem]);
const addPasteListenerSpy = jest.spyOn(comp['textEditorAdapter'], 'addPasteListener');
const uploadCallback = jest.fn();
const action = new AttachmentAction();
action.setUploadCallback(uploadCallback);
comp.registerAction(action);
const pasteListener = addPasteListenerSpy.mock.calls[0][0];
expect(pasteListener).toBeDefined();
await pasteListener('');
expect(clipboardReadSpy).toHaveBeenCalledOnce();
expect(uploadCallback).toHaveBeenCalledExactlyOnceWith([new File([imageBlob], 'image.png', { type: 'image/png' })]);
});

it('should insert unordered list', () => {
const action = new UnorderedListAction();
comp.registerAction(action);
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
export class MockClipboardItem {}
export class MockClipboardItem {
types: string[];

pzdr7 marked this conversation as resolved.
Show resolved Hide resolved
getType(_type: string): Promise<Blob> {
return Promise.resolve(new Blob());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefix this with _ here to avoid a warning for an unused variable while still preserving the signature

}
}
pzdr7 marked this conversation as resolved.
Show resolved Hide resolved
Loading