Skip to content

Fix _send inconsistent return format#106

Merged
nicholasserra merged 1 commit intomasterfrom
nicholasserra-send-return-fix
Dec 6, 2021
Merged

Fix _send inconsistent return format#106
nicholasserra merged 1 commit intomasterfrom
nicholasserra-send-return-fix

Conversation

@nicholasserra
Copy link
Copy Markdown
Collaborator

Refs #105

cc @sjoerdjob-legalsense This solve the issues you're seeing?

@sjoerdjob-legalsense
Copy link
Copy Markdown

@nicholasserra : I didn't actually have a real-life case yet where we were running into it. The only reason I noticed it was because the change referenced were breaking our own subclass of the Django backend (where we returned the message object from _send, to achieve exactly the same effect: pass the message_id to user-code).

Reviewing the code, and the newly added tests, I do believe the issue to be fixed.

return False, None
else:
pm_messages = list(map(self._build_message, messages))
pm_messages = [m for m in pm_messages if m]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If I send three messages, but one of the messages doesn't have a recipient, then at this stage the pm_messages list will have only two elements.

How is user-code supposed to figure out which of the three messages didn't get send? That is, after this point, there's no difference between:

connection.send_messages([wrong_message, correct_message1, correct_message2])
connection.send_messages([correct_message1, wrong_message, correct_message2])
connection.send_messages([correct_message1, correct_message2, wrong_message])

Would be great if the return value could then be

[None, message_id1, message_id2]
[message_id1, None, message_id2]
[message_id1, message_id2, None]

in these cases?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good call. Will track via #108

@nicholasserra nicholasserra merged commit 5345ebb into master Dec 6, 2021
@nicholasserra nicholasserra deleted the nicholasserra-send-return-fix branch December 6, 2021 00:28
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.

2 participants