-
-
Notifications
You must be signed in to change notification settings - Fork 615
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
[15.0][MIG] mail_drop_target #881
Conversation
Hey @JasminSForgeFlow, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
03edf08
to
7462b29
Compare
/ocabot migration mail_drop_target |
7462b29
to
d439613
Compare
Why do you want to remove the dependency on web_drop_target? Code was easier if it was dependent IMO |
With eml it works fine, but it fails when using msg (taken it from the test files) |
I think there's a dependency missing ( I tried loading several different msg messages and could get some of them working (plain text email). but most often I get the following error:
Also got from a different messages:
and also
|
d439613
to
9c1160c
Compare
Rebased and fixed conflict. |
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
Hello @JasminSForgeFlow @LoisRForgeFlow , i just created this PR to import base64js library. Please review. |
@ygol i don't face those errors you had, please share your test files. |
9c1160c
to
d31eceb
Compare
@nguyenminhchien thanks! your commit has been included here. However, now a test from a different module is failing. I have tried to reproduce the error in my local with both mail_drop_target and mail_composer_cc_bcc installed, but all ran good in my case. Can you try to reproduce in your side? |
@LoisRForgeFlow Pull the latest Odoo codebase, the test will be failed. Here is from my local |
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
5207e38
to
ffc81f4
Compare
ffc81f4
to
fa3b61f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Works as expected
@JoanSForgeFlow @LoisRForgeFlow Can you review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The file detection mechanism has been improved, now adeptly managing PDFs and other file formats through the inclusion of the condition else if (drop_file.name.endsWith(".eml")).
- Introducing a reference (this._fileUploaderRef) for the file uploader enhances the structure of the code and facilitates more straightforward access to the uploader component.
It works.
This PR has the |
/ocabot merge nobump |
Sorry @JordiBForgeFlow you are not allowed to merge. To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons. If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functional test 👍
/ocabot merge nobump |
This PR looks fantastic, let's merge it! |
Congratulations, your PR was merged at 233f1b8. Thanks a lot for contributing to OCA. ❤️ |
Standard Migration
Adapt Odoo's out-of-box dropzone functionality, and remove the dependency of web_drop_target.
@ForgeFlow