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

Attached Files are not removed on Uninstall #370

Closed
kiwiradler opened this issue Jan 9, 2024 · 17 comments
Closed

Attached Files are not removed on Uninstall #370

kiwiradler opened this issue Jan 9, 2024 · 17 comments
Assignees

Comments

@kiwiradler
Copy link

Hi,
we have the problem that if you send a private message via the mail module, there are sometimes randomly attached some files from elsewhere in the network. The do not even belong to the people sending the messages. Using Humhub 1.15.2., all modules up do date.
Please investigate asap!

@yurabakhtin yurabakhtin self-assigned this Jan 9, 2024
@yurabakhtin
Copy link
Contributor

Hello @kiwiradler, could you please give more details? For example, maybe you can show some sample message text what was sent and what was received in notification.

I have reviewed the code and tested so files are attached to a message entry only if I upload it and on save the content in database has a string like this ![name.jpg](file-guid:6d650fac-5789-41ad-b1df-97928ca2c84d "name.jpg"), also a record must exists in the table file:

message-file

@kiwiradler
Copy link
Author

kiwiradler commented Jan 12, 2024

Hi,
I don't know if that helps. I marked the files that appeared randomly.
image-20240108-192457-280

image-20240108-190747-638

@yurabakhtin
Copy link
Contributor

@kiwiradler Thanks for the images, for me it looks like the messages were created by current User.
Sorry, but now I don't have any idea how they appered there if you didn't create them.

I can only guess if you have installed the module REST API https://marketplace.humhub.com/module/rest/manual and the messages were created by it.

@kiwiradler
Copy link
Author

kiwiradler commented Jan 23, 2024

It gets worse. See the attachment to see what happened. My colleague has just written a short message. All the files were attached randomly, without any of them being attached intentionally.
IMG_9635

@luke-
Copy link
Contributor

luke- commented Jan 23, 2024

@kiwiradler Can you extract the relevant table records (mail_message table, file table, etc.) from the database and post them here?

@kiwiradler
Copy link
Author

Unfortunately it does not stop - even after I have completely uninstalled and reinstalled the mail-module. I'll have a look at the tables now. The others have been deleted with the deinstallation of the module.
This is what happened today...
Bildschirmfoto 2024-02-19 um 10 52 33

@kiwiradler
Copy link
Author

@kiwiradler Can you extract the relevant table records (mail_message table, file table, etc.) from the database and post them here?

What exactly do you need? If I export the message table, everybody here can read all the messages from our network?! The messages are not encrypted...

@luke-
Copy link
Contributor

luke- commented Feb 19, 2024

You do not have to publish any sensitive information here.

For example, it would be interesting to see the corresponding "files" table entry for an affected file. You can of course remove the file name.

@kiwiradler
Copy link
Author

These two files have been attached randomly to a message I wrote. Spooky enough, the files are not shown in the chat, but when I chose "edit", I can see them. I've definitely no clue whats going on there...
file1
file2

@kiwiradler
Copy link
Author

Maybe it's important to say that I - once a year - delete the mail module and reinstall it to clear all the chats. The files, that have been written with the mail module, they are not deleted this way? Does it mean, that all the files are still "connected" with the database and are not permanently deleted?

@luke-
Copy link
Contributor

luke- commented Feb 20, 2024

Thanks for the screenshot.

These two files have been attached randomly to a message I wrote. Spooky enough, the files are not shown in the chat, but when I chose "edit", I can see them.

I don't understand, do you only see the files when editing a message? In your previous screenshots, the files were visible in the chat.

@luke-
Copy link
Contributor

luke- commented Feb 20, 2024

Maybe it's important to say that I - once a year - delete the mail module and reinstall it to clear all the chats. The files, that have been written with the mail module, they are not deleted this way? Does it mean, that all the files are still "connected" with the database and are not permanently deleted?

That could be the reason. That the files are not removed when the Mail module is deleted. If a new mail module message with e.g. ID 1 is created, old files become visible again.

You could perform an integrity check. This should recognize and remove old files:

https://docs.humhub.org/docs/admin/troubleshooting#data-integrity


@yurabakhtin Can you please check whether the problem is that assigned files are not correctly removed from a message in the Delete module?

@luke- luke- changed the title Major issue: Random files in Messages Attached Files are not removed on Uninstall Feb 20, 2024
@yurabakhtin
Copy link
Contributor

Can you please check whether the problem is that assigned files are not correctly removed from a message in the Delete module?

@luke- I see we deleted only tables of the Messenger module on deactivate/uninstall the module, i.e. these tables are deleted:

  • user_message_tag
  • message_tag
  • user_message
  • message_entry
  • message

the tables deletion are executed by uninstall migration file /migrations/uninstall.php.
So the records from core table file with object_model = \humhub\modules\mail\models\MessageEntry are not deleted, and after reinstall the module such old records are linked by ID with new created enteries, as you described above.

And yes you are right the integrity check tool solves the issues, I have tested it so all records from the core table file are deleted, but only there is no record \humhub\modules\mail\models\MessageEntry with the ID in database, i.e. the tool can fix only future message entries, it cannot remove files from the existing message entries where files were already linked by such mistake.

I.e. ideally it would be good to run the Integrity Check after reinstall new module.

Also we can add this small code File::deleteAll(['object_model' => MessageEntry::class]); into the protected/modules/mail/migrations/uninstall.php so after uninstall of the Messenger module all old linked files will be deleted. But I think better we should run File->delete(), so maybe we should run the code https://github.com/humhub/humhub/blob/master/protected/humhub/modules/file/Events.php#L69-L74 after disable a Module, do you agree this?

@luke-
Copy link
Contributor

luke- commented Feb 21, 2024

@yurabakhtin Can't we just implement the Module::uninstall() method and call "MessageEntry::delete()" on all of them? It should be executed inside a background job. This should delete all dependencies automatically?

@yurabakhtin
Copy link
Contributor

@luke- PR #381 - I have added to delete all MessageEntry records before disabling of the Messanger module, we have similar code for many other modules.
I don't think we should remove records form other message module tables because all tables are deleted on call the code parent::disable();, and only the MessageEntry may has a linked file.

I have tested on run the code $messageEntry->delete(); all records from the core table file with object_model = \humhub\modules\mail\models\MessageEntry are deleted and also the files are deleted from disk completely.

@luke- luke- closed this as completed Feb 22, 2024
@kiwiradler
Copy link
Author

That sounds good! Unfortunately I have no SSH with my webmaster. Is there another chance to run the integrity check?
Thanks for your help!

@luke-
Copy link
Contributor

luke- commented Feb 26, 2024

Unfortunately not, you could try to clean up the file table manually. However, the files in the uploads directory will then remain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants