From dd74c20b77c99d5747b632658449acc8f8d6438c Mon Sep 17 00:00:00 2001 From: nathan Date: Thu, 27 Feb 2025 15:17:36 -0700 Subject: [PATCH] Upload widgets: - Build conflict check & prompt into Et2VfsUpload widget - Doc updates --- api/js/etemplate/Et2File/Et2File.md | 11 +- api/js/etemplate/Et2File/Et2File.ts | 13 +- api/js/etemplate/Et2Vfs/Et2VfsUpload.md | 19 ++- api/js/etemplate/Et2Vfs/Et2VfsUpload.ts | 88 ++++++++++- .../Et2Vfs/test/Et2VfsUpload.test.ts | 143 +++++++++++++++++- api/src/Etemplate/Widget/Vfs.php | 90 +++++++++-- 6 files changed, 340 insertions(+), 24 deletions(-) diff --git a/api/js/etemplate/Et2File/Et2File.md b/api/js/etemplate/Et2File/Et2File.md index 1e58ab9a35..18e6be5b82 100644 --- a/api/js/etemplate/Et2File/Et2File.md +++ b/api/js/etemplate/Et2File/Et2File.md @@ -8,9 +8,18 @@ > ``` -File allows the user to upload files to EGroupware. The uploaded files are processed on the server when the form is +File allows the user to upload files to EGroupware. The uploaded files are processed on the server by the application +after the form is submitted. As files are selected, they will be shown in a list with [FileItem](../et2-file-item) +:::tip +There are two widgets for uploading files, [File](../et2-file) and [VfsUpload](../et2-vfs-upload). + +Use `File` when you don't know where in the VFS the file will be stored or don't intend to store it. + +Use `VfsUpload` otherwise. +::: + ## Examples ### Icon diff --git a/api/js/etemplate/Et2File/Et2File.ts b/api/js/etemplate/Et2File/Et2File.ts index 2771e07cf2..5fc226a453 100644 --- a/api/js/etemplate/Et2File/Et2File.ts +++ b/api/js/etemplate/Et2File/Et2File.ts @@ -22,6 +22,7 @@ export interface FileInfo extends ResumableFile path? : string; // ResumableFile + fileName : string; uniqueIdentifier : string; file : File; progress? : Function; @@ -274,7 +275,7 @@ export class Et2File extends Et2InputWidget(LitElement) return fileItem; } - private async resumableFileAdded(file : FileInfo, event) + protected async resumableFileAdded(file : FileInfo, event) { file = { accepted: true, @@ -322,7 +323,7 @@ export class Et2File extends Et2InputWidget(LitElement) } } - private resumableFileProgress(file : FileInfo, event) + protected resumableFileProgress(file : FileInfo, event) { const fileItem = this.findFileItem(file); if(fileItem && file.progress()) @@ -337,7 +338,7 @@ export class Et2File extends Et2InputWidget(LitElement) } } - private resumableFileComplete(file : FileInfo, jsonResponse) + protected resumableFileComplete(file : FileInfo, jsonResponse) { const response = ((JSON.parse(jsonResponse)['response'] ?? {}).find(i => i['type'] == "data") ?? {})['data'] ?? {}; const fileItem = this.findFileItem(file); @@ -400,7 +401,7 @@ export class Et2File extends Et2InputWidget(LitElement) } } - private resumableFileError(file, message) + protected resumableFileError(file, message) { const fileItem = this.findFileItem(file); fileItem.loading = false; @@ -412,7 +413,7 @@ export class Et2File extends Et2InputWidget(LitElement) fileItem.requestUpdate("loading"); } - private resumableUploadComplete() + protected resumableUploadComplete() { this.requestUpdate(); this.updateComplete.then(() => @@ -616,7 +617,7 @@ export class Et2File extends Et2InputWidget(LitElement) fileItemTemplate(fileInfo : FileInfo, index) { - const label = (fileInfo.accepted ? fileInfo.file.name : fileInfo.warning) ?? fileInfo['name']; + const label = (fileInfo.accepted ? (fileInfo['name'] ?? fileInfo.file.name) : fileInfo.warning) ?? fileInfo['name']; let icon = fileInfo.icon ?? (fileInfo.warning ? "exclamation-triangle" : undefined); // Pull thumbnail from file if we can diff --git a/api/js/etemplate/Et2Vfs/Et2VfsUpload.md b/api/js/etemplate/Et2Vfs/Et2VfsUpload.md index 20f633752c..08f6440dce 100644 --- a/api/js/etemplate/Et2Vfs/Et2VfsUpload.md +++ b/api/js/etemplate/Et2Vfs/Et2VfsUpload.md @@ -9,12 +9,25 @@ ``` VFS Upload allows the user to upload files to a specified location in the VFS. It works much the same -as [File](../et2-file), but files go directly into the VFS without the application needing to handle them. +as [File](../et2-file), but there are differences: + +1. Files go directly into the VFS without the application needing to handle them. With the File widget the file is + stored temporarily and the application must move it. +2. Any operations (save, delete, replace existing file) are handled directly. With the File widget, the application must + handle this. Any option for File will also work for VfsUpload. -`VfsUpload` does return file information to the application since all file actions are done immediately via -AJAX +`VfsUpload` does not return file information to the application since all file actions are done immediately via +AJAX. + +:::tip +There are two widgets for uploading files, [File](../et2-file) and [VfsUpload](../et2-vfs-upload). + +Use `File` when you don't know where in the VFS the file will be stored or don't intend to store it. + +Use `VfsUpload` otherwise. +::: ## Examples diff --git a/api/js/etemplate/Et2Vfs/Et2VfsUpload.ts b/api/js/etemplate/Et2Vfs/Et2VfsUpload.ts index 13683acdba..137d6b1bbc 100644 --- a/api/js/etemplate/Et2Vfs/Et2VfsUpload.ts +++ b/api/js/etemplate/Et2Vfs/Et2VfsUpload.ts @@ -1,4 +1,4 @@ -import {Et2File, FileInfo as UploadFileInfo} from "../Et2File/Et2File"; +import {Et2File, FileInfo, FileInfo as UploadFileInfo} from "../Et2File/Et2File"; import {customElement} from "lit/decorators/custom-element.js"; import {property} from "lit/decorators/property.js"; import {Et2Dialog} from "../Et2Dialog/Et2Dialog"; @@ -25,6 +25,7 @@ export type VfsFileInfo = UploadFileInfo & DialogFileInfo; @customElement('et2-vfs-upload') export class Et2VfsUpload extends Et2File { + @property({type: String}) conflict : "overwrite" | "rename" | "ask" = "ask"; private __path = "" @@ -41,6 +42,10 @@ export class Et2VfsUpload extends Et2File }); } + /** + * Target VFS path. Specifying a directory will allow multiple files. Including the filename will rename the file. + * @param {string} newPath + */ @property({type: String}) set path(newPath : string) { @@ -48,7 +53,7 @@ export class Et2VfsUpload extends Et2File this.multiple = this.__path.endsWith("/"); } - get path() { return this.__path; } + get path() : string { return this.__path; } handleFileRemove(info : VfsFileInfo) { @@ -100,6 +105,85 @@ export class Et2VfsUpload extends Et2File }); } + resumableFileAdded(info : FileInfo, event) + { + const superAdded = super.resumableFileAdded.bind(this); + // If we're not just overwriting, check + if(this.conflict == "overwrite") + { + return superAdded(info, event); + } + this.egw().request("EGroupware\\Api\\Etemplate\\Widget\\Vfs::ajax_conflict_check", [ + this.getInstanceManager()?.etemplate_exec_id, // request_id + this.path, // path + info.file.name, + info.file.type + ]).then(async(data) => + { + if(data && data.exists && this.conflict == "rename" && data.filename) + { + info.fileName = data.filename; + } + else if(data && data.exists && this.conflict == "ask") + { + const upload = await this.confirmConflict(info, data.filename ?? info.fileName); + if(!upload) + { + return; + } + } + return superAdded(info, event); + }); + } + + protected async confirmConflict(info : FileInfo, suggestedName : string) + { + const buttons = [ + { + label: this.egw().lang("Overwrite"), + id: "overwrite", + class: "ui-priority-primary", + "default": true, + image: 'check' + }, + {label: this.egw().lang("Rename"), id: "rename", image: 'edit'}, + {label: this.egw().lang("Cancel"), id: "cancel", image: "cancel"} + ]; + let button_id, value; + if(this.path.endsWith("/")) + { + // Filename is up to user, let them rename + [button_id, value] = <[string, Object]>await Et2Dialog.show_prompt(undefined, + this.egw().lang('Do you want to overwrite existing file %1 in directory %2?', info.fileName, this.path), + this.egw().lang('File %1 already exists', info.fileName), + suggestedName ?? info.fileName, buttons, this.egw() + ).getComplete(); + } + else + { + // Filename is set, only ask to overwrite + buttons.splice(1, 1); + info.fileName = suggestedName ?? info.fileName; + [button_id, value] = <[string, Object]>await Et2Dialog.show_dialog(undefined, + this.egw().lang('Do you want to overwrite existing file %1 in directory %2?', info.fileName, this.label ?? this.title ?? this.path), + this.egw().lang('File %1 already exists', info.fileName), + undefined, buttons, Et2Dialog.QUESTION_MESSAGE, "", this.egw() + ).getComplete(); + } + switch(button_id) + { + case "overwrite": + // Upload as set + return true; + case "rename": + info.fileName = value?.value ?? info.fileName; + return true; + case "cancel": + // Don't upload + return false; + } + } + protected confirmDelete(info : VfsFileInfo) { const confirm = Et2Dialog.show_dialog(undefined, this.egw().lang("Delete file") + "?", diff --git a/api/js/etemplate/Et2Vfs/test/Et2VfsUpload.test.ts b/api/js/etemplate/Et2Vfs/test/Et2VfsUpload.test.ts index 5b1d7b4dff..2349f1ec69 100644 --- a/api/js/etemplate/Et2Vfs/test/Et2VfsUpload.test.ts +++ b/api/js/etemplate/Et2Vfs/test/Et2VfsUpload.test.ts @@ -1,7 +1,8 @@ -import {assert, fixture, html} from '@open-wc/testing'; +import {assert, fixture, html, oneEvent} from '@open-wc/testing'; import * as sinon from "sinon"; import {Et2VfsUpload, VfsFileInfo} from "../Et2VfsUpload"; import {Et2FileItem} from "../../Et2File/Et2FileItem"; +import {Et2Dialog} from "../../Et2Dialog/Et2Dialog"; window.egw = { ajaxUrl: (url) => url, @@ -69,7 +70,7 @@ describe('Et2VfsUpload', async() => element.egw = () => mockEgw; await element.updateComplete; - const confirmStub = sinon.stub(element, 'confirmDelete').resolves([true, undefined]); + const confirmStub = sinon.stub(element, 'confirmDelete').resolves([Et2Dialog.YES_BUTTON, undefined]); const removeStub = sinon.stub(element, 'handleFileRemove').callThrough(); await element.handleFileRemove(fileInfo); @@ -115,7 +116,7 @@ describe('Et2VfsUpload', async() => }; element.egw = () => mockEgw; - const confirmStub = sinon.stub().resolves([true, undefined]); + const confirmStub = sinon.stub().resolves([Et2Dialog.YES_BUTTON, undefined]); sinon.stub(element, 'confirmDelete').callsFake(confirmStub); await element.handleFileRemove(fileInfo); @@ -125,3 +126,139 @@ describe('Et2VfsUpload', async() => assert(mockEgw.message.calledOnceWith('Error deleting file', 'error'), 'Error message should be displayed'); }); }); + +describe('Et2VfsUpload existing file checks', async() => +{ + let element : Et2VfsUpload; + let addSpy; + let completeSpy; + + beforeEach(async() => + { + element = /** @type {Et2VfsUpload} */ (await fixture(html` + `)); + addSpy = sinon.spy(); + element.addEventListener("et2-add", addSpy); + element.addEventListener("change", completeSpy); + }); + + it('Should check for existing file', async() => + { + // Ask is the default, but set it anyway + element.conflict = "ask"; + + const fileInfo : File = { + name: 'file.txt', + type: 'text/plain', + size: 1 + }; + const mockEgw = { + ...window.egw, + lang: sinon.stub().returnsArg(0), + request: sinon.stub().resolves({errs: 0, exists: true}), + message: sinon.stub() + }; + element.egw = () => mockEgw; + + const confirmStub = sinon.stub(element, 'confirmConflict').resolves(true); + + element.addFile(fileInfo); + await element.updateComplete; + await mockEgw.request.returnValues[0]; + + assert(mockEgw.request.calledOnce, 'Request to see if file exists should be sent'); + assert(confirmStub.calledOnce, 'User should be asked about overwriting'); + await oneEvent(element, "change"); + assert(addSpy.calledOnce, 'File upload should proceed'); + }); + + it('Should not check if conflict is "overwrite"', async() => + { + + element.conflict = "overwrite"; + + const fileInfo : File = { + name: 'file.txt', + type: 'text/plain', + size: 1 + }; + const mockEgw = { + ...window.egw, + lang: sinon.stub().returnsArg(0), + request: sinon.stub().resolves({errs: 0, exists: true}), + message: sinon.stub() + }; + element.egw = () => mockEgw; + + const confirmStub = sinon.stub(element, 'confirmConflict').resolves(true); + + element.addFile(fileInfo); + await element.updateComplete; + await mockEgw.request.returnValues[0]; + + assert(mockEgw.request.notCalled, 'Request to see if file exists should not be sent'); + assert(confirmStub.notCalled, 'User should not be asked about overwriting'); + await oneEvent(element, "change"); + assert(addSpy.calledOnce, 'File upload should proceed'); + }); + + it('Should not ask if conflict is "rename"', async() => + { + element.conflict = "rename"; + + const fileInfo : File = { + name: 'file.txt', + type: 'text/plain', + size: 1 + }; + const mockEgw = { + ...window.egw, + lang: sinon.stub().returnsArg(0), + request: sinon.stub().resolves({errs: 0, exists: true}), + message: sinon.stub() + }; + element.egw = () => mockEgw; + + const confirmStub = sinon.stub(element, 'confirmConflict').resolves(true); + + element.addFile(fileInfo); + await element.updateComplete; + await mockEgw.request.returnValues[0]; + + assert(mockEgw.request.calledOnce, 'Request to see if file exists should be sent'); + assert(confirmStub.notCalled, 'User should not be asked about overwriting'); + + await oneEvent(element, "change"); + assert(addSpy.calledOnce, 'File upload should proceed'); + }); + + it('Should not upload if they cancel conflict dialog', async() => + { + // Ask is the default, but set it anyway + element.conflict = "ask"; + const fileInfo : File = { + name: 'file.txt', + type: 'text/plain', + size: 1 + }; + const mockEgw = { + ...window.egw, + lang: sinon.stub().returnsArg(0), + request: sinon.stub().resolves({errs: 0, exists: true}), + message: sinon.stub() + }; + element.egw = () => mockEgw; + + // Stub the parent's async resumableFileAdded method + let parentFileAdded = sinon.spy(Object.getPrototypeOf(element), "resumableFileAdded"); + const confirmStub = sinon.stub(element, 'confirmConflict').resolves(false); + + element.addFile(fileInfo); + await element.updateComplete; + await mockEgw.request.returnValues[0]; + + assert(mockEgw.request.calledOnce, 'Request to see if file exists should be sent'); + assert(confirmStub.calledOnce, 'User should be asked about overwriting'); + assert(addSpy.notCalled, 'File upload should not proceed'); + }) +}); diff --git a/api/src/Etemplate/Widget/Vfs.php b/api/src/Etemplate/Widget/Vfs.php index 0a0adbae9b..e10972e6ab 100644 --- a/api/src/Etemplate/Widget/Vfs.php +++ b/api/src/Etemplate/Widget/Vfs.php @@ -139,6 +139,65 @@ class Vfs extends File parent::ajax_upload(); } + /** + * Check to see if the file already exists before we start uploading it. + * If it does, it returns a suggested alternate filename. + * @param $request_id + * @param $path + * @return void + */ + public static function ajax_conflict_check($request_id, $path, $filename, $mimetype) + { + $response = Api\Json\Response::get(); + $request_id = str_replace(' ', '+', rawurldecode($request_id)); + $response_data = array('errs' => 0); + if(!self::$request = Etemplate\Request::read($request_id)) + { + $response->error("Could not read session"); + return; + } + if($path[0] !== '/') + { + $path = self::get_vfs_path($path); + } + if(!Api\Vfs::is_writable($path)) + { + $response_data['msg'] = 'Permission denied'; + } + else + { + // Path is to a single file + if(!str_ends_with($path, '/')) + { + $response_data['exists'] = Api\Vfs::file_exists($path); + + $extFilename = static::addExtension($path, ['name' => $filename, 'mime' => $mimetype]); + // Check for anything matching, ignoring extension + if($filename != $extFilename) + { + $response_data['filename'] = $extFilename; + $existing = Api\Vfs::find(Api\Vfs::dirname($path), array('type' => 'f', 'maxdepth' => 1, + 'name' => Api\Vfs::basename($path) . '.*')); + if($existing) + { + $response_data['exists'] = true; + } + } + } + elseif(Api\Vfs::is_dir($path)) + { + $response_data['exists'] = Api\Vfs::file_exists($path . $filename); + + if($response_data['exists'] && !$response_data['filename']) + { + $response_data['filename'] = Api\Vfs::basename(Api\Vfs::make_unique($path . $filename)); + } + } + } + + $response->data($response_data); + } + public static function ajax_remove($request_id, $widget_id, $path) { $response = Api\Json\Response::get(); @@ -330,16 +389,9 @@ class Vfs extends File $filename = $file['name']; if ($path && substr($path,-1) !== '/') { - $parts = explode('.', $filename); // check if path already contains a valid extension --> don't add another one - $path_parts = explode('.', Api\Vfs::basename($path)); - if ((!($path_ext = array_pop($path_parts)) || Api\MimeMagic::ext2mime($path_ext) === 'application/octet-stream') && - (($extension = array_pop($parts) ?: Api\MimeMagic::mime2ext($file['mime'])) && $extension != $filename)) - { - // add extension to path - $path .= '.'.$extension; - } - $file['name'] = Api\Vfs::basename($path); + $file['name'] = static::addExtension($path, $file); + $path = rtrim($path, $filename) . $file['name']; } else if ($path) // multiple upload with dir given (trailing slash) { @@ -456,6 +508,26 @@ class Vfs extends File return $path; } + /** + * Sometimes target path has a filename but no extension. Add the appropriate extension for the file. + * @param $path + * @param $filename + * @return void + */ + protected static function addExtension($path, $file) + { + $parts = explode('.', $file['name']); + // check if path already contains a valid extension --> don't add another one + $path_parts = explode('.', Api\Vfs::basename($path)); + if((!($path_ext = array_pop($path_parts)) || Api\MimeMagic::ext2mime($path_ext) === 'application/octet-stream') && + (($extension = array_pop($parts) ?: Api\MimeMagic::mime2ext($file['mime'])) && $extension != $file['name'])) + { + // add extension to path + $path .= '.' . $extension; + } + return Api\Vfs::basename($path); + } + /** * This function behaves like etemplate app function for set/get content of * VFS Select Widget UI