-
-
Notifications
You must be signed in to change notification settings - Fork 723
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
fix: include tags in variants event #4711
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this solves the problem, but... where should we add this query for tags?
We're adding a side effect on the creation of the event to add something that's unrelated with the event itself. Should we do this query instead in the SlackApp addon when we receive an event with a featureName
and tags === undefined
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch.
Good point, however this seems like our approach for other events as well, so at least it's consistent. There are many examples of this exact flow in |
Ok, standardization is a good reasoning to continue with this idea, I'm just thinking that by following my proposal, we can remove all this duplicated code, and we may even catch other events that are in the same situation. After all, not all listeners of the event need to know about the tags, we're adding this information just because of the way we built Slack Addon, therefore we're coupling our code to one particular solution. But I agree, we can consider it out of the scope, especially if this involves changing other unrelated parts of the codebase. |
https://linear.app/unleash/issue/2-1396/fix-variants-event-should-include-the-respective-feature-flag-tags This fixes an issue where the `feature-environment-variants-updated` event would not post to a tagged Slack channel, since it would not take into consideration the feature flag tags.
https://linear.app/unleash/issue/2-1396/fix-variants-event-should-include-the-respective-feature-flag-tags
This fixes an issue where the
feature-environment-variants-updated
event would not post to a tagged Slack channel, since it would not take into consideration the feature flag tags.