-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
[feat] Notification Preferences Page #290
base: gsoc24-rebased
Are you sure you want to change the base?
Changes from 59 commits
6cc2835
7016f86
6d410fc
e6cbb9c
531acda
2dfc5a8
215118b
90221ca
64fe873
fdb8fdf
7c5cbeb
dfca656
0e5ebd2
44ca0dc
85bb0c9
b264324
b175fa1
c0d373f
487f6dd
a5d8b4e
cb8a296
4d5fca5
5c6bf56
8a8437f
8fb919c
7628892
946d743
1268fe6
c8cfdf0
f38dce3
cd40a29
b99f801
3ecbd92
33ec904
a0f08df
5a60f7d
852790b
3caaeda
ec83771
088abf2
5f8e008
1d4faae
aa813a7
40471ba
1e1978f
c912883
afb3be9
3fc3533
06bc604
72211c8
5f265be
7982603
890d764
fcb9e23
6dd8ca5
d1c5215
082c967
c0b7357
6106f9b
234627e
3d53083
9661ae6
0ecaa74
4ec65b7
b9d62b4
baa6111
c10952e
7efa79e
0e687b4
9156700
5413859
a796372
c3f01e5
ad8ff56
6c53bf6
2836bbd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,25 +1,3 @@ | ||
from django.contrib import admin | ||
|
||
from openwisp_notifications.base.admin import NotificationSettingAdminMixin | ||
from openwisp_notifications.swapper import load_model | ||
from openwisp_notifications.widgets import _add_object_notification_widget | ||
from openwisp_users.admin import UserAdmin | ||
from openwisp_utils.admin import AlwaysHasChangedMixin | ||
|
||
Notification = load_model('Notification') | ||
NotificationSetting = load_model('NotificationSetting') | ||
|
||
|
||
class NotificationSettingInline( | ||
NotificationSettingAdminMixin, AlwaysHasChangedMixin, admin.TabularInline | ||
): | ||
model = NotificationSetting | ||
extra = 0 | ||
|
||
def has_change_permission(self, request, obj=None): | ||
return request.user.is_superuser or request.user == obj | ||
|
||
|
||
UserAdmin.inlines = [NotificationSettingInline] + UserAdmin.inlines | ||
|
||
_add_object_notification_widget() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
from rest_framework.permissions import BasePermission | ||
|
||
|
||
class PreferencesPermission(BasePermission): | ||
""" | ||
Permission class for the notification preferences. | ||
|
||
Permission is granted only in these two cases: | ||
1. Superusers can change the notification preferences of any user. | ||
2. Regular users can only change their own preferences. | ||
""" | ||
|
||
def has_permission(self, request, view): | ||
return request.user.is_superuser or request.user.id == view.kwargs.get( | ||
'user_id' | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,11 +15,13 @@ | |
from rest_framework.permissions import IsAuthenticated | ||
from rest_framework.response import Response | ||
|
||
from openwisp_notifications.api.permissions import PreferencesPermission | ||
from openwisp_notifications.api.serializers import ( | ||
IgnoreObjectNotificationSerializer, | ||
NotificationListSerializer, | ||
NotificationSerializer, | ||
NotificationSettingSerializer, | ||
NotificationSettingUpdateSerializer, | ||
) | ||
from openwisp_notifications.swapper import load_model | ||
from openwisp_users.api.authentication import BearerAuthentication | ||
|
@@ -114,11 +116,17 @@ class BaseNotificationSettingView(GenericAPIView): | |
model = NotificationSetting | ||
serializer_class = NotificationSettingSerializer | ||
authentication_classes = [BearerAuthentication, SessionAuthentication] | ||
permission_classes = [IsAuthenticated] | ||
permission_classes = [PreferencesPermission] | ||
Dhanus3133 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
def get_queryset(self): | ||
if getattr(self, 'swagger_fake_view', False): | ||
return NotificationSetting.objects.none() # pragma: no cover | ||
|
||
user_id = self.kwargs.get('user_id') | ||
|
||
if user_id: | ||
return NotificationSetting.objects.filter(user_id=user_id) | ||
|
||
return NotificationSetting.objects.filter(user=self.request.user) | ||
Dhanus3133 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
|
@@ -198,11 +206,64 @@ def perform_create(self, serializer): | |
) | ||
|
||
|
||
class OrganizationNotificationSettingView(GenericAPIView): | ||
permission_classes = [IsAuthenticated, PreferencesPermission] | ||
serializer_class = NotificationSettingUpdateSerializer | ||
|
||
def post(self, request, user_id, organization_id): | ||
notification_settings = NotificationSetting.objects.filter( | ||
organization_id=organization_id, user_id=user_id | ||
) | ||
serializer = self.get_serializer(data=request.data) | ||
if serializer.is_valid(): | ||
for notification_setting in notification_settings: | ||
serializer.update(notification_setting, serializer.validated_data) | ||
Dhanus3133 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return Response(status=status.HTTP_200_OK) | ||
return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) | ||
|
||
|
||
class NotificationPreferenceView(GenericAPIView): | ||
permission_classes = [IsAuthenticated, PreferencesPermission] | ||
serializer_class = NotificationSettingUpdateSerializer | ||
|
||
def get(self, request, user_id): | ||
notification_settings, created = NotificationSetting.objects.get_or_create( | ||
user_id=user_id, | ||
organization=None, | ||
type=None, | ||
defaults={'email': True, 'web': True}, | ||
) | ||
serializer = self.get_serializer(notification_settings) | ||
return Response(serializer.data, status=status.HTTP_200_OK) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIRC, we decided that we would make a migration file that would create global notification setting. I don't think, we would need create notification settings on the fly here. Let me know if i missed on something. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a migration file in the PR. So, the current implement looks good. @nemesifier what do you think about this? |
||
|
||
def post(self, request, user_id): | ||
serializer = self.get_serializer(data=request.data) | ||
Dhanus3133 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if serializer.is_valid(): | ||
email = serializer.validated_data.get('email') | ||
web = serializer.validated_data.get('web') | ||
( | ||
notification_settings, | ||
created, | ||
) = NotificationSetting.objects.update_or_create( | ||
user_id=user_id, | ||
organization=None, | ||
type=None, | ||
defaults={'email': email, 'web': web}, | ||
) | ||
NotificationSetting.objects.filter(user_id=user_id).update( | ||
email=email, web=web | ||
) | ||
return Response(status=status.HTTP_200_OK) | ||
return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) | ||
|
||
|
||
notifications_list = NotificationListView.as_view() | ||
notification_detail = NotificationDetailView.as_view() | ||
notifications_read_all = NotificationReadAllView.as_view() | ||
notification_read_redirect = NotificationReadRedirect.as_view() | ||
notification_setting_list = NotificationSettingListView.as_view() | ||
notification_setting = NotificationSettingView.as_view() | ||
organization_notification_setting = OrganizationNotificationSettingView.as_view() | ||
ignore_object_notification_list = IgnoreObjectNotificationListView.as_view() | ||
ignore_object_notification = IgnoreObjectNotificationView.as_view() | ||
notification_preference = NotificationPreferenceView.as_view() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# Generated by Django 4.2.11 on 2024-06-18 13:05 | ||
|
||
import django.db.models.deletion | ||
from django.db import migrations, models | ||
|
||
|
||
class Migration(migrations.Migration): | ||
dependencies = [ | ||
("openwisp_users", "0020_populate_password_updated_field"), | ||
("openwisp_notifications", "0007_notificationsetting_deleted"), | ||
] | ||
|
||
operations = [ | ||
migrations.AlterField( | ||
model_name="notificationsetting", | ||
name="organization", | ||
field=models.ForeignKey( | ||
blank=True, | ||
null=True, | ||
on_delete=django.db.models.deletion.CASCADE, | ||
to="openwisp_users.organization", | ||
), | ||
), | ||
] |
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.
Why did you exclude the global notifications from here? This means that these settings will not appear when the user browse the
NotificationSettingListView
API endpoint from the browser. Thus, it makes impossible to turn off global notifications using the REST API.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.
We already have a seperate endpoint to update the global notification settings here
openwisp-notifications/openwisp_notifications/api/urls.py
Lines 53 to 57 in 9661ae6
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.
Thinking out loud: What are the benefits of having a separate endpoint for only managing global notifications?