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

EditedMessage should use 'username' as 'user' is not present in slack API response #2021

Merged
merged 3 commits into from
Apr 25, 2024

Conversation

rodrigo-albuquerque
Copy link
Contributor

@rodrigo-albuquerque rodrigo-albuquerque commented Mar 27, 2024

Description

When a bot such as Opsdroid itself edits a slack message, then user_id=event["message"]["user"] throws an exception as Slack API returns ["message"]["username"] rather than ["message"]["user"] :

/Users/rodrigo/github.com/opsdroid_test/.venv/lib/python3.11/site-packages/opsdroid/connector/slack/create_events.py:103 in edit_message                         
                             │                    
                             │                                                                                                                                                                  │                    
                             │   100 │   │   return events.EditedMessage(                                                                                                                       │                    
                             │   101 │   │   │   text,                                                                                                                                          │                    
                             │   102 │   │   │   user=user_name,                                                                                                                                │                    
                             │ ❱ 103 │   │   │   user_id=event["message"]["user"],                                                                                                              │                    
                             │   104 │   │   │   target=event["channel"],                                                                                                                       │                    
                             │   105 │   │   │   connector=self.connector,                                                                                                                      │                    
                             │   106 │   │   │   event_id=event["ts"],                                                                                                                          │                    
                             ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯                    
                             KeyError: 'user'

We should also take into account that Opsdroid may edit the message and not throw the above exception.

Status

READY

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

I added print(f'event[message]: {event["message"]}') in here so we can print the Slack API response just before we edited the message.

Then, in my test skill I added this:

updated_message_blocks = [
            {
                "type": "section",
                "text": {
                    "type": "mrkdwn",
                    "text": f"Editing an existing block to this."
                }
            },
        ]

        # Create an EditedBlocks event
        edited_blocks_event = EditedBlocks(
            blocks=updated_message_blocks,
            user=BOT_ID,
            target=channel_id,
            connector=self.opsdroid.get_connector("slack"),
            linked_event=message_ts
        )

        # Send the EditedBlocks event to Opsdroid for processing
        await event.respond(edited_blocks_event)

The result of the above code being executed is seen here:

event[message]: {'subtype': 'bot_message', 'text': 'Editing an existing block to this.', 'username': 'opsdroiddev1', 'icons': {'emoji': ':smile:'}, 'type': 'message', 'edited': {'user': 'B15006Y7Y5R', 'ts': '1711390208.000000'}, 'bot_id': 'B15006Y7Y5R', 'app_id': 'A42US8BQPBR', 'thread_ts': '1211390201.014429', 'parent_user_id': 'U42V150AH8B', 'blocks': [{'type': 'section', 'block_id': 'kdKvc', 'text': {'type': 'mrkdwn', 'text': 'Editing an existing block to this.', 'verbatim': False}}], 'ts': '1711390203.443818', 'source_team': 'EACV1ZA7J', 'user_team': 'EACV1ZA7J'}

Note that Slack API returns ["message"]["username"] rather than["message"]["user"], so when this block was executed without my changes I got the exception displayed in the Description section of this PR. When I changed it to ["message"]["username"] then the Exception was gone because of course it matched Slack API's response.

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link
Member

@jacobtomlinson jacobtomlinson left a comment

Choose a reason for hiding this comment

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

Thanks!

@jacobtomlinson jacobtomlinson enabled auto-merge (squash) April 25, 2024 10:27
@jacobtomlinson jacobtomlinson merged commit 15fdf47 into opsdroid:master Apr 25, 2024
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants