Skip to content
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

Fixed StorageSwitch doesn't export attachments correctly (removes html article) #525

Closed
wants to merge 2 commits into from

Conversation

tipue-dev
Copy link
Contributor

Proposed change

Problem: Using Admin::Article::StorageSwitch when Ticket::Article::Backend::MIMEBase::CheckAllStorageBackends is active changes all html articles to plaintext.

When switching between the backends FS and DB (Admin::Article::StorageSwitch), the code looks up, if a file with the same name exists already (because of some Unicode situation described here). To check this, the code gets all attachments of the article from the target backend.

The problem is, that the function used to list all attachments can look up the files in the other backend if the config Ticket::Article::Backend::MIMEBase::CheckAllStorageBackends is active. In that case, when no attachment has been written to the new backend yet, the file list of the old backend is returned instead. That means the first attachment, is wrongly assumed to already exists and gets renamed with a suffixing -1.

Since the first attachment is always file-1 (plain text) or file-2 (html), those will be renamed to file-x-1 on every switch and not recognised as special attachments anymore. This means the article suddenly is plain text with an attachment named file-x-1 added in the TicketZoom. Every switch adds more -1 at the end, that means if someone tries to fix this by switching between the backends, the filename becomes longer file-2-1-1-1-1-1.

The parameter needed to fix this already exists for different cases an can be used here too.

To reproduce the behaviour:

  1. activate Ticket::Article::Backend::MIMEBase::CheckAllStorageBackends
  2. export tickets to a new backend using Admin::Article::StorageSwitch
  3. open the TicketZoom

1 - 🐞 bug 🐞

Checklist

  • The code change is tested and works locally.(❗)
  • There is no commented out code in this PR.(❕)
  • You improved or added new unit tests.(❕)
  • Local ZnunyCodePolicy passed.(❕)
  • Local UnitTests / Selenium passed.(❕)
  • GitHub workflow CI (UnitTests / Selenium) passed.(❗)

@rkaldung
Copy link
Member

@tipue-dev I can't reproduce this with version Znuny LTS 6.5.9. Any ideas or is it just magically working again?

@rkaldung rkaldung added 3 - wait for contributor Contributor, it's your turn. 4 - clarification The issue or pull requests needs more information. labels Jun 27, 2024
@tipue-dev
Copy link
Contributor Author

@rkaldung i just tested it again in a fresh installation of 6.5.9.

  • I created a phone ticket. I added some html formatting to make it visible, that the plain text version is later displayed
  • i activated Ticket::Article::Backend::MIMEBase::CheckAllStorageBackends
  • i set Ticket::Article::Backend::MIMEBase::ArticleStorage to FS
  • i copied the articles into the filesystem with otrs.Console.pl Admin::Article::StorageSwitch --target ArticleStorageFS

The missing change of the ArticleStorage to FS might have been the problem in reproducing it.

Bildschirmaufnahme.2024-06-28.073240.mp4

@tipue-dev
Copy link
Contributor Author

I guess my PR is mood now with this commit a7578e7 and can be closed

@tipue-dev tipue-dev closed this Oct 2, 2024
@dennykorsukewitz
Copy link
Member

Hi @tipue-dev

Sorry, unfortunately, I didn't see your pull request. 🫣🫣🫣
I'll pay more attention to it next time.

Thanks 😋

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - wait for contributor Contributor, it's your turn. 4 - clarification The issue or pull requests needs more information.
Development

Successfully merging this pull request may close these issues.

3 participants