Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Slack: Add review to notification message #2498

Merged
merged 7 commits into from
Jan 13, 2022

Conversation

kalisp
Copy link
Member

@kalisp kalisp commented Jan 7, 2022

Brief description

Currently only thumbnail is present, it was required to add link to review.

Description

There are two options now how to add review to Slack notification message.

  • upload file - burnin version is preferred if present
  • use {review_link} placeholder in content of message

Additional info

Uploading of review must be though through as in case of large sizes of reviews publish might be too slow and one will be hitting on Slack limits pretty soon.

Contained in #2499, use #2499 to test both.

Testing notes:

  1. Use configuration like: Screenshot 2022-01-07 121535 (full bot key in Keeper under 'Slack bot token')
  2. Publish any render family
  3. Check test_integration channel

@kalisp kalisp added type: enhancement Enhancements to existing functionality sponsored Client endorsed or requested type: documentation module: Slack labels Jan 7, 2022
@kalisp kalisp requested review from mkolar and 64qam January 7, 2022 11:18
@kalisp kalisp self-assigned this Jan 7, 2022
@mkolar
Copy link
Member

mkolar commented Jan 7, 2022

Task linked: OP-2204 Feature - Slack: URL to reviewable

@iLLiCiTiT
Copy link
Member

Isn't that "review filepath" rather then link?

@@ -69,8 +81,22 @@ Few keys also have Capitalized and UPPERCASE format. Values will be modified acc
**Available keys:**
Copy link
Member

Choose a reason for hiding this comment

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

I would rather point to anatomy data and here add those keys that are not in anatomy data. It is hell to track all docs where these keys are when something in anatomy changes.

task_data = fill_data.get("task")
for key, value in task_data.items():
fill_key = "task[{}]".format(key)
fill_pairs.append((fill_key , value))
Copy link

Choose a reason for hiding this comment

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

whitespace before ','

@kalisp kalisp marked this pull request as draft January 7, 2022 16:45
@kalisp kalisp marked this pull request as ready for review January 12, 2022 16:04
@kalisp kalisp merged commit b92f48f into develop Jan 13, 2022
@kalisp kalisp deleted the feature/OP-2204_Feature---Slack-URL-to-reviewable branch January 13, 2022 15:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
sponsored Client endorsed or requested type: enhancement Enhancements to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants