-
Notifications
You must be signed in to change notification settings - Fork 30k
[FW][FIX] mail: display arrows to switch to a valid attachment #200995
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
[FW][FIX] mail: display arrows to switch to a valid attachment #200995
Conversation
Steps to reproduce: - Create a new invoice - Attach a ZIP file then a PDF file - The file viewer shows but tries to preview the ZIP file - Happens with all non-viewable files Cause: The ZIP file is put as main attachment and the method `register_as_main_attachment` only change the main attachment (with `force=False`) when there is no main attachment. The behavior of the file viewer is inconsistent because we cannot switch between the attachments in the preview, so we cannot see the PDF. But if we add another PDF file (so one ZIP and 2 PDFs), we can switch but it will never show the ZIP again, only the two PDFs. This behavior is due to the [next/previous arrows being displayed](https://github.com/odoo/odoo/blob/e4da068d6c9c8885dd4663d50dee11c9ea1516a3/addons/mail/static/src/components/web_client_view_attachment_view/web_client_view_attachment_view.xml#L13) only if [`attachmentsInWebClientView`](https://github.com/odoo/odoo/blob/e4da068d6c9c8885dd4663d50dee11c9ea1516a3/addons/mail/static/src/models/attachment.js#L323-L328) contains more than one item. But this list only contains viewable attachments (PDF or Images), and the next/previous arrows only take attachments from this list. As the arrows are [changing the main attachment](https://github.com/odoo/odoo/blob/e4da068d6c9c8885dd4663d50dee11c9ea1516a3/addons/mail/static/src/models/web_client_view_attachment_view.js#L17) to change the displayed preview, it never comes back to the problematic ZIP file. Solution: Also display the arrows if the main attachment is not viewable and there is more than one attachment. opw-4486363 X-original-commit: 52661b6
@mathcoutant @alexkuhn this PR targets master and is the last of the forward-port chain containing:
To merge the full chain, use
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port |
@alexkuhn Do I r+ this PR on master? Or do you want to change the behavior to also display the preview of text, video, etc in the chatter? |
@mathcoutant I'd merge current fix in master, there are other priorities right now and this is good enough for the time being. |
@robodoo r+ |
Steps to reproduce: - Create a new invoice - Attach a ZIP file then a PDF file - The file viewer shows but tries to preview the ZIP file - Happens with all non-viewable files Cause: The ZIP file is put as main attachment and the method `register_as_main_attachment` only change the main attachment (with `force=False`) when there is no main attachment. The behavior of the file viewer is inconsistent because we cannot switch between the attachments in the preview, so we cannot see the PDF. But if we add another PDF file (so one ZIP and 2 PDFs), we can switch but it will never show the ZIP again, only the two PDFs. This behavior is due to the [next/previous arrows being displayed](https://github.com/odoo/odoo/blob/e4da068d6c9c8885dd4663d50dee11c9ea1516a3/addons/mail/static/src/components/web_client_view_attachment_view/web_client_view_attachment_view.xml#L13) only if [`attachmentsInWebClientView`](https://github.com/odoo/odoo/blob/e4da068d6c9c8885dd4663d50dee11c9ea1516a3/addons/mail/static/src/models/attachment.js#L323-L328) contains more than one item. But this list only contains viewable attachments (PDF or Images), and the next/previous arrows only take attachments from this list. As the arrows are [changing the main attachment](https://github.com/odoo/odoo/blob/e4da068d6c9c8885dd4663d50dee11c9ea1516a3/addons/mail/static/src/models/web_client_view_attachment_view.js#L17) to change the displayed preview, it never comes back to the problematic ZIP file. Solution: Also display the arrows if the main attachment is not viewable and there is more than one attachment. opw-4486363 closes #200995 X-original-commit: 52661b6 Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com> Signed-off-by: Mathieu Coutant (mcou) <mcou@odoo.com>
Steps to reproduce:
Cause:
The ZIP file is put as main attachment and the method
register_as_main_attachment
only change the main attachment (withforce=False
) when there is no main attachment.The behavior of the file viewer is problematic because we cannot switch between the attachments in the preview, so we cannot see the PDF. But if we add another PDF file (so one ZIP and 2 PDFs), we can switch but it will never show the ZIP again, only the two PDFs. This behavior is due to the next/previous arrows being displayed only if
attachmentsInWebClientView
contains more than one item. But this list only contains viewable attachments (PDF or Images), and the next/previous arrows only take attachments from this list. As the arrows are changing the main attachment to change the displayed preview, it never comes back to the problematic ZIP file.Solution:
Also display the arrows if the main attachment is not viewable and there is more than one attachment.
This way the user can return in the list of viewable attachments.
This is not an optimal because Odoo will still try to display the ZIP file, but it is a simple fix that works.
I tried restraining the main attachment field only to viewable files in this PR, but it seems to break things specially with OCR extraction and apparently it is sometimes wanted to have an XML as main attachment.
opw-4486363
Forward-Port-Of: #200817
Forward-Port-Of: #196446