From b39318f91befc602090854aa20b37e56e680827a Mon Sep 17 00:00:00 2001 From: junaid Date: Tue, 23 Sep 2025 09:57:23 +0000 Subject: [PATCH 1/2] Add notifications_enabled column, refactor digest-preference to notification-preference, fix linting issues --- ...tification-table-and-add-project-types.sql | 2 +- .../2025-05-12-add-provisional-user-table.sql | 1 + ... => 2025-06-11-add-notification-scope.sql} | 0 admin/sql/create_tables.sql | 3 +- config.py.example | 1 + manage.py | 1 + metabrainz/__init__.py | 3 +- metabrainz/db/notification.py | 9 ++-- metabrainz/db/tests/test_notification.py | 8 +-- metabrainz/mail.py | 2 +- metabrainz/model/user_preference.py | 3 +- metabrainz/notifications/views.py | 42 +++++++++++---- metabrainz/notifications/views_test.py | 51 ++++++++++++++----- 13 files changed, 91 insertions(+), 35 deletions(-) rename admin/schema_updates/{2025-06-11-add-notification-scope => 2025-06-11-add-notification-scope.sql} (100%) diff --git a/admin/schema_updates/2025-05-12-add-notification-table-and-add-project-types.sql b/admin/schema_updates/2025-05-12-add-notification-table-and-add-project-types.sql index d8031b52..ae0c3715 100644 --- a/admin/schema_updates/2025-05-12-add-notification-table-and-add-project-types.sql +++ b/admin/schema_updates/2025-05-12-add-notification-table-and-add-project-types.sql @@ -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. diff --git a/admin/schema_updates/2025-05-12-add-provisional-user-table.sql b/admin/schema_updates/2025-05-12-add-provisional-user-table.sql index 663e8381..4e25d265 100644 --- a/admin/schema_updates/2025-05-12-add-provisional-user-table.sql +++ b/admin/schema_updates/2025-05-12-add-provisional-user-table.sql @@ -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. ); diff --git a/admin/schema_updates/2025-06-11-add-notification-scope b/admin/schema_updates/2025-06-11-add-notification-scope.sql similarity index 100% rename from admin/schema_updates/2025-06-11-add-notification-scope rename to admin/schema_updates/2025-06-11-add-notification-scope.sql diff --git a/admin/sql/create_tables.sql b/admin/sql/create_tables.sql index dd160775..91542d25 100644 --- a/admin/sql/create_tables.sql +++ b/admin/sql/create_tables.sql @@ -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. @@ -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. ); diff --git a/config.py.example b/config.py.example index 14df5965..a6589f3e 100644 --- a/config.py.example +++ b/config.py.example @@ -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 diff --git a/manage.py b/manage.py index b2009be6..8c93c954 100644 --- a/manage.py +++ b/manage.py @@ -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"]) 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.') diff --git a/metabrainz/__init__.py b/metabrainz/__init__.py index 5424ec6e..68d47ca6 100644 --- a/metabrainz/__init__.py +++ b/metabrainz/__init__.py @@ -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 @@ -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) diff --git a/metabrainz/db/notification.py b/metabrainz/db/notification.py index 8ca78cfd..e1c1455a 100644 --- a/metabrainz/db/notification.py +++ b/metabrainz/db/notification.py @@ -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) @@ -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} diff --git a/metabrainz/db/tests/test_notification.py b/metabrainz/db/tests/test_notification.py index 7c07f774..f731772a 100644 --- a/metabrainz/db/tests/test_notification.py +++ b/metabrainz/db/tests/test_notification.py @@ -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":"1@abc.com"}, + {"musicbrainz_row_id": 1, "notifications_enabled": True, "digest": True, "digest_age": 7, "user_email":"1@abc.com"}, # User who should get their digest notification. - {"musicbrainz_row_id": 2, "digest": True, "digest_age": 3, "user_email":"2@abc.com"}, + {"musicbrainz_row_id": 2, "notifications_enabled": True, "digest": True, "digest_age": 3, "user_email":"2@abc.com"}, # Digest is false, so no digest notification. - {"musicbrainz_row_id": 3, "digest": False, "digest_age": None, "user_email":"3@abc.com"}, + {"musicbrainz_row_id": 3, "notifications_enabled": True, "digest": False, "digest_age": None, "user_email":"3@abc.com"}, + # Notifications are not enabled, so no digest notification. + {"musicbrainz_row_id": 4, "notifications_enabled": False, "digest": True, "digest_age": 6, "user_email": "4@abc.com"}, ] query = sqlalchemy.text( """INSERT INTO user_preference(musicbrainz_row_id, digest, digest_age, user_email) VALUES(:musicbrainz_row_id, :digest, :digest_age, :user_email)""" diff --git a/metabrainz/mail.py b/metabrainz/mail.py index 0e39e3d7..2bb2cb64 100644 --- a/metabrainz/mail.py +++ b/metabrainz/mail.py @@ -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 = [] diff --git a/metabrainz/model/user_preference.py b/metabrainz/model/user_preference.py index e1ed977a..9afeaf6c 100644 --- a/metabrainz/model/user_preference.py +++ b/metabrainz/model/user_preference.py @@ -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) @@ -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_notifications_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 diff --git a/metabrainz/notifications/views.py b/metabrainz/notifications/views.py index d913d465..606283d7 100644 --- a/metabrainz/notifications/views.py +++ b/metabrainz/notifications/views.py @@ -332,18 +332,19 @@ def send_notifications(): return jsonify({'status':'ok'}), 200 -@notification_bp.route("//digest-preference", methods=["GET", "POST"]) +@notification_bp.route("//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 `. - - **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 } @@ -353,9 +354,10 @@ 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 @@ -363,6 +365,7 @@ def set_digest_preference(user_id): ..code-block:: json { + "notifications_enabled": true, "digest": true, "digest_age": 17 } @@ -371,6 +374,7 @@ def set_digest_preference(user_id): ..code-block:: json { + "notifications_enabled": true, "digest": true, "digest_age": 17 } @@ -387,13 +391,22 @@ 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: @@ -401,8 +414,19 @@ def set_digest_preference(user_id): 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_notifications_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)) diff --git a/metabrainz/notifications/views_test.py b/metabrainz/notifications/views_test.py index c282e5ea..563d89b9 100644 --- a/metabrainz/notifications/views_test.py +++ b/metabrainz/notifications/views_test.py @@ -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 @@ -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_notifications_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_notifications_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.") @@ -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"} From 21b6d0ca086556ad48373aa158c81722689977eb Mon Sep 17 00:00:00 2001 From: junaid Date: Tue, 23 Sep 2025 10:28:20 +0000 Subject: [PATCH 2/2] fix Typo --- metabrainz/model/user_preference.py | 2 +- metabrainz/notifications/views.py | 2 +- metabrainz/notifications/views_test.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/metabrainz/model/user_preference.py b/metabrainz/model/user_preference.py index 9afeaf6c..cd62c8a1 100644 --- a/metabrainz/model/user_preference.py +++ b/metabrainz/model/user_preference.py @@ -22,7 +22,7 @@ def get(cls: Type["UserPreference"], **kwargs) -> Optional["UserPreference"]: return cls.query.filter_by(**kwargs).first() @classmethod - def set_notifications_preference(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 diff --git a/metabrainz/notifications/views.py b/metabrainz/notifications/views.py index 606283d7..da5cca80 100644 --- a/metabrainz/notifications/views.py +++ b/metabrainz/notifications/views.py @@ -414,7 +414,7 @@ def notification_preference(user_id): raise APIBadRequest("Invalid digest age.") try: - result = UserPreference.set_notifications_preference( + result = UserPreference.set_notification_preference( musicbrainz_row_id=user_id, notifications_enabled=notifications_enabled, digest=digest, diff --git a/metabrainz/notifications/views_test.py b/metabrainz/notifications/views_test.py index 563d89b9..a0e2dcf3 100644 --- a/metabrainz/notifications/views_test.py +++ b/metabrainz/notifications/views_test.py @@ -273,7 +273,7 @@ def test_notification_preference(self, mock_requests, mock_digest): self.assertEqual(res.json['error'], 'Invalid user_id.') # POST method test - mock_digest.set_notifications_preference.return_value = mock.MagicMock( + 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} @@ -309,7 +309,7 @@ def test_notification_preference(self, mock_requests, mock_digest): self.assert400(res) self.assertEqual(res.json['error'], b[1]) - mock_digest.set_notifications_preference.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.")