Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ CREATE TABLE notification (
created TIMESTAMP WITH TIME ZONE DEFAULT NOW(),
expire_age SMALLINT NOT NULL, -- in days.
important BOOLEAN DEFAULT FALSE,
email_id TEXT UNIQUE,
email_id TEXT UNIQUE NOT NULL DEFAULT uuid_generate_v4()::text,
subject TEXT,
body TEXT,
template_id TEXT, --MB Mail template id.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ CREATE TABLE user_preference (
id INTEGER GENERATED BY DEFAULT AS IDENTITY, --pkey
musicbrainz_row_id INTEGER UNIQUE, --TODO: foreign key to user's table musicbrainz_row_id.
user_email TEXT UNIQUE,
notifications_enabled BOOLEAN DEFAULT TRUE,
digest BOOLEAN DEFAULT FALSE,
digest_age SMALLINT -- in days.
);
Expand Down
3 changes: 2 additions & 1 deletion admin/sql/create_tables.sql
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ CREATE TABLE notification (
created TIMESTAMP WITH TIME ZONE DEFAULT NOW(),
expire_age SMALLINT NOT NULL, -- in days.
important BOOLEAN DEFAULT FALSE,
email_id TEXT UNIQUE,
email_id TEXT UNIQUE NOT NULL DEFAULT uuid_generate_v4()::text,
subject TEXT,
body TEXT,
template_id TEXT, --MB Mail template id.
Expand All @@ -123,6 +123,7 @@ CREATE TABLE user_preference (
id INTEGER GENERATED BY DEFAULT AS IDENTITY, -- pkey
musicbrainz_row_id INTEGER UNIQUE, --TODO: foreign key to user's table musicbrainz_row_id.
user_email TEXT UNIQUE,
notifications_enabled BOOLEAN DEFAULT TRUE,
digest BOOLEAN DEFAULT FALSE,
digest_age SMALLINT -- in days.
);
Expand Down
1 change: 1 addition & 0 deletions config.py.example
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ SQLALCHEMY_MUSICBRAINZ_URI = ""
SQLALCHEMY_TRACK_MODIFICATIONS = False

POSTGRES_ADMIN_URI = "postgresql://postgres:postgres@meb_db/postgres"
POSTGRES_ADMIN_MEB_URI = "postgresql://postgres:postgres@meb_db:5432/metabrainz"

# DATABASES

Expand Down
1 change: 1 addition & 0 deletions manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def init_db(force=False, create_db=False):
db.run_sql_script_without_transaction(os.path.join(ADMIN_SQL_DIR, 'create_db.sql'))
click.echo('Done.')

db.init_db_engine(application.config["POSTGRES_ADMIN_MEB_URI"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks unneeded to me, the POSTGRES_ADMIN_URI config var does the same. Can you share the error you were facing?

Copy link
Author

@fettuccinae fettuccinae Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://gist.github.com/fettuccinae/787cf9b7368cf9aa2efc977e000575d6
These were the logs I was getting when POSTGRES_ADMIN_URI was used to create extensions( I think it was creating extensions on the POSTGRES database instead on the METABRAINZ database)

click.echo('Creating database extensions... ', nl=False)
db.run_sql_script_without_transaction(os.path.join(ADMIN_SQL_DIR, 'create_extensions.sql'))
click.echo('Done.')
Expand Down
3 changes: 1 addition & 2 deletions metabrainz/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import pprint
import sys

import stripe
from brainzutils.flask import CustomFlask
from brainzutils import sentry
from flask import send_from_directory, request
Expand Down Expand Up @@ -61,7 +60,7 @@ def create_app(debug=None, config_path=None):
sleep(1)

if not os.path.exists(consul_config):
print("No configuration file generated yet. Retried %d times, exiting." % CONSUL_CONFIG_FILE_RETRY_COUNT);
print("No configuration file generated yet. Retried %d times, exiting." % CONSUL_CONFIG_FILE_RETRY_COUNT)
sys.exit(-1)

app.config.from_pyfile(consul_config, silent=True)
Expand Down
9 changes: 6 additions & 3 deletions metabrainz/db/notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,11 @@ def _prepare_notifications(notifications: List[dict]) -> List[dict]:


def filter_non_digest_notifications(notifications: List[dict]) -> List[dict]:
"""Filter notifications which belongs to users with digest disabled.
"""Filter notifications which belongs to users with notifications enabled and digest disabled.
Args:
notifications (List[dict]): List of notifications.
Returns:
List[dict] : List of notifications for users with digest disabled.
List[dict] : List of notifications for users with notifications enabled and digest disabled.

"""
user_ids_to_check = tuple(i["user_id"] for i in notifications)
Expand All @@ -202,7 +202,10 @@ def filter_non_digest_notifications(notifications: List[dict]) -> List[dict]:
query = sqlalchemy.text("""
SELECT musicbrainz_row_id
FROM user_preference
WHERE musicbrainz_row_id IN :user_ids AND digest = FALSE
WHERE
musicbrainz_row_id IN :user_ids
AND digest = FALSE
AND notifications_enabled = TRUE
""")
result = connection.execute(query, {"user_ids": user_ids_to_check})
non_digest_user_ids = {user_id[0] for user_id in result}
Expand Down
8 changes: 5 additions & 3 deletions metabrainz/db/tests/test_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,13 @@ def test_get_digest_notifications(self):
# Prepare user_preference table.
digest_data = [
# User's digest_age is more than 3, so no digest notification.
{"musicbrainz_row_id": 1, "digest": True, "digest_age": 7, "user_email":"[email protected]"},
{"musicbrainz_row_id": 1, "notifications_enabled": True, "digest": True, "digest_age": 7, "user_email":"[email protected]"},
# User who should get their digest notification.
{"musicbrainz_row_id": 2, "digest": True, "digest_age": 3, "user_email":"[email protected]"},
{"musicbrainz_row_id": 2, "notifications_enabled": True, "digest": True, "digest_age": 3, "user_email":"[email protected]"},
# Digest is false, so no digest notification.
{"musicbrainz_row_id": 3, "digest": False, "digest_age": None, "user_email":"[email protected]"},
{"musicbrainz_row_id": 3, "notifications_enabled": True, "digest": False, "digest_age": None, "user_email":"[email protected]"},
# Notifications are not enabled, so no digest notification.
{"musicbrainz_row_id": 4, "notifications_enabled": False, "digest": True, "digest_age": 6, "user_email": "[email protected]"},
]
query = sqlalchemy.text(
"""INSERT INTO user_preference(musicbrainz_row_id, digest, digest_age, user_email) VALUES(:musicbrainz_row_id, :digest, :digest_age, :user_email)"""
Expand Down
2 changes: 1 addition & 1 deletion metabrainz/mail.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def _send_notifications(self, notifications: List[dict]):
)

def send_immediate_notifications(self):
"""Sends notifications that are marked as important or are for users with digest disabled."""
"""Sends notifications that are marked as important or are for users with notifications enabled and digest disabled."""

immediate_notifications = []
unimportant_notifications = []
Expand Down
3 changes: 2 additions & 1 deletion metabrainz/model/user_preference.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class UserPreference(db.Model):
id = db.Column(db.Integer, primary_key=True)
musicbrainz_row_id = db.Column(db.Integer, unique=True)
user_email = db.Column(db.Text, unique=True)
notifications_enabled = db.Column(db.Boolean, default=True)
digest = db.Column(db.Boolean, default=False)
digest_age = db.Column(db.SmallInteger, default=DEFAULT_DIGEST_AGE)

Expand All @@ -21,7 +22,7 @@ def get(cls: Type["UserPreference"], **kwargs) -> Optional["UserPreference"]:
return cls.query.filter_by(**kwargs).first()

@classmethod
def set_digest_info(cls: Type["UserPreference"], musicbrainz_row_id: int, digest: bool, digest_age: Optional[int]=None) -> Optional["UserPreference"]:
def set_notification_preference(cls: Type["UserPreference"], musicbrainz_row_id: int, digest: bool, digest_age: Optional[int]=None) -> Optional["UserPreference"]:
params = {cls.digest: digest}
if digest_age:
params[cls.digest_age] = digest_age
Expand Down
42 changes: 33 additions & 9 deletions metabrainz/notifications/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,18 +332,19 @@ def send_notifications():
return jsonify({'status':'ok'}), 200


@notification_bp.route("/<int:user_id>/digest-preference", methods=["GET", "POST"])
@notification_bp.route("/<int:user_id>/notification-preference", methods=["GET", "POST"])
@ccg_token_required
def set_digest_preference(user_id):
def notification_preference(user_id):
"""
Get and update the digest preference of the user.
Get and update the notification preference of the user.
An access token must be provided in the Authorization header, formatted as `Bearer <token>`.
**To get the digest preference of the user, a GET request must be made to this endpoint.**

**To get the notification preference of the user, a GET request must be made to this endpoint.**
Returns JSON of the following format:

..code-block:: json
{
"notifications_enabled": true,
"digest": false,
"digest_age": 7
}
Expand All @@ -353,16 +354,18 @@ def set_digest_preference(user_id):
:statuscode 400: Invalid user_id.
:resheader Content-Type: *application/json*

**To update the digest preference of the user, a POST request must be made to this endpoint.**
**To update the notification preference of the user, a POST request must be made to this endpoint.**

Request JSON must contain:
- ``notifications_enabled``: (bool) Required
- ``digest``: (bool) Required
- ``digest_age``: (int) Optional, Defaults to 7

Example Request:

..code-block:: json
{
"notifications_enabled": true,
"digest": true,
"digest_age": 17
}
Expand All @@ -371,6 +374,7 @@ def set_digest_preference(user_id):

..code-block:: json
{
"notifications_enabled": true,
"digest": true,
"digest_age": 17
}
Expand All @@ -387,22 +391,42 @@ def set_digest_preference(user_id):
res = UserPreference.get(musicbrainz_row_id=user_id)
if not res:
raise APIBadRequest("Invalid user_id.")
return jsonify({"digest": res.digest, "digest_age": res.digest_age})
return jsonify(
{
"notifications_enabled": res.notifications_enabled,
"digest": res.digest,
"digest_age": res.digest_age,
}
)

elif request.method == "POST":
data = request.json
notifications_enabled = data.get("notifications_enabled")
digest = data.get("digest")
digest_age = data.get("digest_age")

if notifications_enabled is None or not isinstance(notifications_enabled, bool):
raise APIBadRequest("Invalid notifications_enabled value.")
if digest is None or not isinstance(digest, bool):
raise APIBadRequest("Invalid digest value.")
if digest_age is not None:
if not isinstance(digest_age, int) or (digest_age < 1 or digest_age > MAX_DIGEST_AGE):
raise APIBadRequest("Invalid digest age.")

try:
result = UserPreference.set_digest_info(musicbrainz_row_id=user_id, digest=digest, digest_age=digest_age)
return jsonify({"digest": result.digest, "digest_age": result.digest_age})
result = UserPreference.set_notification_preference(
musicbrainz_row_id=user_id,
notifications_enabled=notifications_enabled,
digest=digest,
digest_age=digest_age,
)
return jsonify(
{
"notifications_enabled": result.notifications_enabled,
"digest": result.digest,
"digest_age": result.digest_age,
}
)

except Exception as err:
current_app.logger.error("Cannot update digest preference %s", str(err))
Expand Down
51 changes: 37 additions & 14 deletions metabrainz/notifications/views_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,21 +246,25 @@ def test_send_notifications(self, mock_requests, mock_insert):

@requests_mock.Mocker()
@mock.patch('metabrainz.notifications.views.UserPreference')
def test_set_digest_preference(self, mock_requests, mock_digest):
mock_digest.get.return_value = mock.MagicMock(digest=True, digest_age=19)
def test_notification_preference(self, mock_requests, mock_digest):
mock_digest.get.return_value = mock.MagicMock(
notifications_enabled=True, digest=True, digest_age=19
)
mock_requests.post(self.introspect_url, json={
"active": True,
"client_id": "abc",
"scope": ["notification"],
})
user_id = 1
url = f"notification/{user_id}/digest-preference"
url = f"notification/{user_id}/notification-preference"
headers = {"Authorization": "Bearer good_token"}

# GET method test
res = self.client.get(url, headers=headers)
self.assert200(res)
self.assertEqual(res.json, {"digest": True, "digest_age": 19})
self.assertEqual(
res.json, {"notifications_enabled": True, "digest": True, "digest_age": 19}
)
mock_digest.get.assert_called_with(musicbrainz_row_id=user_id)

mock_digest.get.return_value = None
Expand All @@ -269,24 +273,43 @@ def test_set_digest_preference(self, mock_requests, mock_digest):
self.assertEqual(res.json['error'], 'Invalid user_id.')

# POST method test
mock_digest.set_digest_info.return_value = mock.MagicMock(digest=True, digest_age=21)
params = {"digest": True, "digest_age": 21}
mock_digest.set_notification_preference.return_value = mock.MagicMock(
notifications_enabled=True, digest=True, digest_age=21
)
params = {"notifications_enabled": True, "digest": True, "digest_age": 21}
res = self.client.post(url, headers=headers, json=params)
self.assert200(res)
self.assertEqual(res.json, params)

# Bad requests.
bad_params = [({"digest":"true"}, "Invalid digest value."),
({"digest": True, "digest_age": "200"}, "Invalid digest age."),
({"digest": True, "digest_age": 200}, "Invalid digest age."),
({"digest": True, "digest_age": 0}, "Invalid digest age.")
]
bad_params = [
(
{"notifications_enabled": "true", "digest": "true", "digest_age": 200},
"Invalid notifications_enabled value.",
),
(
{"notifications_enabled": True, "digest": "true", "digest_age": 200},
"Invalid digest value.",
),
(
{"notifications_enabled": True, "digest": True, "digest_age": "200"},
"Invalid digest age.",
),
(
{"notifications_enabled": True, "digest": True, "digest_age": 200},
"Invalid digest age.",
),
(
{"notifications_enabled": True, "digest": True, "digest_age": 0},
"Invalid digest age.",
),
]
for b in bad_params:
res = self.client.post(url, headers=headers, json=b[0])
self.assert400(res)
self.assertEqual(res.json['error'], b[1])

mock_digest.set_digest_info.side_effect = Exception()
mock_digest.set_notification_preference.side_effect = Exception()
res = self.client.post(url, headers=headers, json=params)
self.assert503(res)
self.assertEqual(res.json['error'], "Cannot update digest preference right now.")
Expand All @@ -307,9 +330,9 @@ def test_invalid_tokens(self, mock_requests):
"data": [{"test_data": 1}],
},
{
"url": "notification/1/digest-preference",
"url": "notification/1/notification-preference",
"method": self.client.post,
"data": {"digest": True, "digest_age": 19},
"data": {"notifications_enabled": True, "digest": True, "digest_age": 19},
},
]
headers = {"Authorization": "Bearer token"}
Expand Down
Loading