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

MessageAttachment __str__ change #283

Merged
merged 20 commits into from
Dec 27, 2023
Merged

Conversation

Pietro395
Copy link
Collaborator

Small change to MessageAttachment str method.

Previously, this exception occurred when "Saving and continue edit" a Message by deleting an attachment


  File "/home/intacloud/env/lib/python3.8/site-packages/django/contrib/admin/utils.py", line 571, in construct_change_message
    "object": str(deleted_object),
  File "/home/intacloud/env/lib/python3.8/site-packages/django_mailbox/models.py", line 867, in __str__
    return self.document.url
  File "/home/intacloud/env/lib/python3.8/site-packages/django/db/models/fields/files.py", line 66, in url
    self._require_file()
  File "/home/intacloud/env/lib/python3.8/site-packages/django/db/models/fields/files.py", line 41, in _require_file
    raise ValueError(
ValueError: The 'document' attribute has no file associated with it

Copy link
Collaborator

@pfouque pfouque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Pietro395,
it's not clear how / why we can have an attachment without the message it relates to,
but the str should display more about itself than its parent IMO.
I added a suggestion

django_mailbox/models.py Outdated Show resolved Hide resolved
@Pietro395
Copy link
Collaborator Author

Hi @Pietro395,
it's not clear how / why we can have an attachment without the message it relates to,
but the str should display more about itself than its parent IMO.
I added a suggestion

Hi @pfouque , you are right.
Tomorrow I'll do a quick test of the change

@Pietro395
Copy link
Collaborator Author

LGTM @pfouque

Copy link
Collaborator

@pfouque pfouque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@Pietro395 Pietro395 merged commit 592126c into coddingtonbear:master Dec 27, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants