-
Notifications
You must be signed in to change notification settings - Fork 33
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: Notifications mailing system #327
base: master
Are you sure you want to change the base?
Changes from all commits
1ce98cd
1e9e28c
d3f23df
91b4a0e
4c475e1
c7d1fb6
b741e53
3274ef7
9e7dead
48d4811
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 |
---|---|---|
|
@@ -180,6 +180,7 @@ class User(UserMixin, BaseModel): | |
api_key = CharField() | ||
last_course_viewed = ForeignKeyField(Course, null=True) | ||
uuid = UUIDField(default=uuid4, unique=True) | ||
mail_subscription = BooleanField(default=True) | ||
|
||
def get_id(self): | ||
return str(self.uuid) | ||
|
@@ -383,6 +384,41 @@ def on_notification_saved( | |
instance.delete_instance() | ||
|
||
|
||
class MailMessage(BaseModel): | ||
user = ForeignKeyField(User, backref='mails') | ||
notification = ForeignKeyField(Notification, backref='mails') | ||
date = DateTimeField(default=datetime.now) | ||
|
||
orronai marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@classmethod | ||
def distincit_users(cls): | ||
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. why do you need distinct here? 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. And please add types 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. For each message there is an instance. I need distinct in order to get the distinct users, for sending only 1 message for each one of them. |
||
return cls.select(cls.user).distinct() | ||
|
||
@classmethod | ||
def by_user(cls, user: User) -> Iterable['MailMessage']: | ||
return ( | ||
cls | ||
.select() | ||
.where(cls.user == user) | ||
.join(Notification) | ||
.where(Notification.viewed == False) # NOQA: E712 | ||
) | ||
|
||
@classmethod | ||
def user_messages_number(cls, user: User) -> int: | ||
return ( | ||
cls | ||
.select(fn.Count(cls.id)) | ||
.where(cls.user == user) | ||
.join(Notification) | ||
.where(Notification.viewed == False) # NOQA: E712 | ||
.scalar() | ||
) | ||
|
||
@classmethod | ||
def get_instances_number(cls): | ||
return cls.select(fn.Count(cls.id)) | ||
|
||
|
||
class Exercise(BaseModel): | ||
subject = CharField() | ||
date = DateTimeField() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
import typing | ||
|
||
from flask import Flask | ||
from flask_apscheduler import APScheduler # type: ignore | ||
from flask_babel import Babel # type: ignore | ||
from flask_httpauth import HTTPBasicAuth | ||
from flask_limiter import Limiter # type: ignore | ||
|
@@ -48,6 +49,9 @@ | |
|
||
webmail = Mail(webapp) | ||
|
||
webscheduler = APScheduler(app=webapp) | ||
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. why this scheduler is part of lmsweb? is it running under the WSGI app? |
||
webscheduler.start() | ||
|
||
|
||
# Must import files after app's creation | ||
from lms.lmsdb import models # NOQA: F401, E402, I202 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -370,6 +370,25 @@ def read_all_notification(): | |
return jsonify({'success': success_state}) | ||
|
||
|
||
@webapp.route('/mail/<subscription>', methods=['PATCH']) | ||
def mail_subscription(subscription: str): | ||
success_state = users.change_mail_subscription(current_user, subscription) | ||
title = _('Mail Subscription') | ||
if subscription == 'subscribe': | ||
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. move |
||
body = _( | ||
"You've successfully subscribed to get mails " | ||
'for new notifications', | ||
) | ||
elif subscription == 'unsubscribe': | ||
body = _( | ||
"You've successfully unsubscribed to get mails " | ||
'for new notifications', | ||
) | ||
else: | ||
body = _('Something went wrong...') | ||
return jsonify({'success': success_state, 'title': title, 'body': body}) | ||
|
||
|
||
@webapp.route('/share', methods=['POST']) | ||
@login_required | ||
def share(): | ||
|
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 not
update
query instead of iterating all users in the system? will be much faster than thatThere 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.
And if its default true, are you sure you need this?
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 tried to migrate it on my environment and it created the column with null for all the objects