modoboa / modoboa-amavis

The amavis frontend of Modoboa
https://modoboa.org
MIT License
23 stars 16 forks source link

identify mail_id as a 16 character binary field #146

Closed bigwillystyle42 closed 2 years ago

bigwillystyle42 commented 2 years ago

The delete functionality in qcleanup.py does not work if the mail_id is not defined as a 16 character BinaryField. The proper messages are identified, but the delete fails. There might be a simpler solution there, but I'm not familiar with Django.

Making the field a BinaryField breaks the web UI because the field is expected to be a string. So we have to encode the string to get the field types to match. I chose ASCII arbitrarily, it might not be the right answer. There are likely other places the mail_id field needs to be properly encoded, but the changes I have allow things to work as expected in my usage.

This fixes https://github.com/modoboa/modoboa-amavis/issues/135 (and probably https://github.com/modoboa/modoboa-amavis/issues/136)

tonioo commented 2 years ago

@bigwillystyle42 Thank you for this contribution but could you please rebase your PR?

bigwillystyle42 commented 2 years ago

@tonioo yep, I've done so

tonioo commented 2 years ago

@bigwillystyle42 thank you

tonioo commented 2 years ago

@bigwillystyle42 Do you have any idea about what's causing the error we can see in tests logs?

bigwillystyle42 commented 2 years ago

@tonioo it's probably fallout from the char to binary change. I only did the bare minimum changes I needed to make my environment work.

Adding .encode("ascii") to the end of line 102 in modoboa_amavis/factories.py might fix the tests.

How can I run the tests locally? I'm not a python developer, so I'm not sure where to even start looking.

tonioo commented 2 years ago

@bigwillystyle42 I finally fixed all views and tests, see https://github.com/modoboa/modoboa-amavis/commit/dd02f390cfd7d439687a64e89ee6f427b9cdfd69. Thanks for your help !