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

[feature] Added general setting to disable all notifications #148 #158

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

niteshsinha17
Copy link
Member

Created a new model named General Setting through which we can disable email OR/AND web notifications for all the organization whether it is created after or before disabling.

Also, I have written tests for it.
I think we may need to write code for API too and also need to mention this feature in the documentation.
Before doing this it will be great if you give consent to this solution.

closes #148

@pandafy
Copy link
Member

pandafy commented Dec 26, 2020

@niteshsinha17 the changes that you have made in this PR were not expected.

Read this discussion to get a better idea of what is expected from the issue: #69 (review)

@niteshsinha17
Copy link
Member Author

@niteshsinha17 the changes that you have made in this PR were not expected.

Read this discussion to get a better idea of what is expected from the issue: #69 (review)

Okay, I will check and will make changes according to it.

@niteshsinha17
Copy link
Member Author

niteshsinha17 commented Dec 30, 2020

@pandafy from this #69 (review) I have concluded a few things that I wanted to share with you.

  • I need to create that All setting which was removed in this PR but this time the need is to disable all notifications?
  • If yes then we will again come into the confusing situation which was mentioned in the above PR.

if notifications are enabled in ALL but not enabled in the org, which setting overrides which?

If the above points are true then I think this could be a solution.

  • That 'All' setting should not generate automatically. It will be created by the user by creating a new notification setting to disable all notifications.
  • If a user disables notifications for all organizations for a particular type then we can check the record before sending a notification.
  • If the user enables notifications both web and email for all organizations then we will delete that setting.

What do you think about it?

@pandafy
Copy link
Member

pandafy commented Jan 1, 2021

@nemesisdesign will be the right person to answer this query.

@niteshsinha17 niteshsinha17 self-assigned this Jan 3, 2021
@nemesifier nemesifier added this to Ready for review/testing in OpenWISP Priorities for next releases Jan 13, 2021
@niteshsinha17 niteshsinha17 moved this from Ready for review/testing to In progress in OpenWISP Priorities for next releases Jan 21, 2021
@nemesifier nemesifier moved this from In progress to Backlog in OpenWISP Priorities for next releases Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants