-
Notifications
You must be signed in to change notification settings - Fork 311
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
Organization fanout_notifications
option
#6773
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nicholaspcr
added
c/identity server
This is related to the Identity Server
compat/db
This could affect Database compatibility
labels
Dec 12, 2023
adriansmares
approved these changes
Jan 8, 2024
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.
LGTM. Consider adding this as a checkbox in the organization general settings, in the Console - it may not be as hard as it sounds.
nicholaspcr
force-pushed
the
issue/6751-notification-fanout-opt
branch
from
January 9, 2024 17:39
d2827a1
to
5dd6e22
Compare
nicholaspcr
requested review from
PavelJankoski
and removed request for
kschiffer
January 9, 2024 17:41
kschiffer
approved these changes
Jan 11, 2024
nicholaspcr
force-pushed
the
issue/6751-notification-fanout-opt
branch
from
January 16, 2024 19:53
5dd6e22
to
4b10010
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
c/identity server
This is related to the Identity Server
compat/db
This could affect Database compatibility
ui/web
This is related to a web interface
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Closes #6751
This the OS counterpart of: https://github.com/TheThingsIndustries/lorawan-stack/pull/4006
Changes
fanout_notifications
to organizations table.true
, preserves old behavior.FanoutNotifications
field to organization API definition.FantoutNotifications
field from theorganization
entity.organization
store.notification_registry
, revolves around the scenario where anorganization
is an(admin/tech)_contact
.Testing
Unit tests
Test steps
2.1. Add both as members.
2.2. Add one of the users as the administrative contact
4.1. There should be only an email for the administrative contact of the organization.
fanout_notifications
to true. Usettn-lw-cli org set org-1 --fanout-notifications=true
6.1. There should be one email for each member of the organization.
Observation:
When testing it locally it is usefull to have
dir
as the email provider. Can be done by setting the Stack config to:Regressions
Notes for Reviewers
Checklist
README.md
for the chosen target branch.CHANGELOG.md
.CONTRIBUTING.md
, there are no fixup commits left.