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

notifications slack integration broken image fix #3604

Merged
merged 2 commits into from
May 8, 2020

Conversation

susanev
Copy link
Contributor

@susanev susanev commented May 6, 2020

🔩 Description: What code changed, and why?

there was a broken image in the notifications service, we are gonna use this one instead https://docs.chef.io/images/chef-icon.png h/t @mjingle

⛓️ Related Resources

fixes #3602

👍 Definition of Done

no more broken image url

👟 How to Build and Test the Change

i dont super know how to test this, hopefully @satellite15 does

✅ Checklist

📷 Screenshots, if applicable

zero.

@susanev susanev added bug 🐛 Something isn't working notifications This issue or pull request pertains to notifications in Automate labels May 6, 2020
@susanev susanev requested a review from satellite15 May 6, 2020 12:44
@susanev susanev requested a review from a team as a code owner May 6, 2020 12:44
@susanev
Copy link
Contributor Author

susanev commented May 6, 2020

no clue why im breaking doc shortcodes??

@mjingle mjingle mentioned this pull request May 6, 2020
4 tasks
Signed-off-by: susanev <[email protected]>
@schisamo
Copy link
Contributor

schisamo commented May 7, 2020

Is there a way we can bake this image into the service? I don't want to hold up fixing this broken image but maybe we should have a follow-on PR which doesn't pull the image from an external location.

@susanev
Copy link
Contributor Author

susanev commented May 7, 2020

@schisamo i was under the assumption that it needed to be external because its used in the slack integration... @satellite15 can it be internal to the service?

@satellite15
Copy link

satellite15 commented May 8, 2020

I can't reproducing this. I see the icon on my slack channel before and after the fix.
In both cases it only appears on the first alert and not subsequent alerts.

I see the icon in the slack alerts before and after the fix.

And the URL is in s3.
before https://s3-us-west-2.amazonaws.com/slack-files2/bot_icons/2017-02-17/143310324690_48.png

after https://s3-us-west-2.amazonaws.com/slack-files2/bot_icons/2020-05-08/1136159172752_48.png

I'm guessing slack stores the images and doesn't link to the original and that's why it appears to be working before and after the fix.

I wold say push the new fix. And yes @schisamo keep it external to the service. It needs to be public and accessible by slack so should be a static resource outside of automate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working notifications This issue or pull request pertains to notifications in Automate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image URL included in Slack integration returns 404
4 participants