-
-
Notifications
You must be signed in to change notification settings - Fork 256
Add endpoints for digest settings. #3345
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
base: metabrainz-notifications
Are you sure you want to change the base?
Changes from 7 commits
c04c7b3
2f2e13f
2f435bf
351cf76
7686973
2c1ade8
061498c
cf934fc
895dc81
b87932e
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 |
---|---|---|
@@ -0,0 +1,118 @@ | ||
import * as React from "react"; | ||
import { toast } from "react-toastify"; | ||
|
||
export default function DigestSettings() { | ||
const [digestEnabled, setDigestEnabled] = React.useState(false); | ||
const [digestAge, setDigestAge] = React.useState<number | null>(null); | ||
const [initialDigestEnabled, setInitialDigestEnabled] = React.useState(false); | ||
const [initialDigestAge, setInitialDigestAge] = React.useState<number | null>( | ||
null | ||
); | ||
const [loading, setLoading] = React.useState(true); | ||
const [saving, setSaving] = React.useState(false); | ||
|
||
React.useEffect(() => { | ||
async function fetchDigestSettings() { | ||
try { | ||
const response = await fetch("/settings/digest-setting/"); | ||
if (!response.ok) { | ||
throw new Error(`${response.status} HTTP response.`); | ||
} | ||
const data = await response.json(); | ||
setDigestEnabled(data.digest); | ||
setDigestAge(data.digest_age); | ||
setInitialDigestEnabled(data.digest); | ||
setInitialDigestAge(data.digest_age); | ||
} catch (error) { | ||
// eslint-disable-next-line no-console | ||
console.error("Could not fetch digest settings.", error); | ||
toast.error("Failed to load digest settings."); | ||
} finally { | ||
setLoading(false); | ||
} | ||
} | ||
fetchDigestSettings(); | ||
}, []); | ||
|
||
const updateDigestSettings = async (e: React.FormEvent) => { | ||
e.preventDefault(); | ||
setSaving(true); | ||
try { | ||
const response = await fetch("/settings/digest-setting/", { | ||
method: "POST", | ||
headers: { | ||
"Content-Type": "application/json", | ||
}, | ||
body: JSON.stringify({ | ||
digest: digestEnabled, | ||
digest_age: digestAge, | ||
}), | ||
}); | ||
if (!response.ok) { | ||
throw new Error(`${response.status} HTTP response.`); | ||
} | ||
toast.success("Digest settings saved successfully"); | ||
} catch (error) { | ||
// eslint-disable-next-line no-console | ||
console.error("Could not update digest settings.", error); | ||
toast.error("Failed to save digest settings. Please try again."); | ||
} finally { | ||
setSaving(false); | ||
} | ||
}; | ||
|
||
const hasNoChanges = | ||
digestEnabled === initialDigestEnabled && digestAge === initialDigestAge; | ||
|
||
if (loading) { | ||
return <div>Loading digest settings...</div>; | ||
} | ||
|
||
return ( | ||
<div className="mb-4"> | ||
<form className="mb-4" onSubmit={updateDigestSettings}> | ||
<h3 className="mt-4">Digest Settings</h3> | ||
<div className="form-check mb-4"> | ||
<input | ||
className="form-check-input" | ||
type="checkbox" | ||
id="enable-digest" | ||
checked={digestEnabled} | ||
onChange={(e) => setDigestEnabled(e.target.checked)} | ||
/> | ||
<label className="form-check-label" htmlFor="enable-digest"> | ||
Enable digest | ||
</label> | ||
</div> | ||
|
||
{digestEnabled && ( | ||
<div className="mb-4" style={{ maxWidth: "400px" }}> | ||
<label htmlFor="digest-age" className="form-label"> | ||
Digest age (in days) | ||
</label> | ||
<input | ||
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. How do you like the number of days input inline, like I did in my mockup? <div class="mb-4">Send digest emails every <input type="number" class="d-inline form-control" id="digest-age" min="1" max="100" style="max-width: 4em;"> days</div> 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 prefer your version, its more informative. |
||
type="number" | ||
className="form-control" | ||
id="digest-age" | ||
min={1} | ||
max={100} | ||
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 wonder if that is an acceptable upper limit. 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. We currently have 100 set as the upper limit in the endpoint. But yes, its a lot. What do you think would be a reasonable upper limit? 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. 30 days. |
||
value={digestAge ?? ""} | ||
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. What should be the default value? 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. We can pass it as None through the endpoint, MeB server will default it to 7 days. |
||
onChange={(e) => { | ||
const inputValue = e.target.value; | ||
setDigestAge(inputValue === "" ? null : Number(inputValue)); | ||
}} | ||
/> | ||
</div> | ||
)} | ||
|
||
<button | ||
className="btn btn-success" | ||
type="submit" | ||
disabled={hasNoChanges || saving} | ||
> | ||
Save digest settings | ||
</button> | ||
</form> | ||
</div> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,12 @@ | ||
from requests.auth import HTTPBasicAuth | ||
from oauthlib.oauth2 import BackendApplicationClient | ||
from requests_oauthlib import OAuth2Session | ||
import requests | ||
from brainzutils import cache | ||
from flask import current_app | ||
import requests | ||
|
||
from oauthlib.oauth2 import BackendApplicationClient | ||
from requests.auth import HTTPBasicAuth | ||
from requests_oauthlib import OAuth2Session | ||
|
||
TOKEN_CACHE_KEY = "notification_access_token" | ||
METABRAINZ_NOTIFICATIONS_SEND_URL = "https://metabrainz.org/notification/send" | ||
METABRAINZ_NOTIFICATIONS_ENDPOINT = "https://metabrainz.org/notification" | ||
|
||
|
||
def send_notification( | ||
|
@@ -70,13 +69,65 @@ def send_multiple_notifications(notifications: list[dict]): | |
|
||
token = _fetch_token() | ||
headers = {"Authorization": f"Bearer {token}"} | ||
notification_send_endpoint = METABRAINZ_NOTIFICATIONS_ENDPOINT + "/send" | ||
|
||
response = requests.post( | ||
url=METABRAINZ_NOTIFICATIONS_SEND_URL, json=notifications, headers=headers | ||
) | ||
response = requests.post(url=notification_send_endpoint, json=notifications, headers=headers) | ||
response.raise_for_status() | ||
|
||
|
||
def get_digest_preference(musicbrainz_row_id: int) -> dict: | ||
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. docstrings 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. Oops, sorry. Got lost in branches. Added it now. |
||
"""Retrieves the current digest preference of a user. | ||
|
||
Args: | ||
``musicbrainz_row_id`` (int) | ||
|
||
Returns: | ||
A dict containing | ||
``digest`` (bool): Whether digest is enabled for the user. | ||
``digest_age`` (int): The digest_age set for the user. | ||
|
||
Raises: | ||
A HTTPError if there's a failure. | ||
|
||
""" | ||
digest_endpoint = METABRAINZ_NOTIFICATIONS_ENDPOINT + f"/{musicbrainz_row_id}/digest-preference" | ||
token = _fetch_token() | ||
headers = {"Authorization": f"Bearer {token}"} | ||
|
||
response = requests.get(url=digest_endpoint, headers=headers) | ||
response.raise_for_status() | ||
|
||
return response.json() | ||
|
||
|
||
def set_digest_preference(musicbrainz_row_id: int, digest: bool, digest_age: int = None) -> dict: | ||
"""Sets the digest preference for a user. | ||
|
||
Args: | ||
``musicbrainz_row_id`` (int) | ||
``digest`` (bool): Whether digest should be enabled. | ||
``digest_age`` (int): The age in days for the digest. If set to None, MeB server defaults it to 7 days. | ||
|
||
Returns: | ||
A dict containing | ||
``digest`` (bool): Whether digest is enabled for the user. | ||
``digest_age`` (int): The digest age set for the user. | ||
|
||
Raises: | ||
A HTTPError if there's a failure. | ||
|
||
""" | ||
digest_endpoint = METABRAINZ_NOTIFICATIONS_ENDPOINT + f"/{musicbrainz_row_id}/digest-preference" | ||
token = _fetch_token() | ||
headers = {"Authorization": f"Bearer {token}"} | ||
data = {"digest": digest, "digest_age": digest_age} | ||
|
||
response = requests.post(url=digest_endpoint, json=data, headers=headers) | ||
response.raise_for_status() | ||
|
||
return response.json() | ||
|
||
|
||
def _fetch_token() -> str: | ||
"""Helper function to fetch OAuth2 token from redis cache, If no token is found or it's expired, a new token is requested.""" | ||
|
||
|
@@ -90,9 +141,7 @@ def _fetch_token() -> str: | |
|
||
client = BackendApplicationClient(client_id=client_id, scope="notification") | ||
oauth = OAuth2Session(client=client) | ||
token = oauth.fetch_token( | ||
token_url=token_url, auth=HTTPBasicAuth(client_id, client_secret) | ||
) | ||
token = oauth.fetch_token(token_url=token_url, auth=HTTPBasicAuth(client_id, client_secret)) | ||
access_token = token["access_token"] | ||
expires_in = token["expires_in"] | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
from unittest.mock import patch, MagicMock | ||
from listenbrainz.domain import metabrainz_notifications | ||
from listenbrainz.tests.integration import NonAPIIntegrationTestCase | ||
|
||
|
||
class MetabrainNotificationsTestCase(NonAPIIntegrationTestCase): | ||
@patch("listenbrainz.domain.metabrainz_notifications.send_multiple_notifications") | ||
def test_send_notification(self, mock_send_multiple): | ||
metabrainz_notifications.send_notification( | ||
subject="test123", | ||
body="testbody456", | ||
musicbrainz_row_id=123, | ||
user_email="[email protected]", | ||
from_addr="[email protected]", | ||
) | ||
expected_notification = [ | ||
[ | ||
{ | ||
"subject": "test123", | ||
"body": "testbody456", | ||
"user_id": 123, | ||
"to": "[email protected]", | ||
"project": "listenbrainz", | ||
"sent_from": "[email protected]", | ||
"send_email": True, | ||
"important": True, | ||
"expire_age": 7, | ||
} | ||
] | ||
] | ||
|
||
mock_send_multiple.assert_called_once_with(expected_notification) | ||
|
||
@patch("listenbrainz.domain.metabrainz_notifications._fetch_token") | ||
@patch("requests.post") | ||
def test_send_multiple_notifications(self, mock_post, mock_fetch_token): | ||
mock_fetch_token.return_value = "access_token" | ||
mock_response = MagicMock() | ||
mock_response.raise_for_status.return_value = None | ||
mock_post.return_value = mock_response | ||
|
||
notifications = [{"user_id": 1, "body": "Notification 1"}] | ||
|
||
metabrainz_notifications.send_multiple_notifications(notifications) | ||
|
||
mock_fetch_token.assert_called_once() | ||
expected_url = "https://metabrainz.org/notification/send" | ||
expected_headers = {"Authorization": "Bearer access_token"} | ||
mock_post.assert_called_once_with( | ||
url=expected_url, json=notifications, headers=expected_headers | ||
) | ||
mock_response.raise_for_status.assert_called_once() | ||
|
||
@patch("listenbrainz.domain.metabrainz_notifications._fetch_token") | ||
@patch("requests.get") | ||
def test_get_digest_preference(self, mock_get, mock_fetch_token): | ||
mock_fetch_token.return_value = "access_token" | ||
mock_response = MagicMock() | ||
mock_response.raise_for_status.return_value = None | ||
mock_response.json.return_value = {"digest": True, "digest_age": 7} | ||
mock_get.return_value = mock_response | ||
|
||
result = metabrainz_notifications.get_digest_preference(musicbrainz_row_id=456) | ||
|
||
expected_url = "https://metabrainz.org/notification/456/digest-preference" | ||
mock_get.assert_called_once_with( | ||
url=expected_url, headers={"Authorization": "Bearer access_token"} | ||
) | ||
self.assertEqual(result, {"digest": True, "digest_age": 7}) | ||
|
||
@patch("listenbrainz.domain.metabrainz_notifications._fetch_token") | ||
@patch("requests.post") | ||
def test_set_digest_preference(self, mock_post, mock_fetch_token): | ||
mock_fetch_token.return_value = "access_token" | ||
mock_response = MagicMock() | ||
mock_response.raise_for_status.return_value = None | ||
mock_response.json.return_value = {"digest": True, "digest_age": 14} | ||
mock_post.return_value = mock_response | ||
|
||
result = metabrainz_notifications.set_digest_preference( | ||
musicbrainz_row_id=789, digest=True, digest_age=14 | ||
) | ||
|
||
expected_url = "https://metabrainz.org/notification/789/digest-preference" | ||
expected_data = {"digest": True, "digest_age": 14} | ||
mock_post.assert_called_once_with( | ||
url=expected_url, json=expected_data, headers={"Authorization": "Bearer access_token"} | ||
) | ||
self.assertEqual(result, {"digest": True, "digest_age": 14}) |
Uh oh!
There was an error while loading. Please reload this page.