Fix Slack file upload and unfurling issues (resolves #121672) #132451
Conversation
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
Hey there @tkdrob, @FletcherAU, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
home-assistant
bot
added
Quality Scale: No score
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
|
It might be worth adding a note to the docs regarding the preview image resizing? It's not necessarily intuitive unless you're familiar with Slack's preview requirements. |
|
(That's definitely a "has this been considered" more than a "you should do this" by the way) |
| # Step 3: Resize the image to meet the minimum dimension requirements | ||
| # https://api.slack.com/methods/files.remote.add#markdown | ||
| try: | ||
| data = await resp.read() # Read the bytes from the StreamReader | ||
| # Open the image and convert to RGB if necessary | ||
| image = Image.open(BytesIO(data)) | ||
| if image.mode in ("RGBA", "P"): # Convert if image has alpha channel | ||
| image = image.convert("RGB") | ||
|
|
||
| # Get current dimensions | ||
| width, height = image.size | ||
|
|
||
| # Determine the scaling factor to meet minimum dimensions | ||
| scale_w = 800 / width if width < 800 else 1 | ||
| scale_h = 400 / height if height < 400 else 1 | ||
| scale = max(scale_w, scale_h) # Scale up to meet both requirements | ||
|
|
||
| # Resize if necessary | ||
| if scale > 1: | ||
| new_width = int(width * scale) | ||
| new_height = int(height * scale) | ||
| image = image.resize((new_width, new_height), Image.LANCZOS) | ||
| _LOGGER.info("Image resized to %sx%s", new_width, new_height) | ||
| else: | ||
| _LOGGER.info( | ||
| "Image meets the minimum dimension requirements; no resizing needed" | ||
| ) |
There was a problem hiding this comment.
This looks like it'll fail when uploading files that aren't images.
There was a problem hiding this comment.
You are correct. Thanks for the catch. I updated the code, but I need to add some test coverage for that function and the change. Hoping to get the changes pushed up in the next day or so.
There was a problem hiding this comment.
Pushed up some changes to account for non-image files. Also added some more test coverage.
…le non-image files, and improve error logging
This is a good call out. I will review the docs and suggest possible updates |
|
@FletcherAU I opened a minor documentation PR (home-assistant/home-assistant.io#36694) as well so I think this PR is ready for another review. Thank you! 🙇 |
Proposed change
This PR addresses the file upload and unfurling issues in the Home Assistant Slack integration, as described in #121672. Specifically, it:
In April 2024, Slack announced adjustments to their file upload methods, making it clear that a newer, more stable approach to uploading and managing files is here to stay. This clarification deprecates older workflows and reinforces that developers should rely on updated, officially supported methods (like files.remote.* and files.upload.v2) for future compatibility and consistent behavior. By aligning the Home Assistant Slack integration with these updated methods, we ensure the integration remains robust and compatible with Slack’s evolving platform standards.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: