From ec5c8d6cade3e922ce97682cc182083c2cb2c439 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Tue, 20 Aug 2024 09:00:00 +0200 Subject: [PATCH 1/3] MOBILE-4482 forum: Fix filtered content displayed when editing post When editing a post, the editor displayed the content already filtered, and it could also include some exra content that doesn't belong to the post, like plagiarism info. --- src/addons/mod/forum/components/post/post.ts | 50 +++++++++++- src/addons/mod/forum/services/forum.ts | 82 +++++++++++++++++++- 2 files changed, 127 insertions(+), 5 deletions(-) diff --git a/src/addons/mod/forum/components/post/post.ts b/src/addons/mod/forum/components/post/post.ts index 9e10b3c3504..258b00a22ad 100644 --- a/src/addons/mod/forum/components/post/post.ts +++ b/src/addons/mod/forum/components/post/post.ts @@ -35,6 +35,7 @@ import { AddonModForumDiscussion, AddonModForumPost, AddonModForumPostFormData, + AddonModForumPrepareDraftAreaForPostWSResponse, } from '../../services/forum'; import { CoreTag } from '@features/tag/services/tag'; import { Translate } from '@singletons'; @@ -47,7 +48,7 @@ import { AddonModForumOffline } from '../../services/forum-offline'; import { CoreUtils } from '@services/utils/utils'; import { CoreRatingInfo } from '@features/rating/services/rating'; import { CoreForms } from '@singletons/form'; -import { CoreFileEntry } from '@services/file-helper'; +import { CoreFileEntry, CoreFileHelper } from '@services/file-helper'; import { AddonModForumSharedPostFormData } from '../../pages/discussion/discussion'; import { CoreDom } from '@singletons/dom'; import { CoreAnalytics, CoreAnalyticsEventType } from '@services/analytics'; @@ -95,6 +96,8 @@ export class AddonModForumPostComponent implements OnInit, OnDestroy, OnChanges displaySubject = true; optionsMenuEnabled = false; + protected preparePostData?: AddonModForumPrepareDraftAreaForPostWSResponse; + constructor( protected elementRef: ElementRef, ) {} @@ -226,6 +229,10 @@ export class AddonModForumPostComponent implements OnInit, OnDestroy, OnChanges // Show advanced fields if any of them has not the default value. this.advanced = this.formData.files.length > 0; + + if (!isEditing || !postId || postId <= 0) { + this.preparePostData = undefined; + } } /** @@ -314,6 +321,28 @@ export class AddonModForumPostComponent implements OnInit, OnDestroy, OnChanges // Ask confirm if there is unsaved data. try { await this.confirmDiscard(); + } catch { + // Cancelled. + return; + } + + const modal = await CoreLoadings.show(); + + try { + let message = this.post.message; + + if (this.post.id > 0) { + // Call prepare post for edition to retrieve the message without any added content (like filters and plagiarism). + this.preparePostData = await AddonModForum.preparePostForEdition(this.post.id, 'post'); + + const { text } = CoreFileHelper.replaceDraftfileUrls( + CoreSites.getRequiredCurrentSite().getURL(), + this.preparePostData.messagetext, + this.post.messageinlinefiles?.length ? this.post.messageinlinefiles : (this.preparePostData.files ?? []), + ); + + message = text; + } this.formData.syncId = AddonModForumSync.getDiscussionSyncId(this.discussionId); CoreSync.blockOperation(ADDON_MOD_FORUM_COMPONENT, this.formData.syncId); @@ -322,7 +351,7 @@ export class AddonModForumPostComponent implements OnInit, OnDestroy, OnChanges this.post.parentid, true, this.post.subject, - this.post.message, + message, this.post.attachments, this.post.isprivatereply, this.post.id > 0 ? this.post.id : undefined, @@ -331,8 +360,10 @@ export class AddonModForumPostComponent implements OnInit, OnDestroy, OnChanges this.scrollToForm(); this.analyticsLogEvent('mod_forum_update_discussion_post', `/mod/forum/post.php?edit=${this.post.id}`); - } catch { - // Cancelled. + } catch (error) { + CoreDomUtils.showErrorModalDefault(error, 'addon.mod_forum.errorgetpost', true); + } finally { + modal.dismiss(); } } @@ -369,6 +400,16 @@ export class AddonModForumPostComponent implements OnInit, OnDestroy, OnChanges const isEditOnline = this.formData.id && this.formData.id > 0; const modal = await CoreLoadings.show('core.sending', true); + if (isEditOnline && this.preparePostData) { + // Restore the draft file URLs, otherwise the treated URLs would be saved in the content, which can cause problems. + message = CoreFileHelper.restoreDraftfileUrls( + CoreSites.getRequiredCurrentSite().getURL(), + message, + this.preparePostData.messagetext, + this.post.messageinlinefiles?.length ? this.post.messageinlinefiles : (this.preparePostData.files ?? []), + ); + } + // Add some HTML to the message if needed. message = CoreText.formatHtmlLines(message); @@ -401,6 +442,7 @@ export class AddonModForumPostComponent implements OnInit, OnDestroy, OnChanges if (isEditOnline) { sent = await AddonModForum.updatePost(this.formData.id!, subject, message, { attachmentsid: attachments, + inlineattachmentsid: this.preparePostData?.draftitemid, }); } else if (saveOffline) { // Save post in offline. diff --git a/src/addons/mod/forum/services/forum.ts b/src/addons/mod/forum/services/forum.ts index 3cec9605b49..7f911724fb4 100644 --- a/src/addons/mod/forum/services/forum.ts +++ b/src/addons/mod/forum/services/forum.ts @@ -26,7 +26,13 @@ import { CoreGroups } from '@services/groups'; import { CoreSitesCommonWSOptions, CoreSites, CoreSitesReadingStrategy } from '@services/sites'; import { CoreUrl } from '@singletons/url'; import { CoreUtils } from '@services/utils/utils'; -import { CoreStatusWithWarningsWSResponse, CoreWSExternalFile, CoreWSExternalWarning, CoreWSStoredFile } from '@services/ws'; +import { + CoreStatusWithWarningsWSResponse, + CoreWSExternalFile, + CoreWSExternalWarning, + CoreWSFile, + CoreWSStoredFile, +} from '@services/ws'; import { makeSingleton, Translate } from '@singletons'; import { AddonModForumOffline, AddonModForumOfflineDiscussion, AddonModForumReplyOptions } from './forum-offline'; import { CoreSiteWSPreSets } from '@classes/sites/authenticated-site'; @@ -606,7 +612,11 @@ export class AddonModForumProvider { }; const site = await CoreSites.getSite(options.siteId); + const isGetDiscussionPostsAvailable = this.isGetDiscussionPostsAvailable(site); + if (isGetDiscussionPostsAvailable && site.isVersionGreaterEqualThan('4.0')) { + (params as AddonModForumGetDiscussionPostsWSParams).includeinlineattachments = true; + } const response = isGetDiscussionPostsAvailable ? await site.read('mod_forum_get_discussion_posts', params, preSets) @@ -1264,6 +1274,35 @@ export class AddonModForumProvider { CoreUser.storeUsers(CoreUtils.objectToArray(users)); } + /** + * Prepare post for edition. + * + * @param postId Post ID. + * @param area Area to prepare. + * @param options Other options. + * @returns Data of prepared area. + */ + async preparePostForEdition( + postId: number, + area: 'attachment'|'post', + options: AddonModForumPreparePostOptions = {}, + ): Promise { + const site = await CoreSites.getSite(options.siteId); + + const params: AddonModForumPrepareDraftAreaForPostWSParams = { + postid: postId, + area: area, + }; + if (options.filesToKeep?.length) { + params.filestokeep = options.filesToKeep.map(file => ({ + filename: file.filename ?? '', + filepath: file.filepath ?? '', + })); + } + + return await site.write('mod_forum_prepare_draft_area_for_post', params); + } + /** * Update a certain post. * @@ -1903,6 +1942,7 @@ export type AddonModForumGetDiscussionPostsWSParams = { discussionid: number; // The ID of the discussion from which to fetch posts. sortby?: string; // Sort by this element: id, created or modified. sortdirection?: string; // Sort direction: ASC or DESC. + includeinlineattachments?: boolean; // @since 4.0. Whether inline attachments should be included or not. }; /** @@ -2086,6 +2126,46 @@ export type AddonModForumUpdateDiscussionPostWSParams = { */ export type AddonModForumUpdateDiscussionPostWSResponse = CoreStatusWithWarningsWSResponse; +/** + * Params of mod_forum_prepare_draft_area_for_post WS. + */ +type AddonModForumPrepareDraftAreaForPostWSParams = { + postid: number; // Post to prepare the draft area for. + area: string; // Area to prepare: attachment or post. + draftitemid?: number; // The draft item id to use. 0 to generate one. + filestokeep?: AddonModForumFileToKeep[]; // Only keep these files in the draft file area. Empty for keeping all. +}; + +/** + * Data to pass to mod_forum_prepare_draft_area_for_post to keep a file in the area. + */ +type AddonModForumFileToKeep = { + filename: string; // File name. + filepath: string; // File path. +}; + +/** + * Data returned by mod_forum_prepare_draft_area_for_post WS. + */ +export type AddonModForumPrepareDraftAreaForPostWSResponse = { + draftitemid: number; // Draft item id for the file area. + files?: CoreWSExternalFile[]; + areaoptions: { // Draft file area options. + name: string; // Name of option. + value: string; // Value of option. + }[]; + messagetext: string; // Message text with URLs rewritten. + warnings?: CoreWSExternalWarning[]; +}; + +/** + * Options to pass to preparePostForEdition. + */ +export type AddonModForumPreparePostOptions = { + filesToKeep?: CoreWSFile[]; // Only keep these files in the draft file area. Undefined or empty array for keeping all. + siteId?: string; +}; + /** * Data passed to NEW_DISCUSSION_EVENT event. */ From 3d51b17e331d642375384cae51560da91489a693 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Tue, 20 Aug 2024 14:36:09 +0200 Subject: [PATCH 2/3] MOBILE-4482 forum: Avoid reuploading files when editing post --- src/addons/mod/forum/components/post/post.ts | 186 ++++++++++++------- 1 file changed, 122 insertions(+), 64 deletions(-) diff --git a/src/addons/mod/forum/components/post/post.ts b/src/addons/mod/forum/components/post/post.ts index 258b00a22ad..88acd59900e 100644 --- a/src/addons/mod/forum/components/post/post.ts +++ b/src/addons/mod/forum/components/post/post.ts @@ -39,7 +39,7 @@ import { } from '../../services/forum'; import { CoreTag } from '@features/tag/services/tag'; import { Translate } from '@singletons'; -import { CoreFileUploader } from '@features/fileuploader/services/fileuploader'; +import { CoreFileUploader, CoreFileUploaderStoreFilesResult } from '@features/fileuploader/services/fileuploader'; import { AddonModForumSync } from '../../services/forum-sync'; import { CoreSync } from '@services/sync'; import { CoreText } from '@singletons/text'; @@ -57,6 +57,7 @@ import { CoreToasts } from '@services/toasts'; import { toBoolean } from '@/core/transforms/boolean'; import { CorePopovers } from '@services/popovers'; import { CoreLoadings } from '@services/loadings'; +import { CoreWSFile } from '@services/ws'; /** * Components that shows a discussion post, its attachments and the action buttons allowed (reply, etc.). @@ -214,7 +215,7 @@ export class AddonModForumPostComponent implements OnInit, OnDestroy, OnChanges this.formData.isEditing = !!isEditing; this.formData.subject = subject || this.defaultReplySubject || ''; this.formData.message = message || null; - this.formData.files = files || []; + this.formData.files = (files ?? []).slice(); // Make a copy to avoid modifying the original array. this.formData.isprivatereply = !!isPrivate; this.formData.id = postId; @@ -392,10 +393,9 @@ export class AddonModForumPostComponent implements OnInit, OnDestroy, OnChanges return; } - let saveOffline = false; let message = this.formData.message; const subject = this.formData.subject; - const replyingTo = this.formData.replyingTo!; + const replyingTo = this.formData.replyingTo ?? 0; const files = this.formData.files || []; const isEditOnline = this.formData.id && this.formData.id > 0; const modal = await CoreLoadings.show('core.sending', true); @@ -413,73 +413,56 @@ export class AddonModForumPostComponent implements OnInit, OnDestroy, OnChanges // Add some HTML to the message if needed. message = CoreText.formatHtmlLines(message); - // Upload attachments first if any. - let attachments; - try { - if (files.length) { - try { - attachments = await AddonModForumHelper.uploadOrStoreReplyFiles( - this.forum.id, - isEditOnline ? this.formData.id! : replyingTo, - files, - false, - ); - } catch (error) { - // Cannot upload them in online, save them in offline. - if (!this.forum.id || isEditOnline || CoreUtils.isWebServiceError(error)) { - // Cannot store them in offline. Reject. - throw error; - } - - saveOffline = true; - attachments = await AddonModForumHelper.uploadOrStoreReplyFiles(this.forum.id, replyingTo, files, true); - } - } - let sent = false; - if (isEditOnline) { - sent = await AddonModForum.updatePost(this.formData.id!, subject, message, { + if (this.formData.id && this.formData.id > 0) { + const attachments = await this.uploadAttachmentsForEditOnline(this.formData.id); + + sent = await AddonModForum.updatePost(this.formData.id, subject, message, { attachmentsid: attachments, inlineattachmentsid: this.preparePostData?.draftitemid, }); - } else if (saveOffline) { - // Save post in offline. - await AddonModForumOffline.replyPost( - replyingTo, - this.discussionId, - this.forum.id, - this.forum.name, - this.courseId, - subject, - message, - { - attachmentsid: attachments, - private: !!this.formData.isprivatereply, - }, - ); - - // Set sent to false since it wasn't sent to server. - sent = false; } else { - // Try to send it to server. - // Don't allow offline if there are attachments since they were uploaded fine. - sent = await AddonModForum.replyPost( - replyingTo, - this.discussionId, - this.forum.id, - this.forum.name, - this.courseId, - subject, - message, - { - attachmentsid: attachments, - private: !!this.formData.isprivatereply, - }, - undefined, - !files.length, - ); + const { attachments, saveOffline } = await this.uploadAttachmentsForReply(replyingTo); + + if (saveOffline) { + // Save post in offline. + await AddonModForumOffline.replyPost( + replyingTo, + this.discussionId, + this.forum.id, + this.forum.name, + this.courseId, + subject, + message, + { + attachmentsid: attachments, + private: !!this.formData.isprivatereply, + }, + ); + + // Set sent to false since it wasn't sent to server. + sent = false; + } else { + // Try to send it to server. + // Don't allow offline if there are attachments since they were uploaded fine. + sent = await AddonModForum.replyPost( + replyingTo, + this.discussionId, + this.forum.id, + this.forum.name, + this.courseId, + subject, + message, + { + attachmentsid: attachments, + private: !!this.formData.isprivatereply, + }, + undefined, + !files.length, + ); + } } if (sent && this.forum.id) { @@ -506,6 +489,81 @@ export class AddonModForumPostComponent implements OnInit, OnDestroy, OnChanges } } + /** + * Upload attachments when editing an online post. + * + * @param postId Post ID being edited. + * @returns Draft area id (if any attachment has changed). + */ + protected async uploadAttachmentsForEditOnline(postId: number): Promise { + const files = this.formData.files || []; + const previousAttachments = (this.post.attachments ?? []) as CoreWSFile[]; + + if (!CoreFileUploader.areFileListDifferent(files, previousAttachments)) { + return; + } + + // Use prepare post for edition to avoid re-uploading all files. + let filesToKeep = files.filter((file): file is CoreWSFile => !CoreUtils.isFileEntry(file)); + let removedFiles: { filepath: string; filename: string }[] | undefined; + + if (previousAttachments.length && !filesToKeep.length) { + // Post had attachments but they were all removed. We cannot use the filesToKeep option because it doesn't allow + // removing all files. In this case we'll just keep 1 file and remove it later. + filesToKeep = [previousAttachments[0]]; + removedFiles = [{ + filename: previousAttachments[0].filename ?? '', + filepath: previousAttachments[0].filepath ?? '', + }]; + } + + const preparePostData = await AddonModForum.preparePostForEdition(postId, 'attachment', { filesToKeep }); + + if (removedFiles?.length) { + await CoreFileUploader.deleteDraftFiles(preparePostData.draftitemid, removedFiles); + } + + await CoreFileUploader.uploadFiles(preparePostData.draftitemid, files); + + return preparePostData.draftitemid; + } + + /** + * Upload attachments for a reply that isn't an online post being edited. + * + * @param replyingTo Replying to post ID. + * @returns Draft area id (if any attachment was uploaded) and whether data should be saved offline. + */ + async uploadAttachmentsForReply( + replyingTo: number, + ): Promise<{ attachments: CoreFileUploaderStoreFilesResult | number | undefined; saveOffline: boolean }> { + const files = this.formData.files || []; + if (!files.length) { + return { attachments: undefined, saveOffline: false }; + } + + try { + const attachments = await AddonModForumHelper.uploadOrStoreReplyFiles( + this.forum.id, + replyingTo, + files, + false, + ); + + return { attachments, saveOffline: false }; + } catch (error) { + // Cannot upload them in online, save them in offline. + if (!this.forum.id || CoreUtils.isWebServiceError(error)) { + // Cannot store them in offline. Reject. + throw error; + } + + const attachments = await AddonModForumHelper.uploadOrStoreReplyFiles(this.forum.id, replyingTo, files, true); + + return { attachments, saveOffline: true }; + } + } + /** * Cancel reply. */ From 7448cdc1b4002902bbc2bdf5130c423544ce4c68 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Tue, 20 Aug 2024 15:42:09 +0200 Subject: [PATCH 3/3] MOBILE-4482 glossary: Avoid reuploading files when editing entry --- src/addons/mod/glossary/pages/edit/edit.ts | 40 ++++++++++++----- src/addons/mod/glossary/services/glossary.ts | 46 ++++++++++++++++++++ 2 files changed, 75 insertions(+), 11 deletions(-) diff --git a/src/addons/mod/glossary/pages/edit/edit.ts b/src/addons/mod/glossary/pages/edit/edit.ts index 9507519614d..4c2aa09fee8 100644 --- a/src/addons/mod/glossary/pages/edit/edit.ts +++ b/src/addons/mod/glossary/pages/edit/edit.ts @@ -286,9 +286,9 @@ abstract class AddonModGlossaryFormHandler { * Upload attachments online. * * @param glossary Glossary. - * @returns Uploaded attachments item id. + * @returns Uploaded attachments item id, undefined if nothing to upload or change. */ - protected async uploadAttachments(glossary: AddonModGlossaryGlossary): Promise { + protected async uploadAttachments(glossary: AddonModGlossaryGlossary): Promise { const data = this.page.data; const itemId = await CoreFileUploader.uploadOrReuploadFiles( data.attachments, @@ -643,10 +643,7 @@ class AddonModGlossaryOnlineFormHandler extends AddonModGlossaryFormHandler { data.fullmatch = this.entry.fullmatch; } - // Treat offline attachments if any. - if (this.entry.attachments) { - data.attachments = this.entry.attachments; - } + data.attachments = (this.entry.attachments ?? []).slice(); this.page.originalData = { concept: data.concept, @@ -677,11 +674,7 @@ class AddonModGlossaryOnlineFormHandler extends AddonModGlossaryFormHandler { const definition = CoreText.formatHtmlLines(data.definition); // Upload attachments, if any. - let attachmentsId: number | undefined = undefined; - - if (data.attachments.length) { - attachmentsId = await this.uploadAttachments(glossary); - } + const attachmentsId = await this.uploadAttachments(); // Save entry data. await AddonModGlossary.updateEntry(glossary.id, this.entry.id, data.concept, definition, options, attachmentsId); @@ -694,6 +687,31 @@ class AddonModGlossaryOnlineFormHandler extends AddonModGlossaryFormHandler { return true; } + /** + * Upload attachments online. + * + * @returns Uploaded attachments item id, undefined if nothing to upload or change. + */ + protected async uploadAttachments(): Promise { + const data = this.page.data; + + if (!CoreFileUploader.areFileListDifferent(data.attachments, this.entry.attachments ?? [])) { + return; + } + + const { attachmentsid: attachmentsId } = await AddonModGlossary.prepareEntryForEdition(this.entry.id); + + const removedFiles = CoreFileUploader.getFilesToDelete(this.entry.attachments ?? [], data.attachments); + + if (removedFiles.length) { + await CoreFileUploader.deleteDraftFiles(attachmentsId, removedFiles); + } + + await CoreFileUploader.uploadFiles(attachmentsId, data.attachments); + + return attachmentsId; + } + } /** diff --git a/src/addons/mod/glossary/services/glossary.ts b/src/addons/mod/glossary/services/glossary.ts index a9c12cf1ad8..715ee95755a 100644 --- a/src/addons/mod/glossary/services/glossary.ts +++ b/src/addons/mod/glossary/services/glossary.ts @@ -626,6 +626,7 @@ export class AddonModGlossaryProvider { * * @param siteId Site id. * @returns Whether the site can update entries. + * @since 3.10 */ async canUpdateEntries(siteId?: string): Promise { const site = await CoreSites.getSite(siteId); @@ -1097,6 +1098,26 @@ export class AddonModGlossaryProvider { await site.getDb().insertRecord(ENTRIES_TABLE_NAME, entry); } + /** + * Prepare entry for edition. + * + * @param entryId Entry ID. + * @param siteId Site ID. + * @returns Data of prepared area. + */ + async prepareEntryForEdition( + entryId: number, + siteId?: string, + ): Promise { + const site = await CoreSites.getSite(siteId); + + const params: AddonModGlossaryPrepareEntryForEditionWSParams = { + entryid: entryId, + }; + + return await site.write('mod_glossary_prepare_entry_for_edition', params); + } + } export const AddonModGlossary = makeSingleton(AddonModGlossaryProvider); @@ -1435,6 +1456,31 @@ export type AddonModGlossaryViewEntryWSParams = { id: number; // Glossary entry ID. }; +/** + * Params of mod_glossary_prepare_entry_for_edition WS. + */ +type AddonModGlossaryPrepareEntryForEditionWSParams = { + entryid: number; // Glossary entry id to update. +}; + +/** + * Data returned by mod_glossary_prepare_entry_for_edition WS. + */ +export type AddonModGlossaryPrepareEntryForEditionWSResponse = { + inlineattachmentsid: number; // Draft item id for the text editor. + attachmentsid: number; // Draft item id for the file manager. + areas: { // File areas including options. + area: string; // File area name. + options: { // Draft file area options. + name: string; // Name of option. + value: string; // Value of option. + }[]; + }[]; + aliases: string[]; + categories: number[]; + warnings?: CoreWSExternalWarning[]; +}; + /** * Options to pass to add entry. */