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

Slack: notification fail in new tray publisher #4118

Merged

Conversation

kalisp
Copy link
Member

@kalisp kalisp commented Nov 22, 2022

Brief description

This handles issue with missing 'published_path' key caused by not publishing thumbnail.

Additional info

It also handles task better, it takes task info from instance first, handles legacy case when task data is only string and not dict as expected.

Testing notes:

  1. Enable Slack module
  2. Prepare project_settings/slack/publish/CollectSlackFamilies/profiles/0/channel_messages/0/channels/0
    slack_message
  3. Make sure that thumbnail won't get published in project_settings/global/publish/PreIntegrateThumbnails
  4. Publish

published_path might be missing in case of thumbnail not getting published. This implementation takes from staging if published_path not present
In legacy cases task might be only string with its name, not structure with additional metadata (type etc.). This implementation handles that.
It should take task from instance anatomyData, then from context and handle non dict items.
@kalisp kalisp added the type: bug Something isn't working label Nov 22, 2022
@kalisp kalisp self-assigned this Nov 22, 2022
@ynbot
Copy link
Contributor

ynbot commented Nov 22, 2022

@@ -112,7 +112,13 @@ def _get_filled_message(self, message_templ, instance, review_path=None):
if review_path:
fill_pairs.append(("review_filepath", review_path))

task_data = fill_data.get("task")
task_data = (
copy.deepcopy(instance.data.get("anatomyData", {})).get("task")
Copy link

Choose a reason for hiding this comment

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

continuation line over-indented for hanging indent

@64qam 64qam added this to the next-patch milestone Nov 22, 2022
@kalisp kalisp merged commit ef313d8 into develop Nov 23, 2022
@kalisp kalisp deleted the bugfix/OP-4196_Slack-notification-fail-in-New-Tray-Publisher branch November 23, 2022 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants