-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
fix(notifications): hide provider option for weekly reports and fix org specific settings #57992
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
Conversation
| > | ||
| <BottomJsonForm fields={this.getProviderFields()} /> | ||
| </Form> | ||
| ) : null} |
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.
Does this handle saving the data correctly? As in, will this set NotificationSettingProvider for email?
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.
@snigdhas Yes, I verified. It all goes to the control silo so it's fine. I think the problem is that to get the projects you have to pick the region silo but not an issue for the organization which has replication in the control silo.
| handleOrgChange={handleOrgChange} | ||
| /> | ||
| ) : ( | ||
| t('Settings for Organizations') |
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.
for all organizations?
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.
@scttcper yea, should be all
We only support the email provider for weekly reports. Also, we can hide the dropdown for organization for deploy notifications.
Before:
After:
