[FIX] web_editor: show toaster notification error on uploading folder #151755

visp-odoo · 2024-01-30T12:17:18Z

Steps to reproduce:

OS: Ubuntu 20.04.4 LTS with nautilus
Browser: Google Chrome Version 123.0.6312.58

type /file command in knowledge
select a folder and click on open
traceback occurs

Current behavior before PR:

When a user attempts to upload a folder using /file command, the processing begins, but the folder is not uploaded because the getDataURLFromFile return promise is not fulfilled. Additionally, there is no indication of any warnings or errors during the folder upload process.

Desired behavior after PR is merged:

If a user attempts to upload a folder instead of a file using the /file command, it results in an error message in the toaster notification.

task-3690847

robodoo · 2024-01-30T12:17:42Z

Pull request status dashboard.

julien-banken · 2024-01-31T10:08:15Z

addons/web_editor/controllers/main.py

+    @http.route('/web_editor/get_valid_file', type='json', auth='user')
+    def get_valid_file(self, filetype, extension):
+        if not filetype and not extension:
+            raise ValidationError(_("Please select a single file."))
+        return True


I don't think you need to add a new route just to check that the file type and file extension are defined. You can do that in pure Javascript 😃

Yes, you are right but we want to show validationError.So, i create new route. I think that we can not raise validation error and we can only use toaster notifications in javascript.

(see other comment)

addons/web_editor/static/src/components/upload_progress_toast/upload_service.js

julien-banken · 2024-03-19T15:01:48Z

addons/web_editor/static/src/components/upload_progress_toast/upload_service.js

+                        deleteFile(file.id);
+                        env.services.notification.add(
+                            _.str.sprintf(
+                                env._t('Could not load the file "%s". Please select a single file.'),


The upload service can be used to upload multiple files. Therefore, the error message does not always give correct instructions to the user.

I would therefore either remove the second part of the error message either provide a relevant hint to the user on how they can solve that issue. Other than that, it's looks great to me 😃

Sorry for the back and forth 🙏 It was not really clear to me what the issue was. I followed the task specification but that was not really what we had to do. For the next step, we will have to contact the editor team because we are updating their module.

Also: Can you update the PR description and the commit message to be inline with the changes we made? Thanks in advanced.

Done changes 👍

Thanks in Advance 😊

awa-odoo · 2024-03-26T14:53:36Z

Hereby delegating to @abd-msyukyu-odoo as it seems like it is an editor issue and already largely validated by Julien, so I don't have much added value here 😄

The only thing I will mention is to not forget to update the .POT files if you guys add labels and messages.

Cheers!

awa-odoo · 2024-03-26T14:53:52Z

@robodoo delegate=@abd-msyukyu-odoo

abd-msyukyu-odoo

@visp-odoo Hello, thanks for the PR, I added some comments :)

Could you add in the commit message and the PR description the OS you're using to reproduce the error, and the file browser you use ? (i.e. ubuntu version xxx with nautilus)

abd-msyukyu-odoo · 2024-03-27T09:17:43Z

addons/web_editor/static/src/components/upload_progress_toast/upload_service.js

+                    } catch {
+                        deleteFile(file.id);
+                        env.services.notification.add(
+                            _.str.sprintf(


Could you use the odoo sprintf from odoo/addons/web/static/src/core/utils/strings.js instead of _ library?

Done changes 👍

abd-msyukyu-odoo · 2024-03-27T09:18:05Z

addons/web_editor/static/src/components/upload_progress_toast/upload_service.js

+                        deleteFile(file.id);
+                        env.services.notification.add(
+                            _.str.sprintf(
+                                env._t('Could not load the file "%s".'),


Could you add this line to the .pot file since it will need new translations ?

Also, could you use _t from odoo/addons/web/static/src/core/l10n/translation.js instead of the one from the environment ? (will be easier on the forward ports)

@abd-msyukyu-odoo Done changes 👍

abd-msyukyu-odoo

@visp-odoo Hello! Thanks for the update, I'll confirm with @odoo/rd-framework-editor to see if they are ok with the change, but LGTM.

I added a small comment to sort the imports

abd-msyukyu-odoo · 2024-03-28T08:58:30Z

addons/web_editor/static/src/components/upload_progress_toast/upload_service.js

@@ -5,6 +5,8 @@ import { UploadProgressToast } from './upload_progress_toast';
 import { getDataURLFromFile } from 'web.utils';
 import { checkFileSize } from "@web/core/utils/files";
 import { humanNumber } from "@web/core/utils/numbers";
+import { _t } from "@web/core/l10n/translation";


You could move this line above "@web/core/utils/files"; to keep imports grouped in some sort of alphabetical order (based on the path) => web/core/utils are grouped together, after web/core/10n

(no need to adjust already existing imports)

Done changes 👍

Steps to reproduce: OS: Ubuntu 20.04.4 LTS with nautilus Browser: Google Chrome Version 123.0.6312.58 - type /file command in knowledge - select a folder and click on open - traceback occurs Before this commit: When a user attempts to upload a folder using /file command, the processing begins, but the folder is not actually uploaded because the `getDataURLFromFile` return promise which is not fulfiled. Additionally, there is no indication of any warnings or errors during the folder upload process. After this commit: If a user attempts to upload a folder instead of a file using the /file command, it results in a error message in toaster notification. task-3690847

abd-msyukyu-odoo · 2024-03-28T11:21:37Z

@robodoo r+

Steps to reproduce: OS: Ubuntu 20.04.4 LTS with nautilus Browser: Google Chrome Version 123.0.6312.58 - type /file command in knowledge - select a folder and click on open - traceback occurs Before this commit: When a user attempts to upload a folder using /file command, the processing begins, but the folder is not actually uploaded because the `getDataURLFromFile` return promise which is not fulfiled. Additionally, there is no indication of any warnings or errors during the folder upload process. After this commit: If a user attempts to upload a folder instead of a file using the /file command, it results in a error message in toaster notification. task-3690847 closes #151755 Signed-off-by: Damien Abeloos (abd) <abd@odoo.com>

fw-bot · 2024-04-01T05:54:12Z

@visp-odoo @abd-msyukyu-odoo this pull request has forward-port PRs awaiting action (not merged or closed):

[FW][FIX] web_editor: show toaster notification error on uploading folder #159905
[FW][FIX] web_editor: show toaster notification error on uploading folder #159937
[FW][FIX] web_editor: show toaster notification error on uploading folder #159966

fw-bot · 2024-04-02T05:55:07Z

@visp-odoo @abd-msyukyu-odoo this pull request has forward-port PRs awaiting action (not merged or closed):

[FW][FIX] web_editor: show toaster notification error on uploading folder #159905
[FW][FIX] web_editor: show toaster notification error on uploading folder #159937
[FW][FIX] web_editor: show toaster notification error on uploading folder #159966

visp-odoo force-pushed the 16.0-traceback-on-uploading-folder-visp branch from 11be7e4 to 04bf5e8 Compare January 30, 2024 12:23

C3POdoo added the RD research & development, internal work label Jan 30, 2024

visp-odoo force-pushed the 16.0-traceback-on-uploading-folder-visp branch from 04bf5e8 to 2862974 Compare January 30, 2024 13:00

julien-banken requested changes Jan 31, 2024

View reviewed changes

visp-odoo force-pushed the 16.0-traceback-on-uploading-folder-visp branch 3 times, most recently from cb28461 to f743ff4 Compare March 13, 2024 13:11

visp-odoo requested a review from julien-banken March 14, 2024 12:30

julien-banken reviewed Mar 14, 2024

View reviewed changes

addons/web_editor/static/src/components/upload_progress_toast/upload_service.js Outdated Show resolved Hide resolved

visp-odoo force-pushed the 16.0-traceback-on-uploading-folder-visp branch 4 times, most recently from 3054b83 to 3d591a0 Compare March 19, 2024 09:42

julien-banken reviewed Mar 19, 2024

View reviewed changes

visp-odoo force-pushed the 16.0-traceback-on-uploading-folder-visp branch from 3d591a0 to 7469789 Compare March 20, 2024 04:59

visp-odoo changed the title ~~[FIX] web_editor: show validation error on uploading folder~~ [FIX] web_editor: show toaster notification error on uploading folder Mar 20, 2024

visp-odoo force-pushed the 16.0-traceback-on-uploading-folder-visp branch from 7469789 to 153b493 Compare March 20, 2024 05:04

julien-banken approved these changes Mar 20, 2024

View reviewed changes

abd-msyukyu-odoo reviewed Mar 27, 2024

View reviewed changes

visp-odoo force-pushed the 16.0-traceback-on-uploading-folder-visp branch 2 times, most recently from 741d0b2 to ed090cd Compare March 28, 2024 06:51

abd-msyukyu-odoo reviewed Mar 28, 2024

View reviewed changes

abd-msyukyu-odoo marked this pull request as ready for review March 28, 2024 09:11

C3POdoo requested a review from a team March 28, 2024 09:13

visp-odoo force-pushed the 16.0-traceback-on-uploading-folder-visp branch from ed090cd to 9ee5613 Compare March 28, 2024 09:50

fw-bot mentioned this pull request Mar 28, 2024

[FW][FIX] web_editor: show toaster notification error on uploading folder #159708

Closed

robodoo temporarily deployed to merge March 28, 2024 14:32 Inactive

robodoo closed this Mar 28, 2024

fw-bot deleted the 16.0-traceback-on-uploading-folder-visp branch April 11, 2024 14:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FIX] web_editor: show toaster notification error on uploading folder #151755

[FIX] web_editor: show toaster notification error on uploading folder #151755

visp-odoo commented Jan 30, 2024 •

edited

robodoo commented Jan 30, 2024

julien-banken Jan 31, 2024

visp-odoo Jan 31, 2024 •

edited

julien-banken Feb 1, 2024

julien-banken Mar 19, 2024

julien-banken Mar 19, 2024

visp-odoo Mar 20, 2024

awa-odoo commented Mar 26, 2024

awa-odoo commented Mar 26, 2024

abd-msyukyu-odoo left a comment •

edited

abd-msyukyu-odoo Mar 27, 2024

visp-odoo Mar 28, 2024

abd-msyukyu-odoo Mar 27, 2024

visp-odoo Mar 28, 2024

abd-msyukyu-odoo left a comment

abd-msyukyu-odoo Mar 28, 2024

visp-odoo Mar 28, 2024

abd-msyukyu-odoo commented Mar 28, 2024

fw-bot commented Apr 1, 2024

fw-bot commented Apr 2, 2024

[FIX] web_editor: show toaster notification error on uploading folder #151755

[FIX] web_editor: show toaster notification error on uploading folder #151755

Conversation

visp-odoo commented Jan 30, 2024 • edited

robodoo commented Jan 30, 2024

Choose a reason for hiding this comment

visp-odoo Jan 31, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awa-odoo commented Mar 26, 2024

awa-odoo commented Mar 26, 2024

abd-msyukyu-odoo left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abd-msyukyu-odoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abd-msyukyu-odoo commented Mar 28, 2024

fw-bot commented Apr 1, 2024

fw-bot commented Apr 2, 2024

visp-odoo commented Jan 30, 2024 •

edited

visp-odoo Jan 31, 2024 •

edited

abd-msyukyu-odoo left a comment •

edited